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

Commit

Permalink
Avoid Node URL bug.
Browse files Browse the repository at this point in the history
  • Loading branch information
jagoda committed Mar 26, 2015
1 parent de5aba9 commit 94ca412
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 18 deletions.
5 changes: 3 additions & 2 deletions src/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const assert = require('assert');
const { isRegExp } = require('util');
const URL = require('url');
const Utils = require('jsdom/lib/jsdom/utils');


// Used to assert that actual matches expected value, where expected may be a function or a string.
Expand Down Expand Up @@ -58,7 +59,7 @@ module.exports = class Assert {
// query).
url(expected, message) {
if (typeof expected === 'string') {
const absolute = URL.resolve(this.browser.location.href, expected);
const absolute = Utils.resolveHref(this.browser.location.href, expected);
assertMatch(this.browser.location.href, absolute, message);
} else if (isRegExp(expected) || typeof expected === 'function')
assertMatch(this.browser.location.href, expected, message);
Expand Down Expand Up @@ -182,7 +183,7 @@ module.exports = class Assert {
const matchedRegexp = matchingText.filter(element => url.test(element.href));
assert(matchedRegexp.length, message || `Expected at least one link matching the given text and URL`);
} else {
const absolute = URL.resolve(this.browser.location.href, url);
const absolute = Utils.resolveHref(this.browser.location.href, url);
const matchedURL = matchingText.filter(element => element.href === absolute);
assert(matchedURL.length, message || `Expected at least one link matching the given text and URL`);
}
Expand Down
5 changes: 3 additions & 2 deletions src/document.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const EventSource = require('eventsource');
const WebSocket = require('ws');
const XMLHttpRequest = require('./xhr');
const URL = require('url');
const Utils = require('jsdom/lib/jsdom/utils');


// File access, not implemented yet
Expand Down Expand Up @@ -51,7 +52,7 @@ class DOMURL {
constructor(url, base) {
assert(url != null, new DOM.DOMException('Failed to construct \'URL\': Invalid URL'));
if (base)
url = URL.resolve(base, url);
url = Utils.resolveHref(base, url);
const parsed = URL.parse(url || 'about:blank');
const origin = parsed.protocol && parsed.hostname && `${parsed.protocol}//${parsed.hostname}`;
Object.defineProperties(this, {
Expand Down Expand Up @@ -491,7 +492,7 @@ module.exports = function loadDocument(args) {
let { url } = args;
if (url && browser.site) {
const site = /^(https?:|file:)/i.test(browser.site) ? browser.site : `http://${browser.site}`;
url = URL.resolve(site, URL.parse(URL.format(url)));
url = Utils.resolveHref(site, URL.format(url));
}
url = url || 'about:blank';

Expand Down
19 changes: 18 additions & 1 deletion src/dom/jsdom_patches.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
// Fix things that JSDOM doesn't do quite right.


const DOM = require('./index');
const DOM = require('./index');
const Utils = require('jsdom/lib/jsdom/utils');
const URL = require('url');


DOM.HTMLDocument.prototype.__defineGetter__('scripts', function() {
Expand Down Expand Up @@ -200,3 +202,18 @@ DOM.resourceLoader.load = function(element, href, callback) {
}
};

// Fix residual Node bug. See https://github.com/joyent/node/pull/14146
const jsdomResolveHref = Utils.resolveHref;
Utils.resolveHref = function (baseUrl, href) {
const pattern = /file:?/;
const protocol = URL.parse(baseUrl).protocol;
const original = URL.parse(href);
const resolved = URL.parse(jsdomResolveHref(baseUrl, href));

if (!pattern.test(protocol) && pattern.test(original.protocol) && !original.host && resolved.host) {
return URL.format(original);
}

return URL.format(resolved);
};

3 changes: 2 additions & 1 deletion src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const Storages = require('./storage');
const Tough = require('tough-cookie');
const { Cookie } = Tough;
const URL = require('url');
const Utils = require('jsdom/lib/jsdom/utils');


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

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

if (this.window)
this.tabs.close(this.window);
Expand Down
15 changes: 6 additions & 9 deletions src/resources.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const Path = require('path');
const QS = require('querystring');
const request = require('request');
const URL = require('url');
const Utils = require('jsdom/lib/jsdom/utils');
const Zlib = require('zlib');


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

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

if (redirectUrl) {

Expand Down
7 changes: 4 additions & 3 deletions src/xhr.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
// See http://www.w3.org/TR/XMLHttpRequest/#the-abort()-method


const DOM = require('./dom');
const URL = require('url');
const DOM = require('./dom');
const URL = require('url');
const Utils = require('jsdom/lib/jsdom/utils');


class XMLHttpRequest extends DOM.EventTarget {
Expand Down Expand Up @@ -59,7 +60,7 @@ class XMLHttpRequest extends DOM.EventTarget {
const headers = {};

// Normalize the URL and check security
url = URL.parse(URL.resolve(this._window.location.href, url));
url = URL.parse(Utils.resolveHref(this._window.location.href, url));
// Don't consider port if they are standard for http and https
if ((url.protocol === 'https:' && url.port === '443') ||
(url.protocol === 'http:' && url.port === '80'))
Expand Down
24 changes: 24 additions & 0 deletions test/history_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,30 @@ describe('History', function() {
});
});

// Node has a bug that causes the root path element to be lowercased which
// causes problems when loading files from the file system.
// See https://github.com/joyent/node/pull/14146
describe('open from file system with capitalized root', function() {
const FILE_URL = 'file:///Users/foo/index.html';

before(function (done) {
browser.visit(FILE_URL, function () {
// Ignore errors -- the file isn't real...
done();
});
});

it('should change location URL', function() {
browser.assert.url(FILE_URL);
});
it('should set window location', function() {
assert.equal(browser.window.location.href, FILE_URL);
});
it('should set document location', function() {
assert.equal(browser.document.location.href, FILE_URL);
});
});

describe('change pathname', function() {
before(()=> browser.visit('/'));
before(function(done) {
Expand Down

0 comments on commit 94ca412

Please sign in to comment.