Skip to content

Commit 13b1688

Browse files
joyeecheungMylesBorins
authored andcommitted
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 PR-URL: #10083 Reviewed-By: Anna Henningsen <[email protected]>
1 parent b167727 commit 13b1688

File tree

2 files changed

+85
-65
lines changed

2 files changed

+85
-65
lines changed

lib/url.js

Lines changed: 71 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -433,105 +433,111 @@ function validateHostname(self, rest, hostname) {
433433
}
434434
}
435435

436+
// Automatically escape all delimiters and unwise characters from RFC 2396.
437+
// Also escape single quotes in case of an XSS attack.
438+
// Return undefined if the string doesn't need escaping,
439+
// otherwise return the escaped string.
436440
function autoEscapeStr(rest) {
437-
var newRest = '';
438-
var lastPos = 0;
441+
var escaped = '';
442+
var lastEscapedPos = 0;
439443
for (var i = 0; i < rest.length; ++i) {
440-
// Automatically escape all delimiters and unwise characters from RFC 2396
441-
// Also escape single quotes in case of an XSS attack
444+
// Manual switching is faster than using a Map/Object.
445+
// `escaped` contains substring up to the last escaped cahracter.
442446
switch (rest.charCodeAt(i)) {
443447
case 9: // '\t'
444-
if (i - lastPos > 0)
445-
newRest += rest.slice(lastPos, i);
446-
newRest += '%09';
447-
lastPos = i + 1;
448+
// Concat if there are ordinary characters in the middle.
449+
if (i > lastEscapedPos)
450+
escaped += rest.slice(lastEscapedPos, i);
451+
escaped += '%09';
452+
lastEscapedPos = i + 1;
448453
break;
449454
case 10: // '\n'
450-
if (i - lastPos > 0)
451-
newRest += rest.slice(lastPos, i);
452-
newRest += '%0A';
453-
lastPos = i + 1;
455+
if (i > lastEscapedPos)
456+
escaped += rest.slice(lastEscapedPos, i);
457+
escaped += '%0A';
458+
lastEscapedPos = i + 1;
454459
break;
455460
case 13: // '\r'
456-
if (i - lastPos > 0)
457-
newRest += rest.slice(lastPos, i);
458-
newRest += '%0D';
459-
lastPos = i + 1;
461+
if (i > lastEscapedPos)
462+
escaped += rest.slice(lastEscapedPos, i);
463+
escaped += '%0D';
464+
lastEscapedPos = i + 1;
460465
break;
461466
case 32: // ' '
462-
if (i - lastPos > 0)
463-
newRest += rest.slice(lastPos, i);
464-
newRest += '%20';
465-
lastPos = i + 1;
467+
if (i > lastEscapedPos)
468+
escaped += rest.slice(lastEscapedPos, i);
469+
escaped += '%20';
470+
lastEscapedPos = i + 1;
466471
break;
467472
case 34: // '"'
468-
if (i - lastPos > 0)
469-
newRest += rest.slice(lastPos, i);
470-
newRest += '%22';
471-
lastPos = i + 1;
473+
if (i > lastEscapedPos)
474+
escaped += rest.slice(lastEscapedPos, i);
475+
escaped += '%22';
476+
lastEscapedPos = i + 1;
472477
break;
473478
case 39: // '\''
474-
if (i - lastPos > 0)
475-
newRest += rest.slice(lastPos, i);
476-
newRest += '%27';
477-
lastPos = i + 1;
479+
if (i > lastEscapedPos)
480+
escaped += rest.slice(lastEscapedPos, i);
481+
escaped += '%27';
482+
lastEscapedPos = i + 1;
478483
break;
479484
case 60: // '<'
480-
if (i - lastPos > 0)
481-
newRest += rest.slice(lastPos, i);
482-
newRest += '%3C';
483-
lastPos = i + 1;
485+
if (i > lastEscapedPos)
486+
escaped += rest.slice(lastEscapedPos, i);
487+
escaped += '%3C';
488+
lastEscapedPos = i + 1;
484489
break;
485490
case 62: // '>'
486-
if (i - lastPos > 0)
487-
newRest += rest.slice(lastPos, i);
488-
newRest += '%3E';
489-
lastPos = i + 1;
491+
if (i > lastEscapedPos)
492+
escaped += rest.slice(lastEscapedPos, i);
493+
escaped += '%3E';
494+
lastEscapedPos = i + 1;
490495
break;
491496
case 92: // '\\'
492-
if (i - lastPos > 0)
493-
newRest += rest.slice(lastPos, i);
494-
newRest += '%5C';
495-
lastPos = i + 1;
497+
if (i > lastEscapedPos)
498+
escaped += rest.slice(lastEscapedPos, i);
499+
escaped += '%5C';
500+
lastEscapedPos = i + 1;
496501
break;
497502
case 94: // '^'
498-
if (i - lastPos > 0)
499-
newRest += rest.slice(lastPos, i);
500-
newRest += '%5E';
501-
lastPos = i + 1;
503+
if (i > lastEscapedPos)
504+
escaped += rest.slice(lastEscapedPos, i);
505+
escaped += '%5E';
506+
lastEscapedPos = i + 1;
502507
break;
503508
case 96: // '`'
504-
if (i - lastPos > 0)
505-
newRest += rest.slice(lastPos, i);
506-
newRest += '%60';
507-
lastPos = i + 1;
509+
if (i > lastEscapedPos)
510+
escaped += rest.slice(lastEscapedPos, i);
511+
escaped += '%60';
512+
lastEscapedPos = i + 1;
508513
break;
509514
case 123: // '{'
510-
if (i - lastPos > 0)
511-
newRest += rest.slice(lastPos, i);
512-
newRest += '%7B';
513-
lastPos = i + 1;
515+
if (i > lastEscapedPos)
516+
escaped += rest.slice(lastEscapedPos, i);
517+
escaped += '%7B';
518+
lastEscapedPos = i + 1;
514519
break;
515520
case 124: // '|'
516-
if (i - lastPos > 0)
517-
newRest += rest.slice(lastPos, i);
518-
newRest += '%7C';
519-
lastPos = i + 1;
521+
if (i > lastEscapedPos)
522+
escaped += rest.slice(lastEscapedPos, i);
523+
escaped += '%7C';
524+
lastEscapedPos = i + 1;
520525
break;
521526
case 125: // '}'
522-
if (i - lastPos > 0)
523-
newRest += rest.slice(lastPos, i);
524-
newRest += '%7D';
525-
lastPos = i + 1;
527+
if (i > lastEscapedPos)
528+
escaped += rest.slice(lastEscapedPos, i);
529+
escaped += '%7D';
530+
lastEscapedPos = i + 1;
526531
break;
527532
}
528533
}
529-
if (lastPos === 0)
534+
if (lastEscapedPos === 0) // Nothing has been escaped.
530535
return;
531-
if (lastPos < rest.length)
532-
return newRest + rest.slice(lastPos);
533-
else
534-
return newRest;
536+
// There are ordinary characters at the end.
537+
if (lastEscapedPos < rest.length)
538+
return escaped + rest.slice(lastEscapedPos);
539+
else // The last character is escaped.
540+
return escaped;
535541
}
536542

537543
// format a parsed object into a url string

test/parallel/test-url.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -833,6 +833,20 @@ var parseTests = {
833833
query: '@c'
834834
},
835835

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

0 commit comments

Comments
 (0)