Skip to content

Commit be485c6

Browse files
ok2cok2c
committed
ProtocolExec, AsyncProtocolExec: extract path prefix lazily on demand; test coverage
1 parent 570a63d commit be485c6

9 files changed

Lines changed: 595 additions & 85 deletions

File tree

‎httpclient5/src/main/java/org/apache/hc/client5/http/impl/RequestSupport.java‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.util.List;
3434

3535
import org.apache.hc.core5.annotation.Internal;
36+
import org.apache.hc.core5.function.Supplier;
3637
import org.apache.hc.core5.http.HttpRequest;
3738
import org.apache.hc.core5.net.PathSegmentCodec;
3839
import org.apache.hc.core5.net.PercentCodec;
@@ -93,8 +94,45 @@ static List<String> optimizePathSegments(final List<String> inputSegments) {
9394
}
9495
}
9596

97+
/**
98+
* @since 5.7
99+
*/
100+
public static Supplier<String> pathPrefixSupplier(final String path) {
101+
return new Supplier<String>() {
102+
103+
private volatile String cached;
104+
105+
@Override
106+
public String get() {
107+
// This is racy but the class is not meant to be used
108+
// concurrently by multiple threads
109+
String value = cached;
110+
if (value == null) {
111+
value = extractPathPrefix(path);
112+
cached = value;
113+
}
114+
return value;
115+
}
116+
117+
};
118+
}
119+
120+
/**
121+
* @deprecated Do not use.
122+
*/
123+
@Deprecated
96124
public static String extractPathPrefix(final HttpRequest request) {
97-
String path = request.getPath();
125+
return extractPathPrefix(request.getPath());
126+
}
127+
128+
/**
129+
* @since 5.7
130+
*/
131+
public static String extractPathPrefix(final String requestPath) {
132+
if (requestPath.isEmpty()) {
133+
return "/";
134+
}
135+
String path = requestPath;
98136
boolean hasSegments = false;
99137
boolean requiresOptimization = false;
100138
if (!path.startsWith("/")) {

‎httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncConnectExec.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,7 @@ private void createTunnel(
395395
final AuthExchange proxyAuthExchange = proxy != null ? clientContext.getAuthExchange(proxy) : new AuthExchange();
396396

397397
if (authCacheKeeper != null) {
398-
authCacheKeeper.loadPreemptively(proxy, null, proxyAuthExchange, clientContext);
398+
authCacheKeeper.loadPreemptivelyFromCache(proxy, null, proxyAuthExchange, clientContext);
399399
}
400400

401401
final AsyncClientExchangeHandler internalExchangeHandler = new AsyncClientExchangeHandler() {
@@ -539,9 +539,9 @@ private boolean needAuthentication(
539539

540540
if (authCacheKeeper != null) {
541541
if (proxyAuthRequested) {
542-
authCacheKeeper.updateOnChallenge(proxy, null, proxyAuthExchange, context);
542+
authCacheKeeper.updateCacheOnChallenge(proxy, null, proxyAuthExchange, context);
543543
} else {
544-
authCacheKeeper.updateOnNoChallenge(proxy, null, proxyAuthExchange, context);
544+
authCacheKeeper.updateCacheOnNoChallenge(proxy, null, proxyAuthExchange, context);
545545
}
546546
}
547547

@@ -550,7 +550,7 @@ private boolean needAuthentication(
550550
proxyAuthStrategy, proxyAuthExchange, context);
551551

552552
if (authCacheKeeper != null) {
553-
authCacheKeeper.updateOnResponse(proxy, null, proxyAuthExchange, context);
553+
authCacheKeeper.updateCacheOnResponse(proxy, null, proxyAuthExchange, context);
554554
}
555555

556556
return updated;

‎httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java‎

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@
5050
import org.apache.hc.core5.annotation.Contract;
5151
import org.apache.hc.core5.annotation.Internal;
5252
import org.apache.hc.core5.annotation.ThreadingBehavior;
53+
import org.apache.hc.core5.function.Factory;
54+
import org.apache.hc.core5.function.Supplier;
5355
import org.apache.hc.core5.http.EntityDetails;
5456
import org.apache.hc.core5.http.Header;
5557
import org.apache.hc.core5.http.HttpException;
@@ -84,24 +86,38 @@ public final class AsyncProtocolExec implements AsyncExecChainHandler {
8486

8587
private static final Logger LOG = LoggerFactory.getLogger(AsyncProtocolExec.class);
8688

89+
private static final Factory<HttpRequest, Supplier<String>> DEFAULT_PREFIX_EXTRACTOR_FACTORY = r ->
90+
RequestSupport.pathPrefixSupplier(r.getPath());
91+
8792
private final AuthenticationStrategy targetAuthStrategy;
8893
private final AuthenticationStrategy proxyAuthStrategy;
8994
private final AuthenticationHandler authenticator;
9095
private final SchemePortResolver schemePortResolver;
96+
private final Factory<HttpRequest, Supplier<String>> prefixExtractorFactory;
9197
private final AuthCacheKeeper authCacheKeeper;
9298

9399
AsyncProtocolExec(
94100
final AuthenticationStrategy targetAuthStrategy,
95101
final AuthenticationStrategy proxyAuthStrategy,
96102
final SchemePortResolver schemePortResolver,
103+
final Factory<HttpRequest, Supplier<String>> prefixExtractorFactory,
97104
final boolean authCachingDisabled) {
98105
this.targetAuthStrategy = Args.notNull(targetAuthStrategy, "Target authentication strategy");
99106
this.proxyAuthStrategy = Args.notNull(proxyAuthStrategy, "Proxy authentication strategy");
100107
this.authenticator = new AuthenticationHandler();
101108
this.schemePortResolver = schemePortResolver != null ? schemePortResolver : DefaultSchemePortResolver.INSTANCE;
109+
this.prefixExtractorFactory = prefixExtractorFactory != null ? prefixExtractorFactory : DEFAULT_PREFIX_EXTRACTOR_FACTORY;
102110
this.authCacheKeeper = authCachingDisabled ? null : new AuthCacheKeeper(this.schemePortResolver);
103111
}
104112

113+
public AsyncProtocolExec(
114+
final AuthenticationStrategy targetAuthStrategy,
115+
final AuthenticationStrategy proxyAuthStrategy,
116+
final SchemePortResolver schemePortResolver,
117+
final boolean authCachingDisabled) {
118+
this(targetAuthStrategy, proxyAuthStrategy, schemePortResolver, DEFAULT_PREFIX_EXTRACTOR_FACTORY, authCachingDisabled);
119+
}
120+
105121
@Override
106122
public void execute(
107123
final HttpRequest userRequest,
@@ -148,33 +164,33 @@ public void execute(
148164
request.getScheme(),
149165
authority.getHostName(),
150166
schemePortResolver.resolve(request.getScheme(), authority));
151-
final String pathPrefix = RequestSupport.extractPathPrefix(request);
167+
final Supplier<String> pathPrefixSupplier = prefixExtractorFactory.create(request);
152168
final AuthExchange targetAuthExchange = clientContext.getAuthExchange(target);
153169
final AuthExchange proxyAuthExchange = proxy != null ? clientContext.getAuthExchange(proxy) : new AuthExchange();
154170

155171
if (!targetAuthExchange.isConnectionBased() &&
156172
targetAuthExchange.getPathPrefix() != null &&
157-
!pathPrefix.startsWith(targetAuthExchange.getPathPrefix())) {
173+
!pathPrefixSupplier.get().startsWith(targetAuthExchange.getPathPrefix())) {
158174
// force re-authentication if the current path prefix does not match
159175
// that of the previous authentication exchange.
160176
targetAuthExchange.reset();
161177
}
162178

163179
if (authCacheKeeper != null) {
164-
authCacheKeeper.loadPreemptively(target, pathPrefix, targetAuthExchange, clientContext);
180+
authCacheKeeper.loadPreemptivelyFromCache(target, pathPrefixSupplier, targetAuthExchange, clientContext);
165181
if (proxy != null) {
166-
authCacheKeeper.loadPreemptively(proxy, null, proxyAuthExchange, clientContext);
182+
authCacheKeeper.loadPreemptivelyFromCache(proxy, null, proxyAuthExchange, clientContext);
167183
}
168184
}
169185

170186
final AtomicBoolean challenged = new AtomicBoolean(false);
171-
internalExecute(target, pathPrefix, targetAuthExchange, proxyAuthExchange,
187+
internalExecute(target, pathPrefixSupplier, targetAuthExchange, proxyAuthExchange,
172188
challenged, request, entityProducer, scope, chain, asyncExecCallback);
173189
}
174190

175191
private void internalExecute(
176192
final HttpHost target,
177-
final String pathPrefix,
193+
final Supplier<String> pathPrefixSupplier,
178194
final AuthExchange targetAuthExchange,
179195
final AuthExchange proxyAuthExchange,
180196
final AtomicBoolean challenged,
@@ -220,7 +236,7 @@ public AsyncDataConsumer handleResponse(
220236
// A 407 received through a tunnel comes from the origin, not from the proxy
221237
route.isTunnelled() ? null : proxy,
222238
target,
223-
pathPrefix,
239+
pathPrefixSupplier,
224240
response,
225241
clientContext)) {
226242
challenged.set(true);
@@ -272,7 +288,7 @@ public void completed() {
272288
if (entityProducer != null) {
273289
entityProducer.releaseResources();
274290
}
275-
internalExecute(target, pathPrefix, targetAuthExchange, proxyAuthExchange,
291+
internalExecute(target, pathPrefixSupplier, targetAuthExchange, proxyAuthExchange,
276292
challenged, request, entityProducer, scope, chain, asyncExecCallback);
277293
} catch (final HttpException | IOException ex) {
278294
asyncExecCallback.failed(ex);
@@ -303,7 +319,7 @@ private boolean needAuthentication(
303319
final AuthExchange proxyAuthExchange,
304320
final HttpHost proxy,
305321
final HttpHost target,
306-
final String pathPrefix,
322+
final Supplier<String> pathPrefixSupplier,
307323
final HttpResponse response,
308324
final HttpClientContext context) throws AuthenticationException, MalformedChallengeException {
309325
final RequestConfig config = context.getRequestConfigOrDefault();
@@ -314,9 +330,9 @@ private boolean needAuthentication(
314330

315331
if (authCacheKeeper != null) {
316332
if (targetAuthRequested) {
317-
authCacheKeeper.updateOnChallenge(target, pathPrefix, targetAuthExchange, context);
333+
authCacheKeeper.updateCacheOnChallenge(target, pathPrefixSupplier, targetAuthExchange, context);
318334
} else {
319-
authCacheKeeper.updateOnNoChallenge(target, pathPrefix, targetAuthExchange, context);
335+
authCacheKeeper.updateCacheOnNoChallenge(target, pathPrefixSupplier, targetAuthExchange, context);
320336
}
321337
}
322338

@@ -326,9 +342,9 @@ private boolean needAuthentication(
326342

327343
if (proxy != null && authCacheKeeper != null) {
328344
if (proxyAuthRequested) {
329-
authCacheKeeper.updateOnChallenge(proxy, null, proxyAuthExchange, context);
345+
authCacheKeeper.updateCacheOnChallenge(proxy, null, proxyAuthExchange, context);
330346
} else {
331-
authCacheKeeper.updateOnNoChallenge(proxy, null, proxyAuthExchange, context);
347+
authCacheKeeper.updateCacheOnNoChallenge(proxy, null, proxyAuthExchange, context);
332348
}
333349
}
334350

@@ -339,18 +355,18 @@ private boolean needAuthentication(
339355
targetAuthStrategy, targetAuthExchange, context);
340356

341357
if (!targetAuthExchange.isConnectionBased() && targetAuthExchange.getPathPrefix() == null) {
342-
targetAuthExchange.setPathPrefix(pathPrefix);
358+
targetAuthExchange.setPathPrefix(pathPrefixSupplier.get());
343359
}
344360
if (authCacheKeeper != null) {
345-
authCacheKeeper.updateOnResponse(target, pathPrefix, targetAuthExchange, context);
361+
authCacheKeeper.updateCacheOnResponse(target, pathPrefixSupplier, targetAuthExchange, context);
346362
}
347363
}
348364
if (proxyAuthRequested || proxyMutualAuthRequired) {
349365
proxyNeedsAuth = authenticator.handleResponse(proxy, ChallengeType.PROXY, response,
350366
proxyAuthStrategy, proxyAuthExchange, context);
351367

352368
if (authCacheKeeper != null) {
353-
authCacheKeeper.updateOnResponse(proxy, null, proxyAuthExchange, context);
369+
authCacheKeeper.updateCacheOnResponse(proxy, null, proxyAuthExchange, context);
354370
}
355371
}
356372
return targetNeedsAuth || proxyNeedsAuth;

0 commit comments

Comments
 (0)