Skip to content

querystring: fix empty pairs handling

Rodrigo Muino Tomonari requested to merge github/fork/mscdex/querystring-fix-empty-pairs-handling into master

This is a follow up to https://github.com/nodejs/node/pull/10967, which is (currently) a semver-major.

This PR fixes outstanding issues stemming from the aforementioned PR and includes the tests from https://github.com/nodejs/node/pull/11171. FWIW the original issue for all of this was that empty query string key/value pairs before the end of the string were not being ignored like an empty key/value at the end of the string would be.

I've also included some improvements to the parser in general, along with a couple of new benchmarks to highlight some of those improvements. Here are the results for benchmark/querystring/querystring-parse.js:

                                                                  improvement confidence      p.value
 querystring/querystring-parse.js n=1000000 type="altspaces"          84.35 %        *** 1.191823e-35
 querystring/querystring-parse.js n=1000000 type="encodefake"          2.96 %          * 4.610320e-02
 querystring/querystring-parse.js n=1000000 type="encodelast"          4.78 %         ** 7.673085e-03
 querystring/querystring-parse.js n=1000000 type="encodemany"          2.83 %            5.099655e-02
 querystring/querystring-parse.js n=1000000 type="manyblankpairs"    192.66 %        *** 4.461978e-48
 querystring/querystring-parse.js n=1000000 type="manypairs"          -0.14 %            9.307657e-01
 querystring/querystring-parse.js n=1000000 type="multicharsep"        1.65 %            2.795571e-01
 querystring/querystring-parse.js n=1000000 type="multivalue"          1.40 %            3.885362e-01
 querystring/querystring-parse.js n=1000000 type="multivaluemany"      1.82 %            4.116370e-01
 querystring/querystring-parse.js n=1000000 type="noencode"           -0.45 %            8.223732e-01

CI: https://ci.nodejs.org/job/node-test-pull-request/6279/

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)
  • querystring

Merge request reports