Skip to content

fix: stop treating semicolons as query parameter separators#787

Open
veeceey wants to merge 1 commit into
gorilla:mainfrom
veeceey:fix/remove-semicolon-query-separator
Open

fix: stop treating semicolons as query parameter separators#787
veeceey wants to merge 1 commit into
gorilla:mainfrom
veeceey:fix/remove-semicolon-query-separator

Conversation

@veeceey

@veeceey veeceey commented Feb 10, 2026

Copy link
Copy Markdown

No description provided.

Since Go 1.17, net/url.ParseQuery no longer splits query parameters on
semicolons, in compliance with the URL Living Standard. However,
findFirstQueryKey still used bytes.IndexAny(foundKey, "&;") which split
on both ampersands and semicolons. This created a parser differential
between mux.Vars and net/url that could lead to security vulnerabilities
such as broken access control and web cache poisoning.

Change bytes.IndexAny(foundKey, "&;") to bytes.IndexByte(foundKey, '&')
so that only ampersands are recognized as separators.

Fixes gorilla#781

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@veeceey

veeceey commented Feb 19, 2026

Copy link
Copy Markdown
Author

Friendly ping - any chance someone could take a look at this when they get a chance? Happy to make any changes if needed.

@veeceey

veeceey commented Mar 10, 2026

Copy link
Copy Markdown
Author

just wanted to follow up on this — is there anything blocking it from getting reviewed?

@epaes90 epaes90 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argus Code Review

Score: 100/100

Reviewed 2 files with no issues found. Highlights: Test coverage is present; Changes are focused and well-scoped; No security, performance, or quality issues detected. Score: 100/100.

No inline findings to report.

@themavik themavik left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

findFirstQueryKey now splits only on &, matching net/url after Go 1.17 and fixing the #781 mismatch with ParseQuery. nit: consider one benchmark string where ; appears only inside values so perf still touches that byte class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants