Support http.AllowQuerySemicolons #2248
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@JoelSpeed I've cleaned up the branch and made the changes you suggested that I can. If you still want the chaining (#2076 (comment)) then please give me some direction as to where you want that. Thanks |
|
@tuunit I think chaining is still desired as this would make future additions easier. Can you point @timwsuqld in the right direction? |
JoelSpeed
reviewed
Nov 18, 2023
19f6ab0 to
87ec65b
Compare
87ec65b to
43ddcc8
Compare
JoelSpeed
approved these changes
Nov 20, 2023
tuunit
pushed a commit
to tuunit/oauth2-proxy
that referenced
this pull request
Nov 20, 2023
* Support http.AllowQuerySemicolons * Docs * Make it clear we are overriding the handler * Update documentation for allow-query-semicolons * Fix changelog format * Fix formatting --------- Co-authored-by: MickMake <github@mickmake.com>
tuunit
pushed a commit
to tuunit/oauth2-proxy
that referenced
this pull request
Nov 20, 2023
* Support http.AllowQuerySemicolons * Docs * Make it clear we are overriding the handler * Update documentation for allow-query-semicolons * Fix changelog format * Fix formatting --------- Co-authored-by: MickMake <github@mickmake.com>
|
@JoelSpeed thanks for getting this merged! 👍 |
tuunit
pushed a commit
to tuunit/oauth2-proxy
that referenced
this pull request
Dec 17, 2023
* Support http.AllowQuerySemicolons * Docs * Make it clear we are overriding the handler * Update documentation for allow-query-semicolons * Fix changelog format * Fix formatting --------- Co-authored-by: MickMake <github@mickmake.com>
tuunit
pushed a commit
to tuunit/oauth2-proxy
that referenced
this pull request
Dec 18, 2023
* Support http.AllowQuerySemicolons * Docs * Make it clear we are overriding the handler * Update documentation for allow-query-semicolons * Fix changelog format * Fix formatting --------- Co-authored-by: MickMake <github@mickmake.com>
JoelSpeed
added a commit
that referenced
this pull request
Dec 18, 2023
* Add GitHub groups (orgs/teams) support * align code of getTeams with getOrgs to support Github Enterprise Server instances with different domain * add documentation * add missing import after rebase * add nightly build and push (#2297) * add nightly build and push * add date based nightly build tags * only keep single multiarch image build and push * add changelog * add images to internal docs static files * add docu for nightly builds * remove unnecessary spaces * update nightly repository * Issue 978: Fix Custom cookie name breaks redis for session (#1949) * Issue 978: Fix Custom cookie name breaks redis for session (see #978) * Issue 978: Fix Custom cookie name breaks redis for session (see #978) * Update CHANGELOG.md * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Update CHANGELOG.md --------- Co-authored-by: Nuno Borges <Nuno.Borges@ctw.bmwgroup.com> Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> * Support http.AllowQuerySemicolons (#2248) * Support http.AllowQuerySemicolons * Docs * Make it clear we are overriding the handler * Update documentation for allow-query-semicolons * Fix changelog format * Fix formatting --------- Co-authored-by: MickMake <github@mickmake.com> * Add GitHub groups (orgs/teams) support * align code of getTeams with getOrgs to support Github Enterprise Server instances with different domain * add documentation * fix changelog & documentation * fix missing import --------- Co-authored-by: Tobias Mayer <github@tobiasm.de> Co-authored-by: Nuno Miguel Micaelo Borges <miguelborges99@gmail.com> Co-authored-by: Nuno Borges <Nuno.Borges@ctw.bmwgroup.com> Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> Co-authored-by: Tim White <tim.white@su.org.au> Co-authored-by: MickMake <github@mickmake.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Provide support for using semicolons in query strings. This adds a new bool flag --allow-query-semicolons.
Current behavior causes issues with software that uses
;as a query parameter separator. golang.org/issue/25192 (and some other places online) believe that this is a security issue as proxies and the backend software treat the;differently, which can cause strange behaviour or security issues. For this reason, we should only be enabling https://pkg.go.dev/net/http#AllowQuerySemicolons when the backend software requires;as a query separator, hence why this should be a flag and not enabled by default.(Continuation of #2076)
Motivation and Context
See Issue #1841
How Has This Been Tested?
Tested against OTRS (Znuny), a perl system that still uses
;as an argument separator.Checklist: