url: fast path ascii domains, do not run ToASCII

To match browser behavior fast path ascii only domains and do not run ToASCII on them. Fixes: nodejs#12965 Refs: nodejs#12966 Refs: whatwg/url#309
zimbabao · May 15, 2017 · d816988 · d816988
1 parent a13c377
commit d816988
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 11 deletions.
diff --git a/src/node_url.cc b/src/node_url.cc
@@ -133,6 +133,8 @@ enum url_error_cb_args {
 // https://infra.spec.whatwg.org/#ascii-code-point
 CHAR_TEST(8, IsASCIICodePoint, (ch >= '\0' && ch <= '\x7f'))
 
+CHAR_TEST(8, IsLowerCaseASCII, (ch >='a' && ch <= 'z'))
+
 // https://infra.spec.whatwg.org/#ascii-tab-or-newline
 CHAR_TEST(8, IsASCIITabOrNewline, (ch == '\t' || ch == '\n' || ch == '\r'))
 
@@ -871,7 +873,9 @@ static url_host_type ParseHost(url_host* host,
   if (IsAllASCII(decoded)) {
     // Lowercase ASCII domains
     for (size_t n = 0; n < decoded.size(); n++) {
-      decoded[n] = ASCIILowercase(decoded[n]);
+      if (!IsLowerCaseASCII(decoded[n])) {
+        decoded[n] = ASCIILowercase(decoded[n]);
+      }
     }
   } else {
     // Then we have to Unicode IDNA toASCII

diff --git a/test/fixtures/url-domains-with-hyphens.js b/test/fixtures/url-domains-with-hyphens.js
@@ -0,0 +1,27 @@
+'use strict';
+
+module.exports = {
+  valid: [
+    // URLs with hyphen
+    {
+      ascii: 'r4---sn-a5mlrn7s.gevideo.com',
+      unicode: 'r4---sn-a5mlrn7s.gevideo.com'
+    },
+    {
+      ascii: '-sn-a5mlrn7s.gevideo.com',
+      unicode: '-sn-a5mlrn7s.gevideo.com'
+    },
+    {
+      ascii: 'sn-a5mlrn7s-.gevideo.com',
+      unicode: 'sn-a5mlrn7s-.gevideo.com'
+    },
+    {
+      ascii: '-sn-a5mlrn7s-.gevideo.com',
+      unicode: '-sn-a5mlrn7s-.gevideo.com'
+    },
+    {
+      ascii: '-sn--a5mlrn7s-.gevideo.com',
+      unicode: '-sn--a5mlrn7s-.gevideo.com'
+    }
+  ]
+}
diff --git a/test/parallel/test-whatwg-url-domainto.js b/test/parallel/test-whatwg-url-domainto.js
@@ -11,6 +11,7 @@ const { domainToASCII, domainToUnicode } = require('url');
 
 // Tests below are not from WPT.
 const tests = require('../fixtures/url-idna.js');
+const testsHyphenDomains = require('../fixtures/url-domains-with-hyphens.js');
 
 {
   const expectedError = common.expectsError(
@@ -35,16 +36,10 @@ const tests = require('../fixtures/url-idna.js');
 }
 
 {
-  [
-    'r4---sn-a5mlrn7s.gevideo.com',
-    '-sn-a5mlrn7s.gevideo.com',
-    'sn-a5mlrn7s-.gevideo.com',
-    '-sn-a5mlrn7s-.gevideo.com',
-    '-sn--a5mlrn7s-.gevideo.com'
-  ].forEach((domain) => {
-    assert.strictEqual(domain, domainToASCII(domain),
-                       `domainToASCII(${domain})`);
-  })
+  for (const [i, { ascii, unicode }] of testsHyphenDomains.valid.entries()) {
+    assert.strictEqual(ascii, domainToASCII(unicode),
+                       `domainToASCII(${i + 1})`);
+  }
 }
 
 {