Avoid merging URLs in HttpUtils (bazelbuild#14954)

brentleyjones · Bencodes · web-flow · commit 698da7e563b7 · 2022-03-04T15:34:03.000+01:00
`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 bazelbuild#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 bazelbuild#14922. PiperOrigin-RevId: 431935885 (cherry picked from commit 8cefb8b) Co-authored-by: Benjamin Lee <ben@ben.cm>
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
@@ -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
@@ -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());
+  }
 }
