Skip to content

Commit 6687721

Browse files
petkaantonovpetkaantonov
committed
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: #1589 Fixes: #1588 Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
1 parent dbdd81a commit 6687721

2 files changed

Lines changed: 61 additions & 22 deletions

File tree

‎lib/url.js‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -879,7 +879,7 @@ Object.defineProperty(Url.prototype, 'port', {
879879
return null;
880880
},
881881
set: function(v) {
882-
if (v === null) {
882+
if (isConsideredEmpty(v)) {
883883
this._port = -1;
884884
if (this._host)
885885
this._host = null;
@@ -941,7 +941,7 @@ Object.defineProperty(Url.prototype, 'path', {
941941
return (p == null && s) ? ('/' + s) : null;
942942
},
943943
set: function(v) {
944-
if (v === null) {
944+
if (isConsideredEmpty(v)) {
945945
this._pathname = this._search = null;
946946
return;
947947
}
@@ -973,7 +973,7 @@ Object.defineProperty(Url.prototype, 'protocol', {
973973
return proto;
974974
},
975975
set: function(v) {
976-
if (v === null) {
976+
if (isConsideredEmpty(v)) {
977977
this._protocol = null;
978978
} else {
979979
var proto = '' + v;
@@ -1001,7 +1001,7 @@ Object.defineProperty(Url.prototype, 'href', {
10011001
var parsesQueryStrings = this._parsesQueryStrings;
10021002
// Reset properties.
10031003
Url.call(this);
1004-
if (v !== null && v !== '')
1004+
if (!isConsideredEmpty(v))
10051005
this.parse('' + v, parsesQueryStrings, false);
10061006
},
10071007
enumerable: true,
@@ -1013,7 +1013,7 @@ Object.defineProperty(Url.prototype, 'auth', {
10131013
return this._auth;
10141014
},
10151015
set: function(v) {
1016-
this._auth = v === null ? null : '' + v;
1016+
this._auth = isConsideredEmpty(v) ? null : '' + v;
10171017
this._href = '';
10181018
},
10191019
enumerable: true,
@@ -1026,7 +1026,7 @@ Object.defineProperty(Url.prototype, 'host', {
10261026
return this._host;
10271027
},
10281028
set: function(v) {
1029-
if (v === null) {
1029+
if (isConsideredEmpty(v)) {
10301030
this._port = -1;
10311031
this._hostname = this._host = null;
10321032
} else {
@@ -1053,7 +1053,7 @@ Object.defineProperty(Url.prototype, 'hostname', {
10531053
return this._hostname;
10541054
},
10551055
set: function(v) {
1056-
if (v === null) {
1056+
if (isConsideredEmpty(v)) {
10571057
this._hostname = null;
10581058

10591059
if (this._hasValidPort())
@@ -1080,7 +1080,7 @@ Object.defineProperty(Url.prototype, 'hash', {
10801080
return this._hash;
10811081
},
10821082
set: function(v) {
1083-
if (v === null) {
1083+
if (isConsideredEmpty(v) || v === '#') {
10841084
this._hash = null;
10851085
} else {
10861086
var hash = '' + v;
@@ -1100,7 +1100,7 @@ Object.defineProperty(Url.prototype, 'search', {
11001100
return this._search;
11011101
},
11021102
set: function(v) {
1103-
if (v === null) {
1103+
if (isConsideredEmpty(v) || v === '?') {
11041104
this._search = this._query = null;
11051105
} else {
11061106
var search = escapeSearch('' + v);
@@ -1125,7 +1125,7 @@ Object.defineProperty(Url.prototype, 'pathname', {
11251125
return this._pathname;
11261126
},
11271127
set: function(v) {
1128-
if (v === null) {
1128+
if (isConsideredEmpty(v)) {
11291129
this._pathname = null;
11301130
} else {
11311131
var pathname = escapePathName('' + v).replace(/\\/g, '/');
@@ -1144,6 +1144,10 @@ Object.defineProperty(Url.prototype, 'pathname', {
11441144
configurable: true
11451145
});
11461146

1147+
function isConsideredEmpty(value) {
1148+
return value === null || value === undefined || value === '';
1149+
}
1150+
11471151
// Search `char1` (integer code for a character) in `string`
11481152
// starting from `fromIndex` and ending at `string.length - 1`
11491153
// or when a stop character is found.

‎test/parallel/test-url-accessors.js‎

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,10 @@ const accessorTests = [{
4343
}, {
4444
// Setting href to non-null non-string coerces to string
4545
url: 'google',
46-
set: {href: undefined},
46+
set: {href: 0},
4747
test: {
48-
path: 'undefined',
49-
href: 'undefined'
48+
path: '0',
49+
href: '0'
5050
}
5151
}, {
5252
// Setting port is reflected in host
@@ -180,8 +180,8 @@ const accessorTests = [{
180180
url: 'http://www.google.com',
181181
set: {search: ''},
182182
test: {
183-
search: '?',
184-
path: '/?'
183+
search: null,
184+
path: '/'
185185
}
186186
}, {
187187

@@ -203,10 +203,11 @@ const accessorTests = [{
203203
}, {
204204

205205
// Empty hash is ok
206-
url: 'http://www.google.com',
206+
url: 'http://www.google.com#hash',
207207
set: {hash: ''},
208208
test: {
209-
hash: '#'
209+
hash: null,
210+
href: 'http://www.google.com/'
210211
}
211212
}, {
212213

@@ -252,7 +253,8 @@ const accessorTests = [{
252253
url: 'http://www.google.com',
253254
set: {pathname: ''},
254255
test: {
255-
pathname: '/'
256+
pathname: null,
257+
href: 'http://www.google.com'
256258
}
257259
}, {
258260
// Null path is ok
@@ -290,11 +292,12 @@ const accessorTests = [{
290292
protocol: null
291293
}
292294
}, {
293-
// Empty protocol is invalid
295+
// Empty protocol is ok
294296
url: 'http://www.google.com/path',
295297
set: {protocol: ''},
296298
test: {
297-
protocol: 'http:'
299+
protocol: null,
300+
href: '//www.google.com/path'
298301
}
299302
}, {
300303
// Set query to an object
@@ -327,9 +330,9 @@ const accessorTests = [{
327330
url: 'http://www.google.com/path?key=value',
328331
set: {path: '?key2=value2'},
329332
test: {
330-
pathname: '/',
333+
pathname: null,
331334
search: '?key2=value2',
332-
href: 'http://www.google.com/?key2=value2'
335+
href: 'http://www.google.com?key2=value2'
333336
}
334337
}, {
335338
// path is reflected in search and pathname 3
@@ -349,6 +352,38 @@ const accessorTests = [{
349352
search: null,
350353
href: 'http://www.google.com'
351354
}
355+
}, {
356+
// setting hash to '' removes any hash
357+
url: 'http://www.google.com/#hash',
358+
set: {hash: ''},
359+
test: {
360+
hash: null,
361+
href: 'http://www.google.com/'
362+
}
363+
}, {
364+
// setting hash to '#' removes any hash
365+
url: 'http://www.google.com/#hash',
366+
set: {hash: '#'},
367+
test: {
368+
hash: null,
369+
href: 'http://www.google.com/'
370+
}
371+
}, {
372+
// setting search to '' removes any search
373+
url: 'http://www.google.com/?search',
374+
set: {search: ''},
375+
test: {
376+
search: null,
377+
href: 'http://www.google.com/'
378+
}
379+
}, {
380+
// setting search to '?' removes any search
381+
url: 'http://www.google.com/?search',
382+
set: {search: '?'},
383+
test: {
384+
search: null,
385+
href: 'http://www.google.com/'
386+
}
352387
}
353388

354389
];

0 commit comments

Comments
 (0)