ApacheHttpClientBlockingChannel uses URL to build request

[schlosna]

Before this PR

Noticed new URI(String) and some of its side effects in JFRs from services making high volumes of Dialogue requests.

Splitting out of PR #2432

After this PR

==COMMIT_MSG==
Parsing String to URI can be expensive in terms of CPU and allocations for high throughput services.
==COMMIT_MSG==

Possible downsides?

[changelog-app]

Generate changelog in changelog/@unreleased

What do the change types mean?

• feature: A new feature of the service.
• improvement: An incremental improvement in the functionality or operation of the service.
• fix: Remedies the incorrect behaviour of a component of the service in a backwards-compatible way.
• break: Has the potential to break consumers of this service's API, inclusive of both Palantir services
  and external consumers of the service's API (e.g. customer-written software or integrations).
• deprecation: Advertises the intention to remove service functionality without any change to the
  operation of the service itself.
• manualTask: Requires the possibility of manual intervention (running a script, eyeballing configuration,
  performing database surgery, ...) at the time of upgrade for it to succeed.
• migration: A fully automatic upgrade migration task with no engineer input required.

Note: only one type should be chosen.

How are new versions calculated?

• ❗The break and manual task changelog types will result in a major release!
• 🐛 The fix changelog type will result in a minor release in most cases, and a patch release version for patch branches. This behaviour is configurable in autorelease.
• ✨ All others will result in a minor version release.

Type

• Feature
• Improvement
• Fix
• Break
• Deprecation
• Manual task
• Migration

Description

Parsing String to URI can be expensive in terms of CPU and allocations for high throughput services.

Check the box to generate changelog(s)

• Generate changelog entry

[carterkozak]

I wonder if it's worth rolling the dice with something along these lines to avoid string tokenizing/manipulation overhead? We have most of the data parsed out via URL already, I believe it's just ipv6 brackets that URL doesn't handle well.

String host = url.getHost();
if (host != null && host.startsWith("[") && host.endsWith("]")) {
    host = host.substring(1, host.length() - 1);
}
return new URIAuthority(url.getUserInfo(), host, url.getPort());

[carterkozak]

Alternatively, it may be safer if we check host != null && host.startsWith("[") and in that case, we use the parsing URIAuthority.create path, otherwise (in the common case) use new URIAuthority(url.getUserInfo(), host, url.getPort());

[carterkozak]

+1 I've verified that url.getAuthority() is equivalent to URI.getRawAuthority(), which is used internally within apache httpclient.

[schlosna]

Specifically we're computing the authority similar to what is done in:

https://github.com/apache/httpcomponents-core/blob/rel/v5.3.1/httpcore5/src/main/java/org/apache/hc/core5/http/support/AbstractRequestBuilder.java#L176-L186

[carterkozak]

Let's add a couple test cases for encoding edge cases:

• A URL which includes percent-escaped values in the users password: https://user:slash%[email protected]
• A URL which includes bracket-formatted ipv6 address: https://user@[::1]/path (perhaps with and without a port)

[schlosna]

Thanks, added some more test coverage.

[carterkozak]

thanks!

[autorelease3]

Released 4.6.0