From 5437fa4511b1afda00b0d394a38cab48e623ea7f Mon Sep 17 00:00:00 2001 From: Jeffrey Jagoda Date: Thu, 26 Mar 2015 11:22:32 -0400 Subject: [PATCH] Avoid Node URL bug. --- src/assert.js | 2 +- src/document.js | 2 +- src/history.js | 2 +- src/index.js | 2 +- src/resources.js | 10 +++------- src/url.js | 22 ++++++++++++++++++++++ src/xhr.js | 2 +- test/history_test.js | 24 ++++++++++++++++++++++++ test/url_test.js | 39 +++++++++++++++++++++++++++++++++++++++ 9 files changed, 93 insertions(+), 12 deletions(-) create mode 100644 src/url.js create mode 100644 test/url_test.js diff --git a/src/assert.js b/src/assert.js index 4303fc3af..2e524e28a 100644 --- a/src/assert.js +++ b/src/assert.js @@ -2,7 +2,7 @@ const assert = require('assert'); const { isRegExp } = require('util'); -const URL = require('url'); +const URL = require('./url'); // Used to assert that actual matches expected value, where expected may be a function or a string. diff --git a/src/document.js b/src/document.js index 43835ac29..5a628830a 100644 --- a/src/document.js +++ b/src/document.js @@ -8,7 +8,7 @@ const DOM = require('./dom'); const EventSource = require('eventsource'); const WebSocket = require('ws'); const XMLHttpRequest = require('./xhr'); -const URL = require('url'); +const URL = require('./url'); // File access, not implemented yet diff --git a/src/history.js b/src/history.js index a6a9da776..fecc21f7e 100644 --- a/src/history.js +++ b/src/history.js @@ -32,7 +32,7 @@ const assert = require('assert'); const loadDocument = require('./document'); const DOM = require('./dom'); -const URL = require('url'); +const URL = require('./url'); class Location { diff --git a/src/index.js b/src/index.js index b4aa1f7d2..d2efee779 100644 --- a/src/index.js +++ b/src/index.js @@ -18,7 +18,7 @@ const Resources = require('./resources'); const Storages = require('./storage'); const Tough = require('tough-cookie'); const { Cookie } = Tough; -const URL = require('url'); +const URL = require('./url'); // Version number. We get this from package.json. diff --git a/src/resources.js b/src/resources.js index 2b33aee10..34e88c3ba 100644 --- a/src/resources.js +++ b/src/resources.js @@ -20,7 +20,7 @@ const iconv = require('iconv-lite'); const Path = require('path'); const QS = require('querystring'); const request = require('request'); -const URL = require('url'); +const URL = require('./url'); const Zlib = require('zlib'); @@ -269,14 +269,10 @@ 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); diff --git a/src/url.js b/src/url.js new file mode 100644 index 000000000..795653ba2 --- /dev/null +++ b/src/url.js @@ -0,0 +1,22 @@ +// Monkey patching the Node URL module to avoid a bug with URL resolution. +// See https://github.com/joyent/node/pull/14146 +const URL = require('url'); + + +exports.parse = URL.parse; +exports.format = URL.format; + +exports.resolve = function (from, to) { + const pattern = /file:?/; + const protocol = URL.parse(from).protocol; + // The `to` parameter is ofter a pre-parsed URL object. Need to make a + // defensive copy to avoid the `host` assignment bug. + const original = URL.parse(URL.format(to)); + const resolved = URL.parse(URL.resolve(from, to)); + + if (!pattern.test(protocol) && pattern.test(original.protocol) && !original.host && resolved.host) { + return URL.format(original); + } + + return URL.format(resolved); +}; diff --git a/src/xhr.js b/src/xhr.js index e96d8e03b..a50b9c301 100644 --- a/src/xhr.js +++ b/src/xhr.js @@ -3,7 +3,7 @@ const DOM = require('./dom'); -const URL = require('url'); +const URL = require('./url'); class XMLHttpRequest extends DOM.EventTarget { diff --git a/test/history_test.js b/test/history_test.js index ffdf35e12..4a64dc0e3 100644 --- a/test/history_test.js +++ b/test/history_test.js @@ -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) { diff --git a/test/url_test.js b/test/url_test.js new file mode 100644 index 000000000..62f1ea218 --- /dev/null +++ b/test/url_test.js @@ -0,0 +1,39 @@ +const assert = require('assert'); +const NativeURL = require('url'); +const ZombieURL = require('../src/url'); + + +describe('URL', function() { + describe('resolve', function() { + const patterns = [ + ['http://localhost', 'foo', 'http://localhost/foo'], + ['http://localhost/foo/bar', 'baz', 'http://localhost/foo/baz'], + ['http://localhost/foo/bar', '/bar', 'http://localhost/bar'], + ['http://localhost', 'file://foo/Users', 'file://foo/Users'], + ['http://localhost', 'file:///Users/foo', 'file:///Users/foo'], + ['file://foo/Users', 'file:bar', 'file://foo/bar'] + ]; + + it('is not the native resolve implementation', function() { + assert.notStrictEqual(ZombieURL.resolve, NativeURL.resolve); + }); + + it('resolves URLs correctly', function() { + patterns.forEach(function (pattern) { + assert.equal(ZombieURL.resolve(pattern[0], pattern[1]), pattern[2]); + }); + }); + }); + + describe('parse', function() { + it('is the native parse implementation', function() { + assert.strictEqual(ZombieURL.parse, NativeURL.parse); + }); + }); + + describe('format', function() { + it('is the native format implementation', function() { + assert.strictEqual(ZombieURL.format, NativeURL.format); + }); + }); +});