Bug 1890346 - Make sure net_CoalesceDirs doesn't overwrite initial / r=necko-reviewers,kershaw

Before bug 1887614 landed this bug was impossible to trigger because
urlPtr would always advance by the time we got to copying #?.
However, once we allowed calling net_CoalesceDirs for a dirLength > 0,
urlPtr will stay on the first character, while fwdPtr is on ?.

That meant we incorrectly coalesced "/..?" to "?".

Differential Revision: https://phabricator.services.mozilla.com/D206986

netwerk/base/nsURLHelper.cpp

@@ -219,6 +219,8 @@ void net_CoalesceDirs(netCoalesceFlags flags, char* path) {
   uint32_t traversal = 0;
   uint32_t special_ftp_len = 0;
 
+  MOZ_ASSERT(*path == '/', "We expect the path to begin with /");
+
   /* Remember if this url is a special ftp one: */
   if (flags & NET_COALESCE_DOUBLE_SLASH_IS_ROOT) {
     /* some schemes (for example ftp) have the speciality that
@@ -346,6 +348,12 @@ void net_CoalesceDirs(netCoalesceFlags flags, char* path) {
     urlPtr--;
   }
 
+  // Before we start copying past ?#, we must make sure we don't overwrite
+  // the first / character.
+  if (urlPtr == path) {
+    urlPtr++;
+  }
+
   // Copy remaining stuff past the #?;
   for (; *fwdPtr != '\0'; ++fwdPtr) {
     *urlPtr++ = *fwdPtr;

netwerk/test/gtest/TestStandardURL.cpp

@@ -443,7 +443,27 @@ TEST(TestStandardURL, ParseIPv4Num)
 
 TEST(TestStandardURL, CoalescePath)
 {
-  char buf[100] = "/.";
+  auto testCoalescing = [](const char* input, const char* expected) {
-  net_CoalesceDirs(NET_COALESCE_NORMAL, buf);
+    nsAutoCString buf(input);
-  ASSERT_EQ(nsCString(buf), "/"_ns);
+    net_CoalesceDirs(NET_COALESCE_NORMAL, buf.BeginWriting());
+    ASSERT_EQ(nsCString(buf.get()), nsCString(expected));
+  };
+
+  testCoalescing("/.", "/");
+  testCoalescing("/..", "/");
+  testCoalescing("/foo/foo1/.", "/foo/foo1/");
+  testCoalescing("/foo/../foo1", "/foo1");
+  testCoalescing("/foo/./foo1", "/foo/foo1");
+  testCoalescing("/foo/foo1/..", "/foo/");
+
+  // Bug 1890346
+  testCoalescing("/..?/..", "/?/..");
+
+  testCoalescing("/.?/..", "/?/..");
+  testCoalescing("/./../?", "/?");
+  testCoalescing("/.abc", "/.abc");
+  testCoalescing("//", "//");
+  testCoalescing("/../", "/");
+  testCoalescing("/./", "/");
+  testCoalescing("/.../", "/.../");
 }

netwerk/test/unit/test_standardurl.js

@@ -1056,3 +1056,8 @@ add_task(async function test_bug1873976() {
   let url = Services.io.newURI("file:.");
   equal(url.spec, "file:///");
 });
+
+add_task(async function test_bug1890346() {
+  let url = Services.io.newURI("file:..?/..");
+  equal(url.spec, "file:///?/..");
+});