Commit activity
Last 12 weeks · 37 commits
Community health
4 of 6 standards met
Recent PRs & issues
This PR improves the performance by a few orders of magnitude in my benchmarks, mostly by changing O(n^2) code to something linear. There are 2 separate cases and I can separate one of them into a separate PR for easier review. Duplicate keys This is something that was recently noticed in #539, but there had already been some work in unmerged PR's from @elidoran - #185 and #189. The [](https://github.com/ljharb/qs/blob/6bdfaf5b7c33008ff603af7772844579b42f7a3f/lib/utils.js#L290) makes copies of continually larger arrays every time is called. There are (were) tests that check if does not mutate input arguments, but 3086902ecf7f088d0d1803887643ac6c03d415b9 introduced new test cases that check if the first () input is mutated if it is an overflow object. I know that mutating inputs was one of the objections in #185 and #189, but I believe that the performance improvement is large enough to allow it. This PR does not change behaviour, which is wrong (index vs length - #540) and probably should not even be there (https://github.com/ljharb/qs/issues/537#issuecomment-3787759741, #294). This change should slightly reduce memory usage by making less copies (it reduces the number of allocations, but peak memory usage depends on GC; GHSA-6rw7-vpxm-498p mentions memory exhaustion, but it is not clear how could that happen) and makes the performance comparable to 6.14.1 with (in prior versions duplicates are parsed as arrays regardless of and - #543) Memory usage change This was tested by parsing a string with 100000 elements, where each string had 1000 characters - the input is 100 MB of text. Options: 16.4.0 "Allocation" view from "Allocation timeline": This PR (f8ee66f313bb3f602668eab0a72890f51fcf0ede): My benchmark results are variable, because I have a noisy environment (lots of things open, CPU frequency and core assignment not locked), but the general order of magnitude tells the difference. Unfortunately, this may cause regressions with huge arrays, because pushes the arguments (all elements of array in this case) to the stack. I don't think that this is a huge problem given that this happens at over 100k elements that previously would be parsed in about ~30 seconds. When exactly will this fail depends on the available size of the stack and may be hard to predict. Benchmark code Possible input combinations are: is an array, is not an array - is an array, is an array - is not an array, is not an array - is not an array, is an array - - detected only once in whole test suite, so I don't think makes sense This part includes first 2 commits: 2870c9ce3388f10e8302741a31995c6c32bcc4ef - replace with if possible 15b1e5efb7a8406b32a51c99d951986ef8ac0cae - allow mutation of the first argument in tests that did not allow it Indexed arrays Here the problem lies in and the call. When a string like is parsed the function is called with values of and like those below: While the callback of is executed only for non-empty array items in sparse arrays, it looks like it still is linear with the size of array and not the number of existing items. Since the execution time of and the number of calls to scale with N (number of elements in array with growing indices), this has O(n^2) time complexity. The idea that I had is iterating over own properties and using the fact that for arrays they always are the indices of elements followed by . Benchmarks use the same code as before, but the second query is uncommented. I'm not entirely convinced if this is something worth changing, because for reasonable (affects max execution time) and (affects number of calls) values the current code performs slightly better. If there are people who change both of those limits, then this may be a real problem. This part is in the third commit - f8ee66f313bb3f602668eab0a72890f51fcf0ede.
Summary Evidence Before: The supplied Node reproduction returned and changed from to . The new regression test failed on the base revision (805 passed, 1 failed). After: passed 806/806. passed. ESLint reported 0 errors on the two changed files. Merge Danger Door: Two-way Blast Radius: Low The change copies the selected keys before sorting and leaves query output ordering unchanged.
Recent fixes
With , ISO-8859-1 parsing and , returns instead of the documented array. Converting the decoded array with discards its element boundaries before entity interpretation. Use the existing helper to interpret entities per decoded value. Splitting still happens before percent/entity decoding, so and remain content rather than becoming separators. Bracket grouping, repeated keys and array limits then receive the preserved arrays. Fixes #629. Regression tests cover nested/indexed arrays, repeated parameters, empty entries, charset detection, escaped commas, limits and falsy custom decoder results. passes all 1,563 tests, and passes (18 existing warnings) on Node 24.21.0/Linux arm64. The new array regressions fail before the fix.
In the comma splitting happens first (), producing an array, and only afterwards runs on the result, which turns the array back into the string . So the two options cancel each other out. Numeric entities should be interpreted per element (or before the split) instead of stringifying the array. Reproduced on 6.16.1 (main 47cf3f1). Happy to send a PR with a test if you would take one.
With and an of 0 or less, even throws an array-limit error. The value parser's early check compares the current array length against the limit before any array exists, so at a limit of 0 or below it rejects every value, including scalars and named object fields. Apply that early check only when an array already has elements. Arrays that qs builds from the input (bracket and indexed notation, combined duplicates, and comma-split values) are still rejected by the existing checks, and empty arrays remain within the limit. Arrays passed in already built, via object input, are left as-is, matching non-throw mode and every positive . Behavior only changes when is and is 0 or less, and only from throwing to returning what non-throw mode returns. Full test suite passes on node 0.8 through latest; has 100% coverage. Lint passes, and the production dependency audit reports 0 vulnerabilities.
Repository: ljharb/qs. Description: A querystring parser and serializer with nesting support Stars: 8946, Forks: 953. Primary language: JavaScript. Languages: JavaScript (100%). License: BSD-3-Clause. Topics: browsers, encoding, javascript, node, nodejs, parse, querystrings, stringify, url-parsing. Open PRs: 18, open issues: 51. Last activity: 20m ago. Community health: 85%. Top contributors: ljharb, nlf, papandreou, dead-horse, Connormiha, mizozobu, geek, Jokero, tdzienniak, elidoran and others.