Skip to content

Commit 43cb5c3

Browse files
dxbjavidok2c
authored andcommitted
compare scheme in redirect same-authority check
1 parent 7978d71 commit 43cb5c3

2 files changed

Lines changed: 46 additions & 1 deletion

File tree

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public boolean isRedirectAllowed(
9393
final HttpRequest redirect,
9494
final HttpContext context) {
9595

96-
// If authority (host + effective port) differs, disallow automatic redirect
96+
// If the origin (scheme, host and effective port) differs, disallow automatic redirect
9797
if (!isSameAuthority(currentTarget, newTarget)) {
9898
for (final Iterator<Header> it = redirect.headerIterator(); it.hasNext(); ) {
9999
final Header header = it.next();
@@ -111,6 +111,9 @@ private boolean isSameAuthority(final HttpHost h1, final HttpHost h2) {
111111
if (h1 == null || h2 == null) {
112112
return false;
113113
}
114+
if (!h1.getSchemeName().equalsIgnoreCase(h2.getSchemeName())) {
115+
return false;
116+
}
114117
final String host1 = h1.getHostName();
115118
final String host2 = h2.getHostName();
116119
if (!host1.equalsIgnoreCase(host2)) {

httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultRedirectStrategy.java

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,48 @@ void testRedirectAllowed() throws Exception {
320320

321321

322322

323+
@Test
324+
void testRedirectAllowedCrossSchemeSamePort() {
325+
final DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
326+
327+
// https -> http on the same host and same explicit port is a different origin,
328+
// so a redirect carrying credential-bearing headers is not followed
329+
final HttpHost secure = new HttpHost("https", "example.com", 8443);
330+
final HttpHost insecure = new HttpHost("http", "example.com", 8443);
331+
332+
Assertions.assertFalse(redirectStrategy.isRedirectAllowed(
333+
secure,
334+
insecure,
335+
BasicRequestBuilder.get("/")
336+
.addHeader(HttpHeaders.AUTHORIZATION, "let me pass")
337+
.build(),
338+
null));
339+
340+
Assertions.assertFalse(redirectStrategy.isRedirectAllowed(
341+
secure,
342+
insecure,
343+
BasicRequestBuilder.get("/")
344+
.addHeader(HttpHeaders.COOKIE, "stuff=blah")
345+
.build(),
346+
null));
347+
348+
// no sensitive headers -> the redirect itself is still permitted
349+
Assertions.assertTrue(redirectStrategy.isRedirectAllowed(
350+
secure,
351+
insecure,
352+
BasicRequestBuilder.get("/").build(),
353+
null));
354+
355+
// same scheme, host and port stays same origin
356+
Assertions.assertTrue(redirectStrategy.isRedirectAllowed(
357+
secure,
358+
new HttpHost("https", "example.com", 8443),
359+
BasicRequestBuilder.get("/")
360+
.addHeader(HttpHeaders.AUTHORIZATION, "let me pass")
361+
.build(),
362+
null));
363+
}
364+
323365
@Test
324366
void testRedirectAllowedDefaultPortNormalization() {
325367
final DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();

0 commit comments

Comments
 (0)