Skip to content

fix: keep blank-valued query params significant in query_string_matcher - #804

Merged
markstory merged 2 commits into
getsentry:masterfrom
chuenchen309:fix/query-string-matcher-blank-values
Jul 17, 2026
Merged

markstory merged 2 commits into
getsentry:masterfrom
chuenchen309:fix/query-string-matcher-blank-values

Conversation

@chuenchen309

@chuenchen309 chuenchen309 commented Jul 16, 2026 •

Copy link
Copy Markdown
Contributor

query_string_matcher parses both the incoming request query and the configured query with parse_qsl() at its default keep_blank_values=False, so any empty-valued parameter (b=, or a bare b) is dropped before comparison:

>>> parse_qsl("a=1&b=")
[('a', '1')]          # b= vanished

This matcher is auto-added for any registered URL carrying a query string (BaseResponse._should_match_querystring), so blank params are silently ignored on both sides of the default path:

responses.add(responses.GET, "http://example.com/?a=1", body="M")
requests.get("http://example.com/?a=1&b=")   # wrongly matches (the extra b= is ignored)

It's also inconsistent with the request-param parser in responses/__init__.py, which uses keep_blank_values=True — so request.params and the explicit query_param_matcher already treat b= as a real parameter.

Fix: parse both sides of query_string_matcher with keep_blank_values=True. Added test_query_string_matcher_blank_values (fails on master, passes with this change).


Disclosure: fully AI-authored (Claude Code, autonomous) — an AI wrote the change and ran the tests; the account operator did not hand-review the diff.

query_string_matcher parsed both the request and the configured query
with parse_qsl()'s default keep_blank_values=False, so a parameter with
an empty value (b=, or a bare b) was dropped before comparison. As a
result ?a=1 matched a request to ?a=1&b=, and a registration that
required b= matched a request that omitted it.

This auto-applies to any registered URL that carries a query string (via
_should_match_querystring), so it silently mismatches on the default
path. The request-param parser already uses keep_blank_values=True
(__init__.py), so query_param_matcher treats b= as significant — the two
query-matching paths disagreed on the same request. Parse both sides
with keep_blank_values=True so blanks are compared, not discarded.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@chuenchen309
chuenchen309 requested a review from markstory as a code owner July 16, 2026 14:48
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.90%. Comparing base (aae1d47) to head (2f8ce93).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #804      +/-   ##
==========================================
+ Coverage   99.84%   99.90%   +0.06%     
==========================================
  Files           9        9              
  Lines        3295     3309      +14     
==========================================
+ Hits         3290     3306      +16     
+ Misses          5        3       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@markstory

Copy link
Copy Markdown
Member

Disclosure: this fix was prepared with AI assistance (Claude). I reproduced the behaviour and ran every test above myself before submitting.

In the future, you should remove the LLM checklists and noise from your pull request descriptions. It makes your contributions look like low-quality changes that are likely to be rejected by open source projects.

@markstory
markstory merged commit da2c5d1 into getsentry:master Jul 17, 2026
27 checks passed
@chuenchen309

Copy link
Copy Markdown
Contributor Author

Thanks Mark, that's fair — I've trimmed the description down to just the bug and the fix. I also corrected the disclosure line: this was AI-authored end to end, so I shouldn't have worded it as if I'd run the tests by hand.

MateoGreil pushed a commit to MateoGreil/python-comwatt-client that referenced this pull request Aug 28, 2026
This PR contains the following updates:

| Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) |
|---|---|---|---|
| [responses](https://github.com/getsentry/responses) ([changelog](https://github.com/getsentry/responses/blob/master/CHANGES)) | `==0.26.2` → `==0.26.3` | ![age](https://developer.mend.io/api/mc/badges/age/pypi/responses/0.26.3?slim=true) | ![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/responses/0.26.2/0.26.3?slim=true) |

---

### Release Notes

<details>
<summary>getsentry/responses (responses)</summary>

### [`v0.26.3`](https://github.com/getsentry/responses/blob/HEAD/CHANGES#0263)

[Compare Source](getsentry/responses@0.26.2...0.26.3)

- Added a `keep_headers` argument to `_recorder.record` and
  `Recorder.dump_to_file` to preserve named headers (for example `Date`)
  that are otherwise stripped as verbose defaults, so a signed response can be
  recorded and later verified. Matching is case-insensitive. See [#&#8203;763](getsentry/responses#763)
- Fixed the element type exposed by `CallList` so static type checkers infer
  `Call` values when iterating, indexing, or filtering recorded calls. See [#&#8203;722](getsentry/responses#722)
- Fixed `query_string_matcher` (and the query matching auto-applied to a
  registered URL's own query string) discarding blank-valued query params
  (`b=`), which caused requests with an extra or missing blank param to
  match incorrectly. See [#&#8203;804](getsentry/responses#804)
- Fixed `fragment_identifier_matcher` treating opaque fragments (those without
  `=`, e.g. `/users/5`) as always equal, so a required fragment matched a
  different one or none at all. See [#&#8203;806](getsentry/responses#806)

</details>

---

### Configuration

📅 **Schedule**: (UTC)

- Branch creation
  - At any time (no schedule defined)
- Automerge
  - At any time (no schedule defined)

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate CLI](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0NC4zMC4zIiwidXBkYXRlZEluVmVyIjoiNDQuMzAuMyIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==-->

Co-authored-by: Renovate Bot <[email protected]>
Reviewed-on: https://git.greil.fr/mat/python-comwatt-client/pulls/55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants