Skip to content

Commit 50f08ca

Browse files
committed
Address review feedback
1 parent b30c685 commit 50f08ca

3 files changed

Lines changed: 33 additions & 30 deletions

File tree

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

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -115,33 +115,33 @@ 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(ErrorMessages.create("httpTooManyRedirects", MAX_HTTP_REDIRECTS));
123+
}
124+
var location =
125+
response
126+
.headers()
127+
.firstValue("Location");
128+
if (location.isEmpty()) {
129+
throw new HttpClientException(ErrorMessages.create("httpRedirectNoLocation", currentRequestUri));
130+
}
131+
URI redirectUri;
132+
try {
133+
redirectUri = currentRequestUri.resolve(location.get());
134+
} catch (IllegalArgumentException e) {
135+
throw new HttpClientException(ErrorMessages.create("httpRedirectInvalidUri", currentRequestUri, location.get()));
136+
}
137+
if (currentRequestUri.getScheme().equals("https")
138+
&& redirectUri.getScheme().equals("http")) {
139+
throw new HttpClientException(
140+
ErrorMessages.create("httpRedirectCannotDowngrade", currentRequestUri, redirectUri));
141+
}
142+
currentRequestUri = rewriteUri(redirectUri);
143+
currentRequest = rewriteRequest(request, currentRequestUri);
144+
redirectCount++;
145145
}
146146
}
147147

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)