Skip to content

Commit 60da4b0

Browse files
joyeecheungjoyeecheung
committed
test, url: improve escaping in url.parse
- rename variables in autoEscapeStr so they are easier to understand - comment the escaping algorithm - increase coverage for autoEscapeStr
1 parent b49b496 commit 60da4b0

File tree

2 files changed

+85
-65
lines changed

2 files changed

+85
-65
lines changed

‎lib/url.js

+71-65
Original file line numberDiff line numberDiff line change
@@ -437,105 +437,111 @@ function validateHostname(self, rest, hostname) {
437437
}
438438
}
439439

440+
// Automatically escape all delimiters and unwise characters from RFC 2396.
441+
// Also escape single quotes in case of an XSS attack.
442+
// Return undefined if the string doesn't need escaping,
443+
// otherwise return the escaped string.
440444
function autoEscapeStr(rest) {
441-
var newRest = '';
442-
var lastPos = 0;
445+
var escaped = '';
446+
var lastEscapedPos = 0;
443447
for (var i = 0; i < rest.length; ++i) {
444-
// Automatically escape all delimiters and unwise characters from RFC 2396
445-
// Also escape single quotes in case of an XSS attack
448+
// Manual switching is faster than using a Map/Object.
449+
// `escaped` contains substring up to the last escaped cahracter.
446450
switch (rest.charCodeAt(i)) {
447451
case 9: // '\t'
448-
if (i - lastPos > 0)
449-
newRest += rest.slice(lastPos, i);
450-
newRest += '%09';
451-
lastPos = i + 1;
452+
// Concat if there are ordinary characters in the middle.
453+
if (i > lastEscapedPos)
454+
escaped += rest.slice(lastEscapedPos, i);
455+
escaped += '%09';
456+
lastEscapedPos = i + 1;
452457
break;
453458
case 10: // '\n'
454-
if (i - lastPos > 0)
455-
newRest += rest.slice(lastPos, i);
456-
newRest += '%0A';
457-
lastPos = i + 1;
459+
if (i > lastEscapedPos)
460+
escaped += rest.slice(lastEscapedPos, i);
461+
escaped += '%0A';
462+
lastEscapedPos = i + 1;
458463
break;
459464
case 13: // '\r'
460-
if (i - lastPos > 0)
461-
newRest += rest.slice(lastPos, i);
462-
newRest += '%0D';
463-
lastPos = i + 1;
465+
if (i > lastEscapedPos)
466+
escaped += rest.slice(lastEscapedPos, i);
467+
escaped += '%0D';
468+
lastEscapedPos = i + 1;
464469
break;
465470
case 32: // ' '
466-
if (i - lastPos > 0)
467-
newRest += rest.slice(lastPos, i);
468-
newRest += '%20';
469-
lastPos = i + 1;
471+
if (i > lastEscapedPos)
472+
escaped += rest.slice(lastEscapedPos, i);
473+
escaped += '%20';
474+
lastEscapedPos = i + 1;
470475
break;
471476
case 34: // '"'
472-
if (i - lastPos > 0)
473-
newRest += rest.slice(lastPos, i);
474-
newRest += '%22';
475-
lastPos = i + 1;
477+
if (i > lastEscapedPos)
478+
escaped += rest.slice(lastEscapedPos, i);
479+
escaped += '%22';
480+
lastEscapedPos = i + 1;
476481
break;
477482
case 39: // '\''
478-
if (i - lastPos > 0)
479-
newRest += rest.slice(lastPos, i);
480-
newRest += '%27';
481-
lastPos = i + 1;
483+
if (i > lastEscapedPos)
484+
escaped += rest.slice(lastEscapedPos, i);
485+
escaped += '%27';
486+
lastEscapedPos = i + 1;
482487
break;
483488
case 60: // '<'
484-
if (i - lastPos > 0)
485-
newRest += rest.slice(lastPos, i);
486-
newRest += '%3C';
487-
lastPos = i + 1;
489+
if (i > lastEscapedPos)
490+
escaped += rest.slice(lastEscapedPos, i);
491+
escaped += '%3C';
492+
lastEscapedPos = i + 1;
488493
break;
489494
case 62: // '>'
490-
if (i - lastPos > 0)
491-
newRest += rest.slice(lastPos, i);
492-
newRest += '%3E';
493-
lastPos = i + 1;
495+
if (i > lastEscapedPos)
496+
escaped += rest.slice(lastEscapedPos, i);
497+
escaped += '%3E';
498+
lastEscapedPos = i + 1;
494499
break;
495500
case 92: // '\\'
496-
if (i - lastPos > 0)
497-
newRest += rest.slice(lastPos, i);
498-
newRest += '%5C';
499-
lastPos = i + 1;
501+
if (i > lastEscapedPos)
502+
escaped += rest.slice(lastEscapedPos, i);
503+
escaped += '%5C';
504+
lastEscapedPos = i + 1;
500505
break;
501506
case 94: // '^'
502-
if (i - lastPos > 0)
503-
newRest += rest.slice(lastPos, i);
504-
newRest += '%5E';
505-
lastPos = i + 1;
507+
if (i > lastEscapedPos)
508+
escaped += rest.slice(lastEscapedPos, i);
509+
escaped += '%5E';
510+
lastEscapedPos = i + 1;
506511
break;
507512
case 96: // '`'
508-
if (i - lastPos > 0)
509-
newRest += rest.slice(lastPos, i);
510-
newRest += '%60';
511-
lastPos = i + 1;
513+
if (i > lastEscapedPos)
514+
escaped += rest.slice(lastEscapedPos, i);
515+
escaped += '%60';
516+
lastEscapedPos = i + 1;
512517
break;
513518
case 123: // '{'
514-
if (i - lastPos > 0)
515-
newRest += rest.slice(lastPos, i);
516-
newRest += '%7B';
517-
lastPos = i + 1;
519+
if (i > lastEscapedPos)
520+
escaped += rest.slice(lastEscapedPos, i);
521+
escaped += '%7B';
522+
lastEscapedPos = i + 1;
518523
break;
519524
case 124: // '|'
520-
if (i - lastPos > 0)
521-
newRest += rest.slice(lastPos, i);
522-
newRest += '%7C';
523-
lastPos = i + 1;
525+
if (i > lastEscapedPos)
526+
escaped += rest.slice(lastEscapedPos, i);
527+
escaped += '%7C';
528+
lastEscapedPos = i + 1;
524529
break;
525530
case 125: // '}'
526-
if (i - lastPos > 0)
527-
newRest += rest.slice(lastPos, i);
528-
newRest += '%7D';
529-
lastPos = i + 1;
531+
if (i > lastEscapedPos)
532+
escaped += rest.slice(lastEscapedPos, i);
533+
escaped += '%7D';
534+
lastEscapedPos = i + 1;
530535
break;
531536
}
532537
}
533-
if (lastPos === 0)
538+
if (lastEscapedPos === 0) // Nothing has been escaped.
534539
return;
535-
if (lastPos < rest.length)
536-
return newRest + rest.slice(lastPos);
537-
else
538-
return newRest;
540+
// There are ordinary characters at the end.
541+
if (lastEscapedPos < rest.length)
542+
return escaped + rest.slice(lastEscapedPos);
543+
else // The last character is escaped.
544+
return escaped;
539545
}
540546

541547
// format a parsed object into a url string

‎test/parallel/test-url.js

+14
Original file line numberDiff line numberDiff line change
@@ -834,6 +834,20 @@ var parseTests = {
834834
query: '@c'
835835
},
836836

837+
'http://a.b/\tbc\ndr\ref g"hq\'j<kl>?mn\\op^q=r`99{st|uv}wz': {
838+
protocol: 'http:',
839+
slashes: true,
840+
host: 'a.b',
841+
port: null,
842+
hostname: 'a.b',
843+
hash: null,
844+
pathname: '/%09bc%0Adr%0Def%20g%22hq%27j%3Ckl%3E',
845+
path: '/%09bc%0Adr%0Def%20g%22hq%27j%3Ckl%3E?mn%5Cop%5Eq=r%6099%7Bst%7Cuv%7Dwz',
846+
search: '?mn%5Cop%5Eq=r%6099%7Bst%7Cuv%7Dwz',
847+
query: 'mn%5Cop%5Eq=r%6099%7Bst%7Cuv%7Dwz',
848+
href: 'http://a.b/%09bc%0Adr%0Def%20g%22hq%27j%3Ckl%3E?mn%5Cop%5Eq=r%6099%7Bst%7Cuv%7Dwz'
849+
},
850+
837851
'http://a\r" \t\n<\'b:b@c\r\nd/e?f': {
838852
protocol: 'http:',
839853
slashes: true,

0 commit comments

Comments
 (0)