url: introduce URL_FLAGS_IS_DEFAULT_SCHEME_PORT flag

Introduce `URL_FLAGS_IS_DEFAULT_SCHEME_PORT` flag which is retured when the parser detects that the port passed is the default port for that scheme. PR-URL: #20479 Fixes: #20465 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
nodejs · May 22, 2018 · 526163c · 526163c
1 parent ed5f253
commit 526163c
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 3 deletions.
diff --git a/lib/internal/url.js b/lib/internal/url.js
@@ -46,6 +46,7 @@ const {
   URL_FLAGS_HAS_PATH,
   URL_FLAGS_HAS_QUERY,
   URL_FLAGS_HAS_USERNAME,
+  URL_FLAGS_IS_DEFAULT_SCHEME_PORT,
   URL_FLAGS_SPECIAL,
   kFragment,
   kHost,
@@ -276,7 +277,7 @@ function onParsePortComplete(flags, protocol, username, password,
 function onParseHostComplete(flags, protocol, username, password,
                              host, port, path, query, fragment) {
   onParseHostnameComplete.apply(this, arguments);
-  if (port !== null)
+  if (port !== null || ((flags & URL_FLAGS_IS_DEFAULT_SCHEME_PORT) !== 0))
     onParsePortComplete.apply(this, arguments);
 }
 

diff --git a/src/node_url.cc b/src/node_url.cc
@@ -1749,6 +1749,8 @@ void URL::Parse(const char* input,
             }
             // the port is valid
             url->port = NormalizePort(url->scheme, static_cast<int>(port));
+            if (url->port == -1)
+              url->flags |= URL_FLAGS_IS_DEFAULT_SCHEME_PORT;
             buffer.clear();
           } else if (has_state_override) {
             // TODO(TimothyGu): Similar case as above.

diff --git a/src/node_url.h b/src/node_url.h
@@ -50,7 +50,8 @@ using v8::Value;
   XX(URL_FLAGS_HAS_HOST, 0x80)                                                \
   XX(URL_FLAGS_HAS_PATH, 0x100)                                               \
   XX(URL_FLAGS_HAS_QUERY, 0x200)                                              \
-  XX(URL_FLAGS_HAS_FRAGMENT, 0x400)
+  XX(URL_FLAGS_HAS_FRAGMENT, 0x400)                                           \
+  XX(URL_FLAGS_IS_DEFAULT_SCHEME_PORT, 0x800)                                 \
 
 enum url_parse_state {
   kUnknownState = -1,

diff --git a/test/fixtures/url-setter-tests.js b/test/fixtures/url-setter-tests.js
@@ -2,7 +2,7 @@
 
 /* The following tests are copied from WPT. Modifications to them should be
    upstreamed first. Refs:
-   https://github.com/w3c/web-platform-tests/blob/ed4bb727ed/url/setters_tests.json
+   https://github.com/w3c/web-platform-tests/blob/f0fe479/url/setters_tests.json
    License: http://www.w3.org/Consortium/Legal/2008/04-testsuite-copyright.html
 */
 module.exports =
@@ -727,6 +727,17 @@ module.exports =
                 "port": "80"
             }
         },
+        {
+            "comment": "Port number is removed if new port is scheme default and existing URL has a non-default port",
+            "href": "http://example.net:8080",
+            "new_value": "example.com:80",
+            "expected": {
+                "href": "http://example.com/",
+                "host": "example.com",
+                "hostname": "example.com",
+                "port": ""
+            }
+        },
         {
             "comment": "Stuff after a / delimiter is ignored",
             "href": "http://example.net/path",