qs icon indicating copy to clipboard operation
qs copied to clipboard

Fix parsing of keys containing literal `[]` inside bracket groups

Open techouse opened this issue 6 months ago • 3 comments

Problem

qs splits bracket groups with a regex that forbids [] inside a group:

var child = /(\[[^[][^\]]*])/g; // original: /(\[[^[\]]*])/g

This causes keys like search[withbracket[]] to split into "[withbracket]", then "[]", leaving the inner [] detached from the literal segment. Downstream, it can no longer be treated as a single literal key.

Repro

// expected:
qs.parse('search[withbracket[]]=foobar');
// => { search: { 'withbracket[]': 'foobar' } }

// actual (today):
// inner [] is split out, so the key can’t be treated as one literal segment

Fix approach

Backport splitKeyIntoSegments from qs_dart and replace the regex child matcher with a balanced bracket splitter that treats [] inside a bracket group as literal content (only outer [] act as array push). This preserves withbracket[] as a single segment.

Notes

  • Does not change existing semantics for [] at the outermost level (array push).
  • Honors existing options: allowDots, depth, strictDepth, allowPrototypes, etc.

Test snippet

t.test('parses keys with literal [] inside a bracket group (#493)', function (st) {
    // A bracket pair inside a bracket group should be treated literally as part of the key
    st.deepEqual(
        qs.parse('search[withbracket[]]=foobar'),
        { search: { 'withbracket[]': 'foobar' } },
        'treats inner [] literally when inside a bracket group'
    );

    // Single-level variant
    st.deepEqual(
        qs.parse('a[b[]]=c'),
        { a: { 'b[]': 'c' } },
        'keeps "b[]" as a literal key'
    );

    // Nested with an array push on the outer level
    st.deepEqual(
        qs.parse('list[][x[]]=y'),
        { list: [{ 'x[]': 'y' }] },
        'preserves inner [] while still treating outer [] as array push'
    );

    st.end();
});

Fixes https://github.com/ljharb/qs/issues/493

techouse avatar Aug 26 '25 23:08 techouse

@ljharb, it seems some of the tests fail because the runners are unable to download a specific Node version, i.e.

curl: (56) OpenSSL SSL_read: Connection reset by peer, errno 104
download from https://nodejs.org/dist/v5.3.0/node-v5.3.0-linux-x64.tar.xz failed
grep: /home/runner/.nvm/.cache/bin/node-v5.3.0-linux-x64/node-v5.3.0-linux-x64.tar.xz: No such file or directory
curl: (92) HTTP/2 stream 0 was not closed cleanly: INTERNAL_ERROR (err 2)
download from https://nodejs.org/dist/v13.13.0/node-v13.13.0-linux-x64.tar.xz failed
grep: /home/runner/.nvm/.cache/bin/node-v13.13.0-linux-x64/node-v13.13.0-linux-x64.tar.xz: No such file or directory

techouse avatar Aug 28 '25 08:08 techouse

@ljharb, have you had a chance to look at this yet? 😊

techouse avatar Sep 30 '25 07:09 techouse

This is pretty hard to review. Could you perhaps make a refactoring commit that makes the majority of the changes, and then a fix commit with the test and the thing that makes the test pass?

@ljharb done. 😇

I have split the parse key processing into a refactor-only commit followed by the nested-bracket fix plus its regression tests so it now has the structure you have asked for.

techouse avatar Nov 04 '25 08:11 techouse