From 8cefb8bed4ac82df8640682517372a9249732352 Mon Sep 17 00:00:00 2001 From: Benjamin Lee Date: Wed, 2 Mar 2022 07:36:27 -0800 Subject: [PATCH] Avoid merging URLs in HttpUtils `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 https://github.com/bazelbuild/bazel/issues/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 --- .../lib/bazel/repository/downloader/HttpUtils.java | 1 - .../lib/bazel/repository/downloader/HttpUtilsTest.java | 10 ++++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpUtils.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpUtils.java index 5853ef3c281c8d..4d7c8ba3be2e67 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpUtils.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpUtils.java @@ -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 diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpUtilsTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpUtilsTest.java index aeac4d978f106b..a93bb344fc8949 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpUtilsTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpUtilsTest.java @@ -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()); + } }