Accept array-style keys in from_query_string#577
Conversation
from_query_string raised ValueError on valid query strings with array-style keys such as a[]=1&a[]=2 or user[name]=joe. The format gate used a hardcoded character whitelist that omitted [ and ], so any bracketed key was rejected before parse_qs was called. Replace the whitelist with a structural check (each &-segment is key=value with a non-empty key and no raw whitespace), which accepts bracketed/array keys while still rejecting TOML/YAML/JSON/XML and plain text. Adds tests for array-style and bracketed keys.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #577 +/- ##
=======================================
Coverage 98.32% 98.32%
=======================================
Files 64 64
Lines 2392 2392
=======================================
Hits 2352 2352
Misses 40 40
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This pull request updates the query-string deserialization logic to accept common “array-style” / bracketed keys (e.g., a[], user[name]) that are valid in real-world query strings but were previously rejected by an overly restrictive key/value character whitelist.
Changes:
- Replaces the
QueryStringSerializer.decodewhitelist regex gate with a structuralkey=value-pair validation that allows bracketed keys. - Adds test coverage for array-style (
a[]=1&a[]=2) and bracketed (user[name]=...) keys inIODict.from_query_string.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/dicts/io/test_io_dict_query_string.py | Adds new query-string parsing tests for array-style and bracketed keys. |
| benedict/serializers/query_string.py | Updates query-string decoding validation to allow bracketed/array-style keys before calling parse_qs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pair_re = re.compile(r"^[^\s=&]+=[^\s&]*$") | ||
| pairs = s.split("&") | ||
| if all(pair_re.match(pair) for pair in pairs): |
| d = IODict.from_query_string(s) | ||
| self.assertTrue(isinstance(d, dict)) | ||
| self.assertEqual(d, r) | ||
|
|
Problem
from_query_stringrejects valid query strings that use array-style keys, whichurllib.parse.parse_qsparses without complaint:The
decodegate inQueryStringSerializerused a hardcoded character whitelist ([\w\-\%\+\.\|]) for keys and values, which omits[and](and other valid characters), so any bracketed/array key fails the gate beforeparse_qsis ever called. Array-style keys are common (PHP and HTML-form syntax).Fix
Replace the opaque whitelist with a structural check: split on
&and require every segment to bekey=valuewhere the key is non-empty and neither side contains raw whitespace,=(key) or&. This accepts bracketed/array keys while still rejecting the other formats benedict's autodetect must distinguish (verified against the repo's own fixtures: TOML, YAML, JSON, XML, plain text and URLs are still rejected).Tests
Added cases for array-style (
a[]=1&a[]=2) and bracketed (user[name]=joe&user[age]=42) keys via bothIODict.from_query_stringandIODict(s, format="query_string"). Without the fix they raiseValueError; with it the query-string tests pass and the broader IO/serializer suite is green (279 passed locally).ruff check/formatclean.Disclosure: I used AI assistance (Claude) to help locate and draft this fix, under my direction and review.