Skip to content

compare scheme in redirect same-authority check #848

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ok2c merged 1 commit into from
Jul 16, 2026
+46 −1
Merged

compare scheme in redirect same-authority check #848

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultRedirectStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public boolean isRedirectAllowed(
final HttpRequest redirect,
final HttpContext context) {

// If authority (host + effective port) differs, disallow automatic redirect
// If the origin (scheme, host and effective port) differs, disallow automatic redirect
if (!isSameAuthority(currentTarget, newTarget)) {
for (final Iterator<Header> it = redirect.headerIterator(); it.hasNext(); ) {
final Header header = it.next();
Expand All @@ -111,6 +111,9 @@ private boolean isSameAuthority(final HttpHost h1, final HttpHost h2) {
if (h1 == null || h2 == null) {
return false;
}
if (!h1.getSchemeName().equalsIgnoreCase(h2.getSchemeName())) {

@arturobernalg arturobernalg Jul 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A scheme change makes the origins different, but does not by itself require rejecting the redirect.

@dxbjavid dxbjavid Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, and that's the intent here. the scheme mismatch only marks the origins as different, it doesn't reject the redirect on its own. an https to http hop still goes through unless there's an authorization or cookie header to protect, exactly as it already behaves for a differing host or port. the last assertion in the added test covers the plain https to http redirect staying allowed.

@arturobernalg arturobernalg Jul 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. Still, this changes the boundary from authority to origin and extends the existing rejection policy to scheme changes. That needs a clear rationale

@ok2c ok2c Jul 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dxbjavid Why should scheme even matter, given the strategy takes the port into account? Can there realistically be multiple protocol schemes on the same port?

@dxbjavid dxbjavid Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in practice you're right that a port is normally bound to a single listener, so the same host and port serving both schemes at once is unusual. the concern isn't that the server offers both though, it's that the redirect names the next hop as cleartext: on https://host:8443 to http://host:8443 the client opens a plaintext connection and writes the authorization and cookie headers before any tls, so credentials tied to the secure origin go out in the clear. host and port alone can't tell us that, the scheme can. that's also the rationale for the origin framing (scheme, host, port per rfc 6454) rather than authority, since forwarding those headers is really an origin decision. it doesn't reject the redirect, only strips the sensitive headers, the same as a differing host or port already does.

return false;
}
final String host1 = h1.getHostName();
final String host2 = h2.getHostName();
if (!host1.equalsIgnoreCase(host2)) {
Expand Down
42 changes: 42 additions & 0 deletions httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultRedirectStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,48 @@ void testRedirectAllowed() throws Exception {



@Test
void testRedirectAllowedCrossSchemeSamePort() {
final DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();

// https -> http on the same host and same explicit port is a different origin,
// so a redirect carrying credential-bearing headers is not followed
final HttpHost secure = new HttpHost("https", "example.com", 8443);
final HttpHost insecure = new HttpHost("http", "example.com", 8443);

Assertions.assertFalse(redirectStrategy.isRedirectAllowed(
secure,
insecure,
BasicRequestBuilder.get("/")
.addHeader(HttpHeaders.AUTHORIZATION, "let me pass")
.build(),
null));

Assertions.assertFalse(redirectStrategy.isRedirectAllowed(
secure,
insecure,
BasicRequestBuilder.get("/")
.addHeader(HttpHeaders.COOKIE, "stuff=blah")
.build(),
null));

// no sensitive headers -> the redirect itself is still permitted
Assertions.assertTrue(redirectStrategy.isRedirectAllowed(
secure,
insecure,
BasicRequestBuilder.get("/").build(),
null));

// same scheme, host and port stays same origin
Assertions.assertTrue(redirectStrategy.isRedirectAllowed(
secure,
new HttpHost("https", "example.com", 8443),
BasicRequestBuilder.get("/")
.addHeader(HttpHeaders.AUTHORIZATION, "let me pass")
.build(),
null));
}

@Test
void testRedirectAllowedDefaultPortNormalization() {
final DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
Expand Down
Loading