Skip to content

Commit

Permalink
Multi-address client: move setHostHeader to key factory, avoid NPE warn
Browse files Browse the repository at this point in the history
Motivation:

`setHostHeader` method is too far from where it's used and IDE warns
that `authority` might be `null`.

Modifications:

- Move `setHostHeader` to `CachingKeyFactory`;
- Pass host and port as arguments from the context where it's known that
the `host` is non-null;
- Use `compareTo` for HTTP version check;
  • Loading branch information
idelpivnitskiy committed Jun 17, 2024
1 parent ff218cc commit 5c3e372
Showing 1 changed file with 14 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@
import static io.servicetalk.http.api.HttpExecutionStrategies.offloadAll;
import static io.servicetalk.http.api.HttpExecutionStrategies.offloadNone;
import static io.servicetalk.http.api.HttpHeaderNames.HOST;
import static io.servicetalk.http.api.HttpProtocolVersion.HTTP_1_0;
import static io.servicetalk.http.api.HttpProtocolVersion.HTTP_1_1;
import static io.servicetalk.http.api.HttpRequestMetaDataFactory.newRequestMetaData;
import static io.servicetalk.http.api.HttpRequestMethod.GET;
Expand Down Expand Up @@ -172,7 +171,7 @@ UrlKey get(final HttpRequestMetaData metaData) throws MalformedURLException {
final int parsedPort = metaData.port();
final int port = parsedPort >= 0 ? parsedPort :
(HTTPS_SCHEME.equals(scheme) ? defaultHttpsPort : defaultHttpPort);
setHostHeader(metaData);
setHostHeader(metaData, host, parsedPort);
metaData.requestTarget(absoluteToRelativeFormRequestTarget(metaData.requestTarget(), scheme, host));

final String key = scheme + ':' + host + ':' + port;
Expand All @@ -190,6 +189,18 @@ private static String ensureUrlComponentNonNull(@Nullable final String value,
return value;
}

private static void setHostHeader(final HttpRequestMetaData metaData, final String host, final int port) {
if (metaData.version().compareTo(HTTP_1_1) >= 0 && !metaData.headers().contains(HOST)) {
// Host header must be identical to authority component of the target URI,
// as described in https://datatracker.ietf.org/doc/html/rfc7230#section-5.4
String authority = host;
if (port >= 0) {
authority = authority + ':' + port;
}
metaData.headers().add(HOST, authority);
}
}

// This code is similar to io.servicetalk.http.utils.RedirectSingle#absoluteToRelativeFormRequestTarget
// but cannot be shared because we don't have an internal module for http
private static String absoluteToRelativeFormRequestTarget(final String requestTarget,
Expand Down Expand Up @@ -285,7 +296,7 @@ public StreamingHttpClient apply(final UrlKey urlKey) {

private static void singleClientStrategyUpdate(ContextMap context, HttpExecutionStrategy singleStrategy) {
HttpExecutionStrategy requestStrategy = context.getOrDefault(HTTP_EXECUTION_STRATEGY_KEY, defaultStrategy());
assert null != requestStrategy : "Request strategy unexpectedly null";
assert requestStrategy != null : "Request strategy unexpectedly null";
HttpExecutionStrategy useStrategy = defaultStrategy() == requestStrategy ?
// For all apis except async streaming default conversion has already been done.
// This is the default to required strategy resolution for the async streaming client.
Expand Down Expand Up @@ -496,14 +507,4 @@ private static int verifyPortRange(final int port) {
}
return port;
}

private static void setHostHeader(HttpRequestMetaData metaData) {
if (!HTTP_1_0.equals(metaData.version()) && !metaData.headers().contains(HOST)) {
CharSequence authority = metaData.host();
if (metaData.port() >= 0) {
authority = authority + ":" + metaData.port();
}
metaData.headers().add(HOST, authority);
}
}
}

0 comments on commit 5c3e372

Please sign in to comment.