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

Jump to bottom

proposal: Go 2: net/http/httptest: Alter ResponseRecorder API to make it harder to misuse the header map #25764

Closed
cespare opened this issue Jun 6, 2018 · 2 comments
Closed

proposal: Go 2: net/http/httptest: Alter ResponseRecorder API to make it harder to misuse the header map #25764

cespare opened this issue Jun 6, 2018 · 2 comments
Labels
FrozenDueToAge Proposal v2 A language change or incompatible library change
Milestone
Proposal

Comments

@cespare
Copy link
Contributor

cespare commented Jun 6, 2018

This is follow-up from #8857, #15560, #16717, and #25763.

To quote my summary from #25763:

[If] you write a header after the body has been written and status has been set in an HTTP handler, that's a bug because it has no effect. If you're testing using an httptest.ResponseRecorder, its HeaderMap will record the header and might make you think that your code is working when it's not. This is #8857. In response to that bug the behavior was changed to snapshot the headers instead, but that caused another problem (#15560) so it was reverted.

For Go 2, we should change the ResponseRecorder API to make it impossible for the user to access any header map that may be mutated after Write/WriteHeader. Allowing that makes it too easy to accidentally think headers are being written when they're buggily being set after the body has already been written.

I haven't given the specifics much thought, but maybe ResponseRecorder should be more opaque and only expose its data via Result.

cc @bradfitz
@cespare cespare added the v2 A language change or incompatible library change label Jun 6, 2018
@cespare cespare mentioned this issue Jun 6, 2018
Closed
@gopherbot gopherbot added this to the Proposal milestone Jun 6, 2018
@cespare
Copy link
Contributor Author

cespare commented Jun 10, 2018

Note that besides HeaderMap, it's also easy for the user to call ResponseRecorder.Header to get at internal header map. I don't see how to prevent the user from getting at this with the current ResponseWriter API. Maybe all we can do is delete the HeaderMap field.

@ianlancetaylor
Copy link
Contributor

Folding into the general rethinking of net/http in #5465.

@golang golang locked and limited conversation to collaborators Sep 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal v2 A language change or incompatible library change
Projects
None yet
Milestone
Proposal
Development

No branches or pull requests
3 participants
@cespare @ianlancetaylor @gopherbot