Skip to content

Commit

Permalink
url: ensure host setter matches parse for file url
Browse files Browse the repository at this point in the history
Technically, file URLs are not permitted to have a port. There is
currently an ambiguity in the URL spec. In the current spec
having a port in a file URL is undefined behavior. In the current
implementation, the port is ignored and handled as if it were part
of the host name. This will be changing once the ambiguity is
resolved in the spec. The spec change may involve either ignoring the
port if specified or throwing with an Invalid URL error if the port
is specified. For now, this test verifies the currently implemented
behavior.

Fixes: nodejs#10608
  • Loading branch information
jasnell committed Apr 21, 2017
1 parent bde26a6 commit 9d868c6
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 1 deletion.
4 changes: 3 additions & 1 deletion lib/internal/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,9 @@ Object.defineProperties(URL.prototype, {
ctx.flags &= ~binding.URL_FLAGS_HAS_HOST;
return;
}
binding.parse(host, binding.kHost, null, ctx,
const override = ctx.scheme === 'file:' ?
binding.kFileHost : binding.kHost;
binding.parse(host, override, null, ctx,
onParseHostComplete.bind(this));
}
},
Expand Down
30 changes: 30 additions & 0 deletions test/parallel/test-whatwg-url-filehostport.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
'use strict';

// Technically, file URLs are not permitted to have a port. There is
// currently an ambiguity in the URL spec. In the current spec
// having a port in a file URL is undefined behavior. In the current
// implementation, the port is ignored and handled as if it were part
// of the host name. This will be changing once the ambiguity is
// resolved in the spec. The spec change may involve either ignoring the
// port if specified or throwing with an Invalid URL error if the port
// is specified. For now, this test verifies the currently implemented
// behavior.

require('../common');
const URL = require('url').URL;
const assert = require('assert');

const u = new URL('file://example.net:80/foo');

assert.strictEqual(u.hostname, 'example.net:80');
assert.strictEqual(u.port, '');

u.host = 'example.com:81';

assert.strictEqual(u.hostname, 'example.com:81');
assert.strictEqual(u.port, '');

u.hostname = 'example.org:81';

assert.strictEqual(u.hostname, 'example.org');
assert.strictEqual(u.port, '');

0 comments on commit 9d868c6

Please sign in to comment.