luigidemasi commented on code in PR #143:
URL: https://github.com/apache/camel-kit/pull/143#discussion_r4440812813
##########
knowledge/mcp/src/main/java/org/apache/camel/kit/knowledge/mcp/IndexDownloader.java:
##########
@@ -568,9 +603,41 @@ private InputStream openStream(URI uri) throws
IOException, InterruptedException
}
private HttpResponse<InputStream> send(HttpRequest request) throws
IOException, InterruptedException {
+ final int maxRedirects = 5;
long deadline = System.nanoTime() +
request.timeout().orElseThrow().toNanos();
- return httpClient.send(request, info ->
HttpResponse.BodySubscribers.mapping(
- HttpResponse.BodySubscribers.ofPublisher(), publisher -> new
TimedBody(publisher, deadline)));
+ HttpRequest current = request;
+ for (int hop = 0;; hop++) {
+ HttpResponse<InputStream> response = httpClient.send(current, info
-> HttpResponse.BodySubscribers.mapping(
+ HttpResponse.BodySubscribers.ofPublisher(), publisher ->
new TimedBody(publisher, deadline)));
+ int status = response.statusCode();
+ if (status != 301 && status != 302 && status != 303 && status !=
307 && status != 308) {
+ return response;
+ }
+ // Validate the redirect target's transport BEFORE following it; a
304 is not a redirect and
+ // is returned above for the conditional-manifest path.
+ String target =
response.headers().firstValue("Location").orElse(null);
+ response.body().close();
+ if (hop >= maxRedirects) {
+ throw new IOException("Too many index redirects starting from
" + request.uri());
+ }
+ if (target == null) {
+ throw new IOException("Index redirect without a Location
header from " + current.uri());
+ }
+ URI next;
+ try {
+ next = current.uri().resolve(target);
+ } catch (IllegalArgumentException e) {
+ throw new IOException("Invalid index redirect target: " +
target, e);
+ }
+ requireFetchableUri(next);
+ long remaining = deadline - System.nanoTime();
+ if (remaining <= 0) {
+ throw new HttpTimeoutException("Index redirect handling timed
out from " + request.uri());
+ }
+ HttpRequest.Builder builder =
HttpRequest.newBuilder(next).timeout(Duration.ofNanos(remaining)).GET();
+ current.headers().firstValue("Accept").ifPresent(accept ->
builder.header("Accept", accept));
+ current = builder.build();
Review Comment:
[P2] Preserve the manifest validator when following redirects
Rebuilding the request copies only `Accept`, dropping the `If-None-Match`
header added by `fetchManifest()`. A manifest behind even a same-origin 302
therefore downloads again on every resolution instead of returning 304 when
unchanged.
I reproduced this with a local redirect/manifest server: first resolve
returns 200 with `ETag: "v1"`; on the second resolve, the target receives
`If-None-Match: "v1"` and returns 304 at `88430bb6`, but receives no validator
and returns 200 at this head. Forwarding that one header makes the same probe
pass. The installed archive is still reused; the regression is conditional
manifest revalidation.
Please preserve `If-None-Match` across the validated redirect hops and add a
two-resolution regression test that requires the target's 304 response.
_AI-generated by Codex on behalf of @luigidemasi._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
