Skip to content

Commit 4e8c661

Browse files
committedJun 9, 2016
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: nodejs#3361
1 parent cbbdc29 commit 4e8c661

File tree

2 files changed

+44
-22
lines changed

2 files changed

+44
-22
lines changed
 

‎lib/url.js

+9-8
Original file line numberDiff line numberDiff line change
@@ -553,7 +553,7 @@ Url.prototype.format = function() {
553553
var protocol = this.protocol || '';
554554
var pathname = this.pathname || '';
555555
var hash = this.hash || '';
556-
var host = false;
556+
var host = '';
557557
var query = '';
558558

559559
if (this.host) {
@@ -602,13 +602,14 @@ Url.prototype.format = function() {
602602

603603
// only the slashedProtocols get the //. Not mailto:, xmpp:, etc.
604604
// unless they had them to begin with.
605-
if (this.slashes ||
606-
(!protocol || slashedProtocol[protocol]) && host !== false) {
607-
host = '//' + (host || '');
608-
if (pathname && pathname.charCodeAt(0) !== 47/*/*/)
609-
pathname = '/' + pathname;
610-
} else if (!host) {
611-
host = '';
605+
if (this.slashes || slashedProtocol[protocol]) {
606+
if (this.slashes || host) {
607+
if (pathname && pathname.charCodeAt(0) !== 47/*/*/)
608+
pathname = '/' + pathname;
609+
host = `//${host}`;
610+
} else if (protocol.startsWith('file')) {
611+
host = '//';
612+
}
612613
}
613614

614615
search = search.replace('#', '%23');

‎test/parallel/test-url.js

+35-14
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
/* eslint-disable max-len */
22
'use strict';
33
require('../common');
4-
var assert = require('assert');
4+
const assert = require('assert');
5+
const inspect = require('util').inspect;
56

6-
var url = require('url');
7+
const url = require('url');
78

89
// URLs to parse, and expected data
910
// { url : parsed }
@@ -881,8 +882,16 @@ for (const u in parseTests) {
881882
}
882883
});
883884

884-
assert.deepStrictEqual(actual, expected);
885-
assert.deepStrictEqual(spaced, expected);
885+
assert.deepStrictEqual(
886+
actual,
887+
expected,
888+
`expected ${inspect(expected)}, got ${inspect(actual)}`
889+
);
890+
assert.deepStrictEqual(
891+
spaced,
892+
expected,
893+
`expected ${inspect(expected)}, got ${inspect(spaced)}`
894+
);
886895

887896
expected = parseTests[u].href;
888897
actual = url.format(parseTests[u]);
@@ -1165,20 +1174,28 @@ var formatTests = {
11651174
hash: '#frag',
11661175
search: '?abc=the#1?&foo=bar',
11671176
pathname: '/fooA100%mBr',
1177+
},
1178+
1179+
// https://github.com/nodejs/node/issues/3361
1180+
'file:///home/user': {
1181+
href: 'file:///home/user',
1182+
protocol: 'file',
1183+
pathname: '/home/user',
1184+
path: '/home/user'
11681185
}
11691186
};
11701187
for (const u in formatTests) {
11711188
const expect = formatTests[u].href;
11721189
delete formatTests[u].href;
11731190
const actual = url.format(u);
11741191
const actualObj = url.format(formatTests[u]);
1175-
assert.equal(actual, expect,
1176-
'wonky format(' + u + ') == ' + expect +
1177-
'\nactual:' + actual);
1178-
assert.equal(actualObj, expect,
1179-
'wonky format(' + JSON.stringify(formatTests[u]) +
1180-
') == ' + expect +
1181-
'\nactual: ' + actualObj);
1192+
assert.strictEqual(actual, expect,
1193+
'wonky format(' + u + ') == ' + expect +
1194+
'\nactual:' + actual);
1195+
assert.strictEqual(actualObj, expect,
1196+
'wonky format(' + JSON.stringify(formatTests[u]) +
1197+
') == ' + expect +
1198+
'\nactual: ' + actualObj);
11821199
}
11831200

11841201
/*
@@ -1556,7 +1573,7 @@ var relativeTests2 = [
15561573
];
15571574
relativeTests2.forEach(function(relativeTest) {
15581575
const a = url.resolve(relativeTest[1], relativeTest[0]);
1559-
const e = relativeTest[2];
1576+
const e = url.format(relativeTest[2]);
15601577
assert.equal(a, e,
15611578
'resolve(' + [relativeTest[1], relativeTest[0]] + ') == ' + e +
15621579
'\n actual=' + a);
@@ -1598,9 +1615,13 @@ relativeTests2.forEach(function(relativeTest) {
15981615
var actual = url.resolveObject(url.parse(relativeTest[1]), relativeTest[0]);
15991616
var expected = url.parse(relativeTest[2]);
16001617

1601-
assert.deepStrictEqual(actual, expected);
1618+
assert.deepStrictEqual(
1619+
actual,
1620+
expected,
1621+
`expected ${inspect(expected)} but got ${inspect(actual)}`
1622+
);
16021623

1603-
expected = relativeTest[2];
1624+
expected = url.format(relativeTest[2]);
16041625
actual = url.format(actual);
16051626

16061627
assert.equal(actual, expected,

0 commit comments

Comments
 (0)