From 4e8c6613e43c47212964af6e7e4ebf73bc2f716d Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Tue, 7 Jun 2016 22:49:56 -0700 Subject: [PATCH 1/3] 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. Fixes: https://github.com/nodejs/node/issues/3361 --- lib/url.js | 17 +++++++------- test/parallel/test-url.js | 49 ++++++++++++++++++++++++++++----------- 2 files changed, 44 insertions(+), 22 deletions(-) diff --git a/lib/url.js b/lib/url.js index c4d6ed2e33a5f5..691fe1567a404d 100644 --- 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,14 @@ 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.startsWith('file')) { + host = '//'; + } } search = search.replace('#', '%23'); diff --git a/test/parallel/test-url.js b/test/parallel/test-url.js index c5933b9c677ba4..046dadf1b96521 100644 --- 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,6 +1174,14 @@ 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) { @@ -1172,13 +1189,13 @@ for (const u in formatTests) { 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, From 99f87da20a507225d0fca059acedfcf0cdc94ace Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Thu, 9 Jun 2016 12:11:59 -0700 Subject: [PATCH 2/3] squash: use string concatenation instead of template --- lib/url.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/url.js b/lib/url.js index 691fe1567a404d..bdc55011413e81 100644 --- a/lib/url.js +++ b/lib/url.js @@ -606,7 +606,7 @@ Url.prototype.format = function() { if (this.slashes || host) { if (pathname && pathname.charCodeAt(0) !== 47/*/*/) pathname = '/' + pathname; - host = `//${host}`; + host = '//' + host; } else if (protocol.startsWith('file')) { host = '//'; } From 4c9f18c811663fd3dfa8324e2dff15abab205e4e Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Thu, 9 Jun 2016 12:19:28 -0700 Subject: [PATCH 3/3] squash: startsWith() -> charCodeAt() --- lib/url.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/url.js b/lib/url.js index bdc55011413e81..f072b33c86975c 100644 --- a/lib/url.js +++ b/lib/url.js @@ -607,7 +607,11 @@ Url.prototype.format = function() { if (pathname && pathname.charCodeAt(0) !== 47/*/*/) pathname = '/' + pathname; host = '//' + host; - } else if (protocol.startsWith('file')) { + } 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 = '//'; } }