Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/http: add AllowQuerySemicolons [freeze exception] #45973

Closed
rsc opened this issue May 5, 2021 · 5 comments
Closed

net/http: add AllowQuerySemicolons [freeze exception] #45973

rsc opened this issue May 5, 2021 · 5 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted release-blocker
Milestone

Comments

@rsc
Copy link
Contributor

rsc commented May 5, 2021

Over in #25192 (comment), @AGWA makes the suggestion to add to net/http a standard Handler wrapper to allow semicolons for people who want to opt into that. The function would be something like:

func AllowQuerySemicolons(h http.Handler) http.Handler {
        return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
                if r.URL.RawQuery != "" {
                        r2 := new(http.Request)
                        *r2 = *r
                        r2.URL = new(url.URL)
                        *r2.URL = *r.URL
                        r2.URL.RawQuery = strings.ReplaceAll(r.URL.RawQuery, ";", "&")
                        h.ServeHTTP(w, r2)
                } else {
                        h.ServeHTTP(w, r)
                }
        })
}

It seems like a nice way to reduce the pain of the semicolon change itself in #25192. We should consider adding it for Go 1.17 (even though we are in the freeze).

@rsc
Copy link
Contributor Author

rsc commented May 5, 2021

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor Author

rsc commented May 12, 2021

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc rsc moved this from Active to Likely Accept in Proposals (old) May 12, 2021
@rsc
Copy link
Contributor Author

rsc commented May 19, 2021

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc moved this from Likely Accept to Accepted in Proposals (old) May 19, 2021
@rsc rsc changed the title proposal: net/http: add AllowQuerySemicolons net/http: add AllowQuerySemicolons May 19, 2021
@rsc rsc modified the milestones: Proposal, Backlog May 19, 2021
@rsc rsc modified the milestones: Backlog, Go1.17 Jun 7, 2021
@FiloSottile FiloSottile changed the title net/http: add AllowQuerySemicolons net/http: add AllowQuerySemicolons [freeze exception] Jun 7, 2021
@FiloSottile FiloSottile added NeedsFix The path to resolution is known, but the work has not been done. and removed Proposal-FinalCommentPeriod labels Jun 7, 2021
@dmitshur
Copy link
Contributor

dmitshur commented Jun 7, 2021

Thank you for making this freeze exception request. It is approved.

@gopherbot
Copy link

Change https://golang.org/cl/326309 mentions this issue: net/http: add AllowQuerySemicolons

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted release-blocker
Projects
No open projects
Development

No branches or pull requests

4 participants