fix: match sequence-valued params in query_param_matcher - #816
fix: match sequence-valued params in query_param_matcher#816dylanpulver wants to merge 2 commits into
Conversation
query_param_matcher converted int/float values to strings only at the top
level of the params mapping, so a list- or tuple-valued param never matched.
responses parses the request's query string into strings and groups a
repeated key into a list (collapsing a key seen once to a bare value), so
{"ids": [1, 2]} was compared against {'ids': ['1', '2']} and failed, as did
any tuple and any single-item sequence.
Normalize sequence values the same way the request parser produces them.
An empty sequence still does not match a request that omits the key.
3c2cb47 to
0c371a5
Compare
| values = [ | ||
| str(item) if isinstance(item, (int, float)) else item for item in value | ||
| ] | ||
| return values[0] if len(values) == 1 else values |
There was a problem hiding this comment.
Don't you also need to recurse into nested structures? Some one could make multi-level query parameter string data that won't have the same normalization applied?
| [ | ||
| {"ids": [1, 2]}, | ||
| {"ids": (1, 2)}, | ||
| {"ids": ["a", 2], "page": 1}, |
There was a problem hiding this comment.
| {"ids": ["a", 2], "page": 1}, | |
| {"ids": ["a", 2], "page": 1}, | |
| {"ids": [1, 2], "attrs": {"counter": 3, "favorites": [1, 2]}}, |
Shouldn't data like this also work?
requests builds the query string by iterating a sequence value once and then
handing the result to urlencode(..., doseq=True), which splices a sequence
nested inside that value in one further level. It also drops a None element.
So {"ids": [[1, 2], [3]]} is sent as ids=1&ids=2&ids=3 and {"ids": [1, None]}
as ids=1, neither of which the matcher's expectation reproduced.
Mirror both rules. Nesting deeper than one level is url-encoded by requests as
its str(), and a dict value is emitted by requests as its keys alone (the
values never reach the wire), so neither is normalized here.
Co-Authored-By: Claude <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 264057f. Configure here.
| # ``requests`` omits a ``None`` element from the query string. | ||
| continue | ||
| if isinstance(item, (list, tuple)): | ||
| values.extend(_stringify(nested) for nested in item) |
There was a problem hiding this comment.
Nested None not stringified
Low Severity
When _normalize flattens a nested sequence, each item goes through _stringify, which only converts int and float. A None inside that nested sequence stays None, while urlencode with doseq emits the string 'None', so the matcher misses a request built from the same params dict.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 264057f. Configure here.
2 participants
What type of PR is this? (check all applicable)
Description
query_param_matcherstringifiedint/floatvalues only at the top level of themapping, so a sequence-valued param never matched:
responsesparses the request's query string into strings and groups a repeated keyinto a list, collapsing a key seen once to a bare value, so the matcher compared
{"ids": [1, 2]}against{'ids': ['1', '2']}. Tuples and single-item sequencesfailed the same way even with string items. Sequences are now normalized the way that
parser produces them.
Deliberately unchanged: an empty sequence, and
None, still fail to match a requestthat omits the key.
requestsdrops both, but dropping the key here would also changewhat
strict_match=Falsefilters on.Checked with an oracle that reuses one dict for both sides across 78 param shapes and
never reads
matchers.py: 56 failures on master, 2 after, both the empty-sequence caseabove. Reverting only
matchers.pyfails all 5 new cases, so none of them is a pin.Suite goes 233 → 238 passed; coverage and
mypyfailure sets are identical to master.Related Issues
PR checklist
Before submitting this pull request, I have done the following:
toxandpre-commitchecks locally —pre-commit run(all hooks, pinnedversions) is green and both
mypyinvocations report the same errors as cleanmaster; pytest ran on 3.13 only, not the full tox matrix.
Added/updated tests?
Disclosure: prepared with AI assistance (Claude Code, Claude Opus 5,
claude-opus-5). I have reviewed the change and the measurements above.