Skip to content

Commit 904606d

Browse files
committed
querystring: rebalance performance of querystring.unescape
1 parent db8217b commit 904606d

File tree

3 files changed

+65
-6
lines changed

3 files changed

+65
-6
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
'use strict';
2+
const common = require('../common.js');
3+
const querystring = require('querystring');
4+
5+
const bench = common.createBenchmark(main, {
6+
input: [
7+
'there is nothing to unescape here',
8+
'there+are+spaces+to+unescape+here',
9+
'there%20are%20several%20spaces%20that%20need%20to%20be%20unescaped',
10+
'%20%21%22%23%24%25%26%27%28%29%2A%2B%2C%2D%2E%2F%30%31%32%33%34%35%36%37',
11+
'there%2Qare%0-fake%escaped values in%%%%this%9Hstring',
12+
'there%2Qare%0-fake%escaped%20values in%%%%this%9Hstring',
13+
'%%2a',
14+
'%2sf%2a',
15+
'%2%2af%2a',
16+
],
17+
n: [10e6],
18+
decodeSpaces: [1, 0],
19+
});
20+
21+
function main({ input, n, decodeSpaces }) {
22+
decodeSpaces = !!decodeSpaces;
23+
bench.start();
24+
for (let i = 0; i < n; i += 1)
25+
querystring.unescape(input, decodeSpaces);
26+
bench.end(n);
27+
}

lib/querystring.js

+34-6
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,15 @@ const {
2828
ArrayIsArray,
2929
Int8Array,
3030
MathAbs,
31+
NumberParseInt,
3132
NumberIsFinite,
3233
ObjectKeys,
34+
RegExpPrototypeExec,
3335
String,
36+
StringFromCharCode,
3437
StringPrototypeCharCodeAt,
38+
StringPrototypeIndexOf,
39+
StringPrototypeReplace,
3540
StringPrototypeSlice,
3641
decodeURIComponent,
3742
} = primordials;
@@ -125,17 +130,40 @@ function unescapeBuffer(s, decodeSpaces) {
125130

126131
/**
127132
* @param {string} s
128-
* @param {boolean} decodeSpaces
133+
* @param {boolean} [decodeSpaces=false]
129134
* @returns {string}
130135
*/
131-
function qsUnescape(s, decodeSpaces) {
132-
try {
133-
return decodeURIComponent(s);
134-
} catch {
135-
return QueryString.unescapeBuffer(s, decodeSpaces).toString();
136+
function qsUnescape(s, decodeSpaces = false) {
137+
if (StringPrototypeIndexOf(s, '%') === -1) {
138+
return s;
139+
}
140+
141+
if (RegExpPrototypeExec(MALFORMED_URI_REGEX, s) !== null) {
142+
if (decodeSpaces) {
143+
return StringPrototypeReplace(s, decodeSpacesRegexp, decodeSpacesReplacer);
144+
}
145+
return StringPrototypeReplace(s, baseRegexp, baseReplacer);
146+
136147
}
148+
return decodeURIComponent(s);
149+
137150
}
138151

152+
const MALFORMED_URI_REGEX = /(?:%(?:[^0-9a-fA-F]|[0-9a-fA-F][^0-9a-fA-F]))/u;
153+
154+
const baseRegexp = /(%[\da-fA-F]{2})/gu;
155+
156+
function baseReplacer(match) {
157+
return StringFromCharCode(NumberParseInt(match[1] + match[2], 16));
158+
}
159+
160+
const decodeSpacesRegexp = /(%[\da-fA-F]{2}|[+])/gu;
161+
162+
function decodeSpacesReplacer(match) {
163+
return match === '+' ?
164+
' ' :
165+
StringFromCharCode(NumberParseInt(match[1] + match[2], 16));
166+
}
139167

140168
// These characters do not need escaping when generating query strings:
141169
// ! - . _ ~

test/parallel/test-querystring.js

+4
Original file line numberDiff line numberDiff line change
@@ -170,10 +170,14 @@ const qsNoMungeTestCases = [
170170
const qsUnescapeTestCases = [
171171
['there is nothing to unescape here',
172172
'there is nothing to unescape here'],
173+
['there+is+nothing+to+unescape+here',
174+
'there+is+nothing+to+unescape+here'],
173175
['there%20are%20several%20spaces%20that%20need%20to%20be%20unescaped',
174176
'there are several spaces that need to be unescaped'],
175177
['there%2Qare%0-fake%escaped values in%%%%this%9Hstring',
176178
'there%2Qare%0-fake%escaped values in%%%%this%9Hstring'],
179+
['there%2Qare%0-fake%escaped%20values in%%%%this%9Hstring',
180+
'there%2Qare%0-fake%escaped values in%%%%this%9Hstring'],
177181
['%20%21%22%23%24%25%26%27%28%29%2A%2B%2C%2D%2E%2F%30%31%32%33%34%35%36%37',
178182
' !"#$%&\'()*+,-./01234567'],
179183
['%%2a', '%*'],

0 commit comments

Comments
 (0)