Skip to content

Commit 2b1100b

Browse files
committed
Address review feedback
1 parent b30c685 commit 2b1100b

3 files changed

Lines changed: 32 additions & 30 deletions

File tree

pkl-core/src/main/java/org/pkl/core/http/RequestRewritingClient.java

Lines changed: 24 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -115,33 +115,32 @@ private <T> HttpResponse<T> doSend(
115115
while (true) {
116116
httpRequestChecker.check(currentRequestUri);
117117
var response = delegate.send(currentRequest, responseBodyHandler, httpRequestChecker);
118-
if (HttpUtils.isRedirectStatusCode(response.statusCode())) {
119-
if (redirectCount >= MAX_HTTP_REDIRECTS) {
120-
throw new HttpClientException(ErrorMessages.create("httpTooManyRedirects"));
121-
}
122-
var location =
123-
response
124-
.headers()
125-
.firstValue("Location")
126-
.orElseThrow(
127-
() -> new HttpClientException(ErrorMessages.create("httpRedirectNoLocation")));
128-
URI redirectUri;
129-
try {
130-
redirectUri = currentRequestUri.resolve(location);
131-
} catch (IllegalArgumentException e) {
132-
throw new HttpClientException(ErrorMessages.create("httpRedirectInvalidUri", location));
133-
}
134-
if (currentRequestUri.getScheme().equals("https")
135-
&& redirectUri.getScheme().equals("http")) {
136-
throw new HttpClientException(
137-
ErrorMessages.create("httpRedirectCannotDowngrade", currentRequestUri, redirectUri));
138-
}
139-
currentRequestUri = rewriteUri(redirectUri);
140-
currentRequest = rewriteRequest(request, currentRequestUri);
141-
redirectCount++;
142-
} else {
118+
if (!HttpUtils.isRedirectStatusCode(response.statusCode())) {
143119
return response;
144120
}
121+
if (redirectCount >= MAX_HTTP_REDIRECTS) {
122+
throw new HttpClientException(
123+
ErrorMessages.create("httpTooManyRedirects", MAX_HTTP_REDIRECTS));
124+
}
125+
var location = response.headers().firstValue("Location");
126+
if (location.isEmpty()) {
127+
throw new HttpClientException(
128+
ErrorMessages.create("httpRedirectNoLocation", currentRequestUri));
129+
}
130+
URI redirectUri;
131+
try {
132+
redirectUri = currentRequestUri.resolve(location.get());
133+
} catch (IllegalArgumentException e) {
134+
throw new HttpClientException(
135+
ErrorMessages.create("httpRedirectInvalidUri", currentRequestUri, location.get()));
136+
}
137+
if (currentRequestUri.getScheme().equals("https") && redirectUri.getScheme().equals("http")) {
138+
throw new HttpClientException(
139+
ErrorMessages.create("httpRedirectCannotDowngrade", currentRequestUri, redirectUri));
140+
}
141+
currentRequestUri = rewriteUri(redirectUri);
142+
currentRequest = rewriteRequest(request, currentRequestUri);
143+
redirectCount++;
145144
}
146145
}
147146

pkl-core/src/main/resources/org/pkl/core/errorMessages.properties

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1150,18 +1150,21 @@ HTTP Header value is invalid because it is longer than 4096 characters. \
11501150
Value: `{0}`
11511151

11521152
httpTooManyRedirects=\
1153-
Too many redirects. Possible redirect loop?
1153+
Too many redirects, exceeded the redirect threshold ({0}).
11541154

11551155
httpRedirectNoLocation=\
1156-
Cannot follow HTTP redirect because no 'Location' header was found
1156+
Cannot follow HTTP redirect because no 'Location' header was found.\n\
1157+
\n\
1158+
HTTP Request: `GET {0}`
11571159

11581160
httpRedirectInvalidUri=\
11591161
Cannot follow HTTP redirect because the response 'Location' header has a malformed URI.\n\
11601162
\n\
1161-
Location header: `{0}`
1163+
HTTP Request: `GET {0}`\n\
1164+
Location header: `{1}`
11621165

11631166
httpRedirectCannotDowngrade=\
11641167
Cannot follow redirect from ''https:'' URL to ''http:'' URL.\
11651168
\n\
1166-
Original Request URL: `{0}`\n\
1169+
HTTP Request: `GET {0}`\n\
11671170
Redirected to: `{1}`

stdlib/EvaluatorSettings.pkl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ class Http {
149149
/// In the following example, an original request for `https://pkg.pkl-lang.org/my/pkg@1.0.0` is
150150
/// replaced with `https://my.internal.mirror/my/pkg@1.0.0`.
151151
///
152-
/// This does not affect `3XX` status code redirect following.
152+
/// This also affects `3XX` status code redirect following.
153153
///
154154
/// Example:
155155
///

0 commit comments

Comments
 (0)