url: return valid file: urls fom url.format()

`file:` URLs that do not start with `file://` are invalid. Browsers convert `file:/etc/passwd` to `file:///etc/passwd`. This is also what the docs indicate we are doing, but we're not. PR-URL: #7234 Fixes: #3361 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Brian White <mscdex@mscdex.net>
nodejs · Jun 16, 2016 · 336b027 · 336b027
1 parent 874839c
commit 336b027
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 22 deletions.
diff --git a/lib/url.js b/lib/url.js
@@ -553,7 +553,7 @@ Url.prototype.format = function() {
   var protocol = this.protocol || '';
   var pathname = this.pathname || '';
   var hash = this.hash || '';
-  var host = false;
+  var host = '';
   var query = '';
 
   if (this.host) {
@@ -602,13 +602,18 @@ Url.prototype.format = function() {
 
   // only the slashedProtocols get the //.  Not mailto:, xmpp:, etc.
   // unless they had them to begin with.
-  if (this.slashes ||
-      (!protocol || slashedProtocol[protocol]) && host !== false) {
-    host = '//' + (host || '');
-    if (pathname && pathname.charCodeAt(0) !== 47/*/*/)
-      pathname = '/' + pathname;
-  } else if (!host) {
-    host = '';
+  if (this.slashes || slashedProtocol[protocol]) {
+    if (this.slashes || host) {
+      if (pathname && pathname.charCodeAt(0) !== 47/*/*/)
+        pathname = '/' + pathname;
+      host = '//' + host;
+    } else if (protocol.length >= 4 &&
+               protocol.charCodeAt(0) === 102/*f*/ &&
+               protocol.charCodeAt(1) === 105/*i*/ &&
+               protocol.charCodeAt(2) === 108/*l*/ &&
+               protocol.charCodeAt(3) === 101/*e*/) {
+      host = '//';
+    }
   }
 
   search = search.replace('#', '%23');

diff --git a/test/parallel/test-url.js b/test/parallel/test-url.js
@@ -1,9 +1,10 @@
 /* eslint-disable max-len */
 'use strict';
 require('../common');
-var assert = require('assert');
+const assert = require('assert');
+const inspect = require('util').inspect;
 
-var url = require('url');
+const url = require('url');
 
 // URLs to parse, and expected data
 // { url : parsed }
@@ -881,8 +882,16 @@ for (const u in parseTests) {
     }
   });
 
-  assert.deepStrictEqual(actual, expected);
-  assert.deepStrictEqual(spaced, expected);
+  assert.deepStrictEqual(
+    actual,
+    expected,
+    `expected ${inspect(expected)}, got ${inspect(actual)}`
+  );
+  assert.deepStrictEqual(
+    spaced,
+    expected,
+    `expected ${inspect(expected)}, got ${inspect(spaced)}`
+  );
 
   expected = parseTests[u].href;
   actual = url.format(parseTests[u]);
@@ -1165,20 +1174,28 @@ var formatTests = {
     hash: '#frag',
     search: '?abc=the#1?&foo=bar',
     pathname: '/fooA100%mBr',
+  },
+
+  // https://github.com/nodejs/node/issues/3361
+  'file:///home/user': {
+    href: 'file:///home/user',
+    protocol: 'file',
+    pathname: '/home/user',
+    path: '/home/user'
   }
 };
 for (const u in formatTests) {
   const expect = formatTests[u].href;
   delete formatTests[u].href;
   const actual = url.format(u);
   const actualObj = url.format(formatTests[u]);
-  assert.equal(actual, expect,
-               'wonky format(' + u + ') == ' + expect +
-               '\nactual:' + actual);
-  assert.equal(actualObj, expect,
-               'wonky format(' + JSON.stringify(formatTests[u]) +
-               ') == ' + expect +
-               '\nactual: ' + actualObj);
+  assert.strictEqual(actual, expect,
+                     'wonky format(' + u + ') == ' + expect +
+                     '\nactual:' + actual);
+  assert.strictEqual(actualObj, expect,
+                     'wonky format(' + JSON.stringify(formatTests[u]) +
+                     ') == ' + expect +
+                     '\nactual: ' + actualObj);
 }
 
 /*
@@ -1556,7 +1573,7 @@ var relativeTests2 = [
 ];
 relativeTests2.forEach(function(relativeTest) {
   const a = url.resolve(relativeTest[1], relativeTest[0]);
-  const e = relativeTest[2];
+  const e = url.format(relativeTest[2]);
   assert.equal(a, e,
                'resolve(' + [relativeTest[1], relativeTest[0]] + ') == ' + e +
                '\n  actual=' + a);
@@ -1598,9 +1615,13 @@ relativeTests2.forEach(function(relativeTest) {
   var actual = url.resolveObject(url.parse(relativeTest[1]), relativeTest[0]);
   var expected = url.parse(relativeTest[2]);
 
-  assert.deepStrictEqual(actual, expected);
+  assert.deepStrictEqual(
+    actual,
+    expected,
+    `expected ${inspect(expected)} but got ${inspect(actual)}`
+  );
 
-  expected = relativeTest[2];
+  expected = url.format(relativeTest[2]);
   actual = url.format(actual);
 
   assert.equal(actual, expected,