Skip to content

Commit

Permalink
url: return valid file: urls fom url.format()
Browse files Browse the repository at this point in the history
`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: #3361
  • Loading branch information
Trott committed Jun 9, 2016
1 parent cbbdc29 commit 4e8c661
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 22 deletions.
17 changes: 9 additions & 8 deletions lib/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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');
Expand Down
49 changes: 35 additions & 14 deletions test/parallel/test-url.js
Original file line number Diff line number Diff line change
@@ -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 }
Expand Down Expand Up @@ -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]);
Expand Down Expand Up @@ -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);
}

/*
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 4e8c661

Please sign in to comment.