From 0f39ef4ca11bf0c6c6891dec3fa0d2d3d16513cb Mon Sep 17 00:00:00 2001 From: Rod Vagg Date: Sun, 3 May 2015 15:05:36 -0700 Subject: [PATCH] Revert "url: fix treatment of some values as non-empty" This reverts commit 66877216bd833face753d9a5d573ad477895d880. It was agreed that this change contained too much potential ecosystem breakage, particularly around the inability to `delete` properties off a `Url` object. It may be re-introduced for a later release, along with better work on ecosystem compatibility. PR-URL: #1602 Reviewed-By: Mikeal Rogers Reviewed-By: Ben Noordhuis Reviewed-By: Forrest L Norvell Reviewed-By: Chris Dickinson Reviewed-By: Isaac Z. Schlueter Reviewed-By: Jeremiah Senkpiel --- lib/url.js | 24 +++++------- test/parallel/test-url-accessors.js | 59 ++++++----------------------- 2 files changed, 22 insertions(+), 61 deletions(-) diff --git a/lib/url.js b/lib/url.js index df04ebc12aa322..52c39038767012 100644 --- a/lib/url.js +++ b/lib/url.js @@ -879,7 +879,7 @@ Object.defineProperty(Url.prototype, 'port', { return null; }, set: function(v) { - if (isConsideredEmpty(v)) { + if (v === null) { this._port = -1; if (this._host) this._host = null; @@ -941,7 +941,7 @@ Object.defineProperty(Url.prototype, 'path', { return (p == null && s) ? ('/' + s) : null; }, set: function(v) { - if (isConsideredEmpty(v)) { + if (v === null) { this._pathname = this._search = null; return; } @@ -973,7 +973,7 @@ Object.defineProperty(Url.prototype, 'protocol', { return proto; }, set: function(v) { - if (isConsideredEmpty(v)) { + if (v === null) { this._protocol = null; } else { var proto = '' + v; @@ -1001,7 +1001,7 @@ Object.defineProperty(Url.prototype, 'href', { var parsesQueryStrings = this._parsesQueryStrings; // Reset properties. Url.call(this); - if (!isConsideredEmpty(v)) + if (v !== null && v !== '') this.parse('' + v, parsesQueryStrings, false); }, enumerable: true, @@ -1013,7 +1013,7 @@ Object.defineProperty(Url.prototype, 'auth', { return this._auth; }, set: function(v) { - this._auth = isConsideredEmpty(v) ? null : '' + v; + this._auth = v === null ? null : '' + v; this._href = ''; }, enumerable: true, @@ -1026,7 +1026,7 @@ Object.defineProperty(Url.prototype, 'host', { return this._host; }, set: function(v) { - if (isConsideredEmpty(v)) { + if (v === null) { this._port = -1; this._hostname = this._host = null; } else { @@ -1053,7 +1053,7 @@ Object.defineProperty(Url.prototype, 'hostname', { return this._hostname; }, set: function(v) { - if (isConsideredEmpty(v)) { + if (v === null) { this._hostname = null; if (this._hasValidPort()) @@ -1080,7 +1080,7 @@ Object.defineProperty(Url.prototype, 'hash', { return this._hash; }, set: function(v) { - if (isConsideredEmpty(v) || v === '#') { + if (v === null) { this._hash = null; } else { var hash = '' + v; @@ -1100,7 +1100,7 @@ Object.defineProperty(Url.prototype, 'search', { return this._search; }, set: function(v) { - if (isConsideredEmpty(v) || v === '?') { + if (v === null) { this._search = this._query = null; } else { var search = escapeSearch('' + v); @@ -1125,7 +1125,7 @@ Object.defineProperty(Url.prototype, 'pathname', { return this._pathname; }, set: function(v) { - if (isConsideredEmpty(v)) { + if (v === null) { this._pathname = null; } else { var pathname = escapePathName('' + v).replace(/\\/g, '/'); @@ -1144,10 +1144,6 @@ Object.defineProperty(Url.prototype, 'pathname', { configurable: true }); -function isConsideredEmpty(value) { - return value === null || value === undefined || value === ''; -} - // Search `char1` (integer code for a character) in `string` // starting from `fromIndex` and ending at `string.length - 1` // or when a stop character is found. diff --git a/test/parallel/test-url-accessors.js b/test/parallel/test-url-accessors.js index 3c39fd6149a394..387c149d23263f 100644 --- a/test/parallel/test-url-accessors.js +++ b/test/parallel/test-url-accessors.js @@ -43,10 +43,10 @@ const accessorTests = [{ }, { // Setting href to non-null non-string coerces to string url: 'google', - set: {href: 0}, + set: {href: undefined}, test: { - path: '0', - href: '0' + path: 'undefined', + href: 'undefined' } }, { // Setting port is reflected in host @@ -180,8 +180,8 @@ const accessorTests = [{ url: 'http://www.google.com', set: {search: ''}, test: { - search: null, - path: '/' + search: '?', + path: '/?' } }, { @@ -203,11 +203,10 @@ const accessorTests = [{ }, { // Empty hash is ok - url: 'http://www.google.com#hash', + url: 'http://www.google.com', set: {hash: ''}, test: { - hash: null, - href: 'http://www.google.com/' + hash: '#' } }, { @@ -253,8 +252,7 @@ const accessorTests = [{ url: 'http://www.google.com', set: {pathname: ''}, test: { - pathname: null, - href: 'http://www.google.com' + pathname: '/' } }, { // Null path is ok @@ -292,12 +290,11 @@ const accessorTests = [{ protocol: null } }, { - // Empty protocol is ok + // Empty protocol is invalid url: 'http://www.google.com/path', set: {protocol: ''}, test: { - protocol: null, - href: '//www.google.com/path' + protocol: 'http:' } }, { // Set query to an object @@ -330,9 +327,9 @@ const accessorTests = [{ url: 'http://www.google.com/path?key=value', set: {path: '?key2=value2'}, test: { - pathname: null, + pathname: '/', search: '?key2=value2', - href: 'http://www.google.com?key2=value2' + href: 'http://www.google.com/?key2=value2' } }, { // path is reflected in search and pathname 3 @@ -352,38 +349,6 @@ const accessorTests = [{ search: null, href: 'http://www.google.com' } -}, { - // setting hash to '' removes any hash - url: 'http://www.google.com/#hash', - set: {hash: ''}, - test: { - hash: null, - href: 'http://www.google.com/' - } -}, { - // setting hash to '#' removes any hash - url: 'http://www.google.com/#hash', - set: {hash: '#'}, - test: { - hash: null, - href: 'http://www.google.com/' - } -}, { - // setting search to '' removes any search - url: 'http://www.google.com/?search', - set: {search: ''}, - test: { - search: null, - href: 'http://www.google.com/' - } -}, { - // setting search to '?' removes any search - url: 'http://www.google.com/?search', - set: {search: '?'}, - test: { - search: null, - href: 'http://www.google.com/' - } } ];