Skip to content
This repository was archived by the owner on Dec 16, 2023. It is now read-only.

Commit a972ff4

Browse files
committed
Avoid Node URL bug.
1 parent de5aba9 commit a972ff4

File tree

7 files changed

+62
-18
lines changed

7 files changed

+62
-18
lines changed

src/assert.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
const assert = require('assert');
44
const { isRegExp } = require('util');
55
const URL = require('url');
6+
const Utils = require('jsdom/lib/jsdom/utils');
67

78

89
// Used to assert that actual matches expected value, where expected may be a function or a string.
@@ -58,7 +59,7 @@ module.exports = class Assert {
5859
// query).
5960
url(expected, message) {
6061
if (typeof expected === 'string') {
61-
const absolute = URL.resolve(this.browser.location.href, expected);
62+
const absolute = Utils.resolveHref(this.browser.location.href, expected);
6263
assertMatch(this.browser.location.href, absolute, message);
6364
} else if (isRegExp(expected) || typeof expected === 'function')
6465
assertMatch(this.browser.location.href, expected, message);
@@ -182,7 +183,7 @@ module.exports = class Assert {
182183
const matchedRegexp = matchingText.filter(element => url.test(element.href));
183184
assert(matchedRegexp.length, message || `Expected at least one link matching the given text and URL`);
184185
} else {
185-
const absolute = URL.resolve(this.browser.location.href, url);
186+
const absolute = Utils.resolveHref(this.browser.location.href, url);
186187
const matchedURL = matchingText.filter(element => element.href === absolute);
187188
assert(matchedURL.length, message || `Expected at least one link matching the given text and URL`);
188189
}

src/document.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const EventSource = require('eventsource');
99
const WebSocket = require('ws');
1010
const XMLHttpRequest = require('./xhr');
1111
const URL = require('url');
12+
const Utils = require('jsdom/lib/jsdom/utils');
1213

1314

1415
// File access, not implemented yet
@@ -51,7 +52,7 @@ class DOMURL {
5152
constructor(url, base) {
5253
assert(url != null, new DOM.DOMException('Failed to construct \'URL\': Invalid URL'));
5354
if (base)
54-
url = URL.resolve(base, url);
55+
url = Utils.resolveHref(base, url);
5556
const parsed = URL.parse(url || 'about:blank');
5657
const origin = parsed.protocol && parsed.hostname && `${parsed.protocol}//${parsed.hostname}`;
5758
Object.defineProperties(this, {
@@ -491,7 +492,7 @@ module.exports = function loadDocument(args) {
491492
let { url } = args;
492493
if (url && browser.site) {
493494
const site = /^(https?:|file:)/i.test(browser.site) ? browser.site : `http://${browser.site}`;
494-
url = URL.resolve(site, URL.parse(URL.format(url)));
495+
url = Utils.resolveHref(site, URL.format(url));
495496
}
496497
url = url || 'about:blank';
497498

src/dom/jsdom_patches.js

+20-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
// Fix things that JSDOM doesn't do quite right.
22

33

4-
const DOM = require('./index');
4+
const DOM = require('./index');
5+
const Utils = require('jsdom/lib/jsdom/utils');
6+
const URL = require('url');
57

68

79
DOM.HTMLDocument.prototype.__defineGetter__('scripts', function() {
@@ -200,3 +202,20 @@ DOM.resourceLoader.load = function(element, href, callback) {
200202
}
201203
};
202204

205+
// Fix residual Node bug. See https://github.com/joyent/node/pull/14146
206+
const jsdomResolveHref = Utils.resolveHref;
207+
Utils.resolveHref = function (baseUrl, href) {
208+
const pattern = /file:?/;
209+
const protocol = URL.parse(baseUrl).protocol;
210+
// The `to` parameter is ofter a pre-parsed URL object. Need to make a
211+
// defensive copy to avoid the `host` assignment bug.
212+
const original = URL.parse(URL.format(href));
213+
const resolved = URL.parse(jsdomResolveHref(baseUrl, href));
214+
215+
if (!pattern.test(protocol) && pattern.test(original.protocol) && !original.host && resolved.host) {
216+
return URL.format(original);
217+
}
218+
219+
return URL.format(resolved);
220+
};
221+

src/index.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ const Storages = require('./storage');
1919
const Tough = require('tough-cookie');
2020
const { Cookie } = Tough;
2121
const URL = require('url');
22+
const Utils = require('jsdom/lib/jsdom/utils');
2223

2324

2425
// Version number. We get this from package.json.
@@ -536,7 +537,7 @@ class Browser extends EventEmitter {
536537
[options, callback] = [{}, options];
537538

538539
const site = /^(https?:|file:)/i.test(this.site) ? this.site : `http://${this.site || 'localhost'}/`;
539-
url = URL.resolve(site, URL.parse(URL.format(url)));
540+
url = Utils.resolveHref(site, URL.format(url));
540541

541542
if (this.window)
542543
this.tabs.close(this.window);

src/resources.js

+6-9
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ const Path = require('path');
2121
const QS = require('querystring');
2222
const request = require('request');
2323
const URL = require('url');
24+
const Utils = require('jsdom/lib/jsdom/utils');
2425
const Zlib = require('zlib');
2526

2627

@@ -269,19 +270,15 @@ Resources.addHandler = function(handler) {
269270
// It turns relative URLs into absolute URLs based on the current document URL
270271
// or base element, or if no document open, based on browser.site property.
271272
//
272-
// Also handles file: URLs and creates query string from request.params for
273+
// Also creates query string from request.params for
273274
// GET/HEAD/DELETE requests.
274275
Resources.normalizeURL = function(req, next) {
275-
if (/^file:/.test(req.url))
276-
// File URLs are special, need to handle missing slashes and not attempt
277-
// to parse (downcases path)
278-
req.url = req.url.replace(/^file:\/{1,3}/, 'file:///');
279-
else if (this.document)
276+
if (this.document)
280277
// Resolve URL relative to document URL/base, or for new browser, using
281278
// Browser.site
282279
req.url = DOM.resourceLoader.resolve(this.document, req.url);
283280
else
284-
req.url = URL.resolve(this.site || 'http://localhost', req.url);
281+
req.url = Utils.resolveHref(this.site || 'http://localhost', req.url);
285282

286283
if (req.params) {
287284
const { method } = req;
@@ -442,10 +439,10 @@ Resources.handleHTTPResponse = function(req, res, next) {
442439
if ((statusCode === 301 || statusCode === 307) &&
443440
(req.method === 'GET' || req.method === 'HEAD'))
444441
// Do not follow POST redirects automatically, only GET/HEAD
445-
redirectUrl = URL.resolve(req.url, res.headers.location || '');
442+
redirectUrl = Utils.resolveHref(req.url, res.headers.location || '');
446443
else if (statusCode === 302 || statusCode === 303)
447444
// Follow redirect using GET (e.g. after form submission)
448-
redirectUrl = URL.resolve(req.url, res.headers.location || '');
445+
redirectUrl = Utils.resolveHref(req.url, res.headers.location || '');
449446

450447
if (redirectUrl) {
451448

src/xhr.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22
// See http://www.w3.org/TR/XMLHttpRequest/#the-abort()-method
33

44

5-
const DOM = require('./dom');
6-
const URL = require('url');
5+
const DOM = require('./dom');
6+
const URL = require('url');
7+
const Utils = require('jsdom/lib/jsdom/utils');
78

89

910
class XMLHttpRequest extends DOM.EventTarget {
@@ -59,7 +60,7 @@ class XMLHttpRequest extends DOM.EventTarget {
5960
const headers = {};
6061

6162
// Normalize the URL and check security
62-
url = URL.parse(URL.resolve(this._window.location.href, url));
63+
url = URL.parse(Utils.resolveHref(this._window.location.href, url));
6364
// Don't consider port if they are standard for http and https
6465
if ((url.protocol === 'https:' && url.port === '443') ||
6566
(url.protocol === 'http:' && url.port === '80'))

test/history_test.js

+24
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,30 @@ describe('History', function() {
337337
});
338338
});
339339

340+
// Node has a bug that causes the root path element to be lowercased which
341+
// causes problems when loading files from the file system.
342+
// See https://github.com/joyent/node/pull/14146
343+
describe('open from file system with capitalized root', function() {
344+
const FILE_URL = 'file:///Users/foo/index.html';
345+
346+
before(function (done) {
347+
browser.visit(FILE_URL, function () {
348+
// Ignore errors -- the file isn't real...
349+
done();
350+
});
351+
});
352+
353+
it('should change location URL', function() {
354+
browser.assert.url(FILE_URL);
355+
});
356+
it('should set window location', function() {
357+
assert.equal(browser.window.location.href, FILE_URL);
358+
});
359+
it('should set document location', function() {
360+
assert.equal(browser.document.location.href, FILE_URL);
361+
});
362+
});
363+
340364
describe('change pathname', function() {
341365
before(()=> browser.visit('/'));
342366
before(function(done) {

0 commit comments

Comments
 (0)