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 · 8233c34 · 8233c34
1 parent 9516aa1
commit 8233c34
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 3 deletions.
diff --git a/src/node_url.cc b/src/node_url.cc
@@ -829,6 +829,16 @@ static url_host_type ParseOpaqueHost(url_host* host,
   return type;
 }
 
+static inline bool IsAllASCII(std::string* input) {
+  for (size_t n = 0; n < input->size(); n++) {
+    const char ch = (*input)[n];
+    if (ch & 0x80) {
+      return false;
+    }
+  }
+  return true;
+}
+
 static url_host_type ParseHost(url_host* host,
                                const char* input,
                                size_t length,
@@ -853,9 +863,18 @@ static url_host_type ParseHost(url_host* host,
   // First, we have to percent decode
   PercentDecode(input, length, &decoded);
 
-  // Then we have to punycode toASCII
-  if (!ToASCII(&decoded, &decoded))
-    goto end;
+  // Match browser behavior for ASCII only domains
+  // and do not run them through ToASCII algorithm.
+  if (IsAllASCII(&decoded)) {
+    // Lowercase aschii domains
+    for (size_t n = 0; n < decoded.size(); n++) {
+      decoded[n] = std::tolower(decoded[n]);
+    }
+  } else {
+    // Then we have to punycode toASCII
+    if (!ToASCII(&decoded, &decoded))
+      goto end;
+  }
 
   // If any of the following characters are still present, we have to fail
   for (size_t n = 0; n < decoded.size(); n++) {

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(
@@ -34,6 +35,13 @@ const tests = require('../fixtures/url-idna.js');
   }
 }
 
+{
+  for (const [i, { ascii, unicode }] of testsHyphenDomains.valid.entries()) {
+    assert.strictEqual(ascii, domainToASCII(unicode),
+                       `domainToASCII(${i + 1})`);
+  }
+}
+
 {
   const convertFunc = {
     ascii: domainToASCII,