Skip to content

Commit

Permalink
Avoid merging URLs in HttpUtils (#14954)
Browse files Browse the repository at this point in the history
`mergeUrls` does not need to rebuild the URL from scratch if user information exists on the original URL. This behavior can actually break the 302 redirect due to subtle changes in the URL/encoding and should be avoided when possible.

This fixes #14866 by correcting the implementation of `mergeUrls` to match the documentation that was added instead of rebuilding the URL from scratch which breaks the encoding of signed URLs.

Closes #14922.

PiperOrigin-RevId: 431935885
(cherry picked from commit 8cefb8b)

Co-authored-by: Benjamin Lee <ben@ben.cm>
  • Loading branch information
brentleyjones and Bencodes committed Mar 4, 2022
1 parent a6a4305 commit 698da7e
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 1 deletion.
Expand Up @@ -77,7 +77,6 @@ private static URL mergeUrls(URI preferred, URL original) throws IOException {
// scheme of their redirect URLs.
if (preferred.getHost() != null
&& preferred.getScheme() != null
&& (preferred.getUserInfo() != null || original.getUserInfo() == null)
&& (preferred.getFragment() != null || original.getRef() == null)) {
// In this case we obviously do not inherit anything from the original URL, as all inheritable
// fields are either set explicitly or not present in the original either. Therefore, it is
Expand Down
Expand Up @@ -139,4 +139,14 @@ public void getLocation_preservesQuotingIfNotInheriting() throws Exception {
when(connection.getHeaderField("Location")).thenReturn(redirect);
assertThat(HttpUtils.getLocation(connection)).isEqualTo(URI.create(redirect).toURL());
}

@Test
public void getLocation_preservesQuotingWithUserIfNotInheriting() throws Exception {
String redirect =
"http://redirected.example.org/foo?"
+ "response-content-disposition=attachment%3Bfilename%3D%22bar.tar.gz%22";
when(connection.getURL()).thenReturn(new URL("http://a:b@original.example.org"));
when(connection.getHeaderField("Location")).thenReturn(redirect);
assertThat(HttpUtils.getLocation(connection)).isEqualTo(URI.create(redirect).toURL());
}
}

0 comments on commit 698da7e

Please sign in to comment.