url: fix treatment of some values as non-empty

In addition to null, undefined and the empty string
are treated as empty (removing the component from the url).

The string '#' is treated same as empty values when
setting .hash.

The string '?' is treated same as empty values when
setting .search.

PR-URL: nodejs#1589
Fixes: nodejs#1588
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

lib/url.js

@@ -879,7 +879,7 @@ Object.defineProperty(Url.prototype, 'port', {
     return null;
   },
   set: function(v) {
-    if (v === null) {
+    if (isConsideredEmpty(v)) {
       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 (v === null) {
+    if (isConsideredEmpty(v)) {
       this._pathname = this._search = null;
       return;
     }
@@ -973,7 +973,7 @@ Object.defineProperty(Url.prototype, 'protocol', {
     return proto;
   },
   set: function(v) {
-    if (v === null) {
+    if (isConsideredEmpty(v)) {
       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 (v !== null && v !== '')
+    if (!isConsideredEmpty(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 = v === null ? null : '' + v;
+    this._auth = isConsideredEmpty(v) ? null : '' + v;
     this._href = '';
   },
   enumerable: true,
@@ -1026,7 +1026,7 @@ Object.defineProperty(Url.prototype, 'host', {
     return this._host;
   },
   set: function(v) {
-    if (v === null) {
+    if (isConsideredEmpty(v)) {
       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 (v === null) {
+    if (isConsideredEmpty(v)) {
       this._hostname = null;
 
       if (this._hasValidPort())
@@ -1080,7 +1080,7 @@ Object.defineProperty(Url.prototype, 'hash', {
     return this._hash;
   },
   set: function(v) {
-    if (v === null) {
+    if (isConsideredEmpty(v) || v === '#') {
       this._hash = null;
     } else {
       var hash = '' + v;
@@ -1100,7 +1100,7 @@ Object.defineProperty(Url.prototype, 'search', {
     return this._search;
   },
   set: function(v) {
-    if (v === null) {
+    if (isConsideredEmpty(v) || v === '?') {
       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 (v === null) {
+    if (isConsideredEmpty(v)) {
       this._pathname = null;
     } else {
       var pathname = escapePathName('' + v).replace(/\\/g, '/');
@@ -1144,6 +1144,10 @@ 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.

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: undefined},
+  set: {href: 0},
   test: {
-    path: 'undefined',
-    href: 'undefined'
+    path: '0',
+    href: '0'
   }
 }, {
   // Setting port is reflected in host
@@ -180,8 +180,8 @@ const accessorTests = [{
   url: 'http://www.google.com',
   set: {search: ''},
   test: {
-    search: '?',
-    path: '/?'
+    search: null,
+    path: '/'
   }
 }, {
 
@@ -203,10 +203,11 @@ const accessorTests = [{
 }, {
 
   // Empty hash is ok
-  url: 'http://www.google.com',
+  url: 'http://www.google.com#hash',
   set: {hash: ''},
   test: {
-    hash: '#'
+    hash: null,
+    href: 'http://www.google.com/'
   }
 }, {
 
@@ -252,7 +253,8 @@ const accessorTests = [{
   url: 'http://www.google.com',
   set: {pathname: ''},
   test: {
-    pathname: '/'
+    pathname: null,
+    href: 'http://www.google.com'
   }
 }, {
   // Null path is ok
@@ -290,11 +292,12 @@ const accessorTests = [{
     protocol: null
   }
 }, {
-  // Empty protocol is invalid
+  // Empty protocol is ok
   url: 'http://www.google.com/path',
   set: {protocol: ''},
   test: {
-    protocol: 'http:'
+    protocol: null,
+    href: '//www.google.com/path'
   }
 }, {
   // Set query to an object
@@ -327,9 +330,9 @@ const accessorTests = [{
   url: 'http://www.google.com/path?key=value',
   set: {path: '?key2=value2'},
   test: {
-    pathname: '/',
+    pathname: null,
     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
@@ -349,6 +352,38 @@ 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/'
+  }
 }
 
 ];