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/httptest: ResponseRecorder does not mimic real behavior of WriteHeader() #8857

Closed
scottferg opened this issue Oct 2, 2014 · 5 comments
Milestone

Comments

@scottferg
Copy link
Contributor

httptest.ResponseRecorder does not mimic the behavior of the net/http package when
headers are written after a call to WriteHeader().

Per the documentation for http.ResponseWriter:

    Header returns the header map that will be sent by WriteHeader.
    Changing the header after a call to WriteHeader (or Write) has
    no effect.

However, if you change the header after a call to WriteHeader while using
httptest.ResponseRecorder the result will appear as though the header was set (and sent)
correctly. 

This appears to be caused by the fact that in this function:

// Header returns the response headers.
func (rw *ResponseRecorder) Header() http.Header {
        m := rw.HeaderMap
        if m == nil {
                m = make(http.Header)
                rw.HeaderMap = m
        }
        return m
}

there is no check to see if rw.wroteHeader is true. Anything set before or after
`rw.wroteHeader = true` will still show up in this map.


What steps reproduce the problem?
If possible, include a link to a program on play.golang.org.

1. In an http.HandlerFunc, call rw.Header().Set("Some-Header",
"Some-Value") after a call to rw.WriteHeader(statuscode)
2. Call this handler by passing in an httptest.ResponseRecorder

What happened?

recorder.HeaderMap["Some-Header"][0] will be set to "Some-Value"

What should have happened instead?

"Some-Header" should not have been set
@ianlancetaylor
Copy link
Contributor

Comment 1:

Labels changed: added repo-main, release-none.

@bradfitz bradfitz removed the new label Dec 18, 2014
@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@gopherbot
Copy link

CL https://golang.org/cl/10373 mentions this issue.

@bradfitz bradfitz modified the milestones: Go1.7, Unplanned Feb 5, 2016
@bradfitz bradfitz self-assigned this Feb 5, 2016
@bradfitz
Copy link
Contributor

bradfitz commented Feb 5, 2016

It's been requested that we fix this for the next release.

Unfortunately the CL above was never updated so didn't make it into Go 1.6.

@gopherbot
Copy link

CL https://golang.org/cl/20047 mentions this issue.

@gopherbot
Copy link

CL https://golang.org/cl/23257 mentions this issue.

gopherbot pushed a commit that referenced this issue May 19, 2016
In Go versions 1 up to and including Go 1.6,
ResponseRecorder.HeaderMap was both the map that handlers got access
to, and was the map tests checked their results against. That did not
mimic the behavior of the real HTTP server (Issue #8857), so HeaderMap
was changed to be a snapshot at the first write in
https://golang.org/cl/20047. But that broke cases where the Handler
never did a write (#15560), so revert the behavior.

Instead, introduce the ResponseWriter.Result method, returning an
*http.Response. It subsumes ResponseWriter.Trailers which was added
for Go 1.7 in CL 20047. Result().Header now contains the correct
answer, and HeaderMap is unchanged in behavior from previous Go
releases, so we don't break people's tests. People wanting the correct
behavior can use ResponseWriter.Result.

Fixes #15560
Updates #8857

Change-Id: I7ea9b56a6b843103784553d67f67847b5315b3d2
Reviewed-on: https://go-review.googlesource.com/23257
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators May 19, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants