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: allow handlers to modify the http.Request #27277

Closed
nhooyr opened this issue Aug 27, 2018 · 4 comments

Comments

Projects
None yet
5 participants
@nhooyr
Copy link
Contributor

commented Aug 27, 2018

The docs say

Except for reading the body, handlers should not modify the provided Request.

At the moment, it seems like modifying the provided http request is safe and this guarantee is restrictive but not useful.

In particular, following this guarantee prevents the subrouting approach presented in https://blog.merovius.de/2017/06/18/how-not-to-use-an-http-router.html as the *http.Request's URL is modified when sending it to sub handlers so that they can route the remaining part of the URL on their own. The alternative would be to create a shallow copy of the request every time and then modifying the URL which can become expensive, as there would be a shallow copy for every single segment of the path.

@FiloSottile

This comment has been minimized.

Copy link
Member

commented Aug 30, 2018

@bradfitz can answer properly, but I believe the point is to avoid a situation where we can't look at the Request after it has been handled, and once we allow changes there's no going back. I find the performance argument against shallow copies weak, but I'll leave this for Brad.

@FiloSottile FiloSottile changed the title net/http: purpose of requiring that handlers not modify the http.Request? net/http: allow handlers to modify the http.Request Aug 30, 2018

@FiloSottile FiloSottile added this to the Go1.12 milestone Aug 30, 2018

@carlmjohnson

This comment has been minimized.

Copy link
Contributor

commented Aug 31, 2018

@Merovius, what do you think about changing the blog post to use a shallow copy of the request?

@Merovius

This comment has been minimized.

Copy link

commented Aug 31, 2018

A shallow copy wouldn't help (URL is a pointer). You would have to use a deep copy. Deep copies might not be too expensive to do on every request, but they are too expensive to do in every frame IMO. And personally, I think if Go ever starts relying on Request not being modified, tons and tons of Go code out there will break silently, for better or for worse (note, that the comment was only added in 2016). So, overall, I thought about this when writing the article but decided to simply not care. It might be worth mentioning the tradeoff though.

FWIW, there is another reason not to modify the URL, which is that you need the full path for relative redirects (blegh). I've been thinking about how to address that for a while. I don't have a really nice solution yet - personally, I use a modified ResponseWriter for redirects that takes this into account, but that comes with its own set of problems.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Sep 26, 2018

At the moment, it seems like modifying the provided http request is safe ...

I'm pretty sure that's not true. We only add these docs in response to real problems. At this point I don't think we can just change the semantics here - lots of real things will break, even if no real thing you use will break.

We clearly can't change this without breaking a lot of existing code.

@rsc rsc closed this Sep 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.