url: file URL path normalization

Refs: whatwg/url#544 Refs: web-platform-tests/wpt#25716 PR-URL: #35477 Fixes: #35429 Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
nodejs · Oct 12, 2020 · bb62f4a · bb62f4a
1 parent a844387
commit bb62f4a
Show file tree

Hide file tree

Showing 8 changed files with 214 additions and 101 deletions.
diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js
@@ -362,7 +362,7 @@ function initializePolicy() {
     // no bare specifiers for now
     let manifestURL;
     if (require('path').isAbsolute(experimentalPolicy)) {
-      manifestURL = new URL(`file:///${experimentalPolicy}`);
+      manifestURL = new URL(experimentalPolicy, 'file://');
     } else {
       const cwdURL = pathToFileURL(process.cwd());
       cwdURL.pathname += '/';

diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc
@@ -640,7 +640,7 @@ class NodeInspectorClient : public V8InspectorClient {
     node::url::URL url = node::url::URL::FromFilePath(resource_name);
     // TODO(ak239spb): replace this code with url.href().
     // Refs: https://github.com/nodejs/node/issues/22610
-    return Utf8ToStringView(url.protocol() + "//" + url.path());
+    return Utf8ToStringView(url.protocol() + "/" + url.path());
   }
 
   node::Environment* env_;

diff --git a/src/node_url.cc b/src/node_url.cc
@@ -1904,16 +1904,19 @@ void URL::Parse(const char* input,
               state = kFragment;
               break;
             default:
+              url->query.clear();
+              if (base->flags & URL_FLAGS_HAS_HOST) {
+                url->flags |= URL_FLAGS_HAS_HOST;
+                url->host = base->host;
+              }
+              if (base->flags & URL_FLAGS_HAS_PATH) {
+                url->flags |= URL_FLAGS_HAS_PATH;
+                url->path = base->path;
+              }
               if (!StartsWithWindowsDriveLetter(p, end)) {
-                if (base->flags & URL_FLAGS_HAS_HOST) {
-                  url->flags |= URL_FLAGS_HAS_HOST;
-                  url->host = base->host;
-                }
-                if (base->flags & URL_FLAGS_HAS_PATH) {
-                  url->flags |= URL_FLAGS_HAS_PATH;
-                  url->path = base->path;
-                }
                 ShortenUrlPath(url);
+              } else {
+                url->path.clear();
               }
               state = kPath;
               continue;
@@ -1927,20 +1930,13 @@ void URL::Parse(const char* input,
         if (ch == '/' || ch == '\\') {
           state = kFileHost;
         } else {
-          if (has_base &&
-              base->scheme == "file:" &&
-              !StartsWithWindowsDriveLetter(p, end)) {
-            if (IsNormalizedWindowsDriveLetter(base->path[0])) {
+          if (has_base && base->scheme == "file:") {
+            url->flags |= URL_FLAGS_HAS_HOST;
+            url->host = base->host;
+            if (!StartsWithWindowsDriveLetter(p, end) &&
+                IsNormalizedWindowsDriveLetter(base->path[0])) {
               url->flags |= URL_FLAGS_HAS_PATH;
               url->path.push_back(base->path[0]);
-            } else {
-              if (base->flags & URL_FLAGS_HAS_HOST) {
-                url->flags |= URL_FLAGS_HAS_HOST;
-                url->host = base->host;
-              } else {
-                url->flags &= ~URL_FLAGS_HAS_HOST;
-                url->host.clear();
-              }
             }
           }
           state = kPath;
@@ -2024,29 +2020,19 @@ void URL::Parse(const char* input,
                 url->path.empty() &&
                 buffer.size() == 2 &&
                 IsWindowsDriveLetter(buffer)) {
-              if ((url->flags & URL_FLAGS_HAS_HOST) &&
-                  !url->host.empty()) {
-                url->host.clear();
-                url->flags |= URL_FLAGS_HAS_HOST;
-              }
               buffer[1] = ':';
             }
             url->flags |= URL_FLAGS_HAS_PATH;
             url->path.emplace_back(std::move(buffer));
           }
           buffer.clear();
-          if (url->scheme == "file:" &&
-              (ch == kEOL ||
-               ch == '?' ||
-               ch == '#')) {
-            while (url->path.size() > 1 && url->path[0].empty()) {
-              url->path.erase(url->path.begin());
-            }
-          }
           if (ch == '?') {
             url->flags |= URL_FLAGS_HAS_QUERY;
+            url->query.clear();
             state = kQuery;
           } else if (ch == '#') {
+            url->flags |= URL_FLAGS_HAS_FRAGMENT;
+            url->fragment.clear();
             state = kFragment;
           }
         } else {

diff --git a/test/cctest/test_url.cc b/test/cctest/test_url.cc
@@ -148,14 +148,14 @@ TEST_F(URLTest, FromFilePath) {
 #else
   file_url = URL::FromFilePath("/");
   EXPECT_EQ("file:", file_url.protocol());
-  EXPECT_EQ("/", file_url.path());
+  EXPECT_EQ("//", file_url.path());
 
   file_url = URL::FromFilePath("/a/b/c");
   EXPECT_EQ("file:", file_url.protocol());
-  EXPECT_EQ("/a/b/c", file_url.path());
+  EXPECT_EQ("//a/b/c", file_url.path());
 
   file_url = URL::FromFilePath("/a/%%.js");
   EXPECT_EQ("file:", file_url.protocol());
-  EXPECT_EQ("/a/%25%25.js", file_url.path());
+  EXPECT_EQ("//a/%25%25.js", file_url.path());
 #endif
 }
diff --git a/test/fixtures/wpt/README.md b/test/fixtures/wpt/README.md
@@ -12,7 +12,7 @@ Last update:
 
 - console: https://github.com/web-platform-tests/wpt/tree/3b1f72e99a/console
 - encoding: https://github.com/web-platform-tests/wpt/tree/d7f9e16c9a/encoding
-- url: https://github.com/web-platform-tests/wpt/tree/e2ddf48b78/url
+- url: https://github.com/web-platform-tests/wpt/tree/050308a616/url
 - resources: https://github.com/web-platform-tests/wpt/tree/1d14e821b9/resources
 - interfaces: https://github.com/web-platform-tests/wpt/tree/15e47f779c/interfaces
 - html/webappapis/microtask-queuing: https://github.com/web-platform-tests/wpt/tree/2c5c3c4c27/html/webappapis/microtask-queuing

diff --git a/test/fixtures/wpt/url/resources/setters_tests.json b/test/fixtures/wpt/url/resources/setters_tests.json
@@ -1672,26 +1672,26 @@
             "href": "file://monkey/",
             "new_value": "\\\\",
             "expected": {
-                "href": "file://monkey/",
-                "pathname": "/"
+                "href": "file://monkey//",
+                "pathname": "//"
             }
         },
         {
             "comment": "File URLs and (back)slashes",
             "href": "file:///unicorn",
             "new_value": "//\\/",
             "expected": {
-                "href": "file:///",
-                "pathname": "/"
+                "href": "file://////",
+                "pathname": "////"
             }
         },
         {
             "comment": "File URLs and (back)slashes",
             "href": "file:///unicorn",
             "new_value": "//monkey/..//",
             "expected": {
-                "href": "file:///",
-                "pathname": "/"
+                "href": "file://///",
+                "pathname": "///"
             }
         },
         {