Skip to content

Preserve blank parameters in fragment identifier matching - #838

Merged
markstory merged 1 commit into
getsentry:masterfrom
rusetiq:fix-blank-fragment-parameters
Oct 6, 2026
Merged

markstory merged 1 commit into
getsentry:masterfrom
rusetiq:fix-blank-fragment-parameters

Conversation

@rusetiq

@rusetiq rusetiq commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

What type of PR is this?

  • Bug Fix

Description

fragment_identifier_matcher('flag=&value=1') currently accepts #value=1, and fragment_identifier_matcher('flag=') accepts #other= or no fragment. parse_qsl drops blank values by default, so these distinct fragments become equal after parsing.

Retain blank values when parsing both expected and actual query-string-style fragments. Parameter order remains insignificant, while blank parameter names and multiplicity are checked. Opaque fragment matching is unchanged.

Add eight direct matcher cases and a requests integration test covering missing, different, reordered, and duplicate parameters. Five regression cases fail before the fix; all pass afterward.

Related Issues

No existing issue identified for this specific case.

PR checklist

  • Read the contributing guidelines
  • Ran tox and pre-commit checks locally
  • Added my changes to CHANGES

Validation on macOS / Python 3.13.14:

  • Ran the pytest command from tox: python -m pytest . --asyncio-mode=auto --cov responses --cov-report term-missing -q: 261 passed, 99% overall coverage.
  • All configured pre-commit hooks passed for the changed files.
  • Both configured mypy invocations were run with mypy 2.4.0. They report three existing diagnostics (two in matchers.py and one in test_recorder.py), also reproduced on an unchanged archive of the upstream commit. This change adds none.
  • git diff --check passed.

The complete multi-version tox matrix was not run locally.

Added/updated tests?

  • Yes

This contribution was prepared with assistance from OpenAI Codex.

@rusetiq
rusetiq requested a review from markstory as a code owner October 5, 2026 18:41
@markstory
markstory merged commit e68019c into getsentry:master Oct 6, 2026
23 of 24 checks passed
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