Drop Set-Cookie from the recovered 500 (closes #193)
check / check (push) Successful in 3m35s
check / check (push) Successful in 3m35s
When a handler sets a cookie and then panics before sending anything, the recover middleware now deletes Set-Cookie before writing its 500, so a request that failed never hands the client a credential. Every other header, Location included, is left as http.Error leaves it, matching chi's Recoverer. A response that was already sent is untouched. Tests cover the uncommitted case (no cookie, Location kept) and assert the cookie still reaches the client when the response was committed before the panic. Model: opus-5-5
This commit was merged in pull request #337.
This commit is contained in:
@@ -304,16 +304,44 @@ func TestRecovererRepanicsErrAbortHandler(t *testing.T) {
|
||||
)
|
||||
}
|
||||
|
||||
// TestRecovererDropsSetCookieFromTheRecovered500 covers a handler that
|
||||
// sets a cookie and a redirect target and then panics before sending
|
||||
// anything. A request that failed must not hand the client a
|
||||
// credential, so the 500 carries no cookie; Location is left alone.
|
||||
func TestRecovererDropsSetCookieFromTheRecovered500(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
probe := newRecovererProbe(
|
||||
t, false,
|
||||
func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.Header().Set("Set-Cookie", "session=x")
|
||||
w.Header().Set("Location", "/after")
|
||||
|
||||
panic(panicMarker)
|
||||
},
|
||||
)
|
||||
|
||||
resp, err := probe.get(t)
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, resp.Body.Close())
|
||||
|
||||
assert.Equal(t, http.StatusInternalServerError, resp.StatusCode)
|
||||
assert.Empty(t, resp.Cookies())
|
||||
assert.Equal(t, "/after", resp.Header.Get("Location"))
|
||||
}
|
||||
|
||||
// TestRecovererKeepsAnAlreadyCommittedResponse covers a handler that
|
||||
// panics after sending its status. The bytes are already on the wire,
|
||||
// so a second WriteHeader would change nothing the client sees and
|
||||
// would draw net/http's "superfluous response.WriteHeader" report.
|
||||
// cookie included, so a second WriteHeader would change nothing the
|
||||
// client sees and would draw net/http's "superfluous
|
||||
// response.WriteHeader" report.
|
||||
func TestRecovererKeepsAnAlreadyCommittedResponse(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
probe := newRecovererProbe(
|
||||
t, false,
|
||||
func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.Header().Set("Set-Cookie", "session=x")
|
||||
w.WriteHeader(committedStatus)
|
||||
_, _ = w.Write([]byte("partial"))
|
||||
|
||||
@@ -331,6 +359,7 @@ func TestRecovererKeepsAnAlreadyCommittedResponse(t *testing.T) {
|
||||
|
||||
assert.Equal(t, committedStatus, resp.StatusCode)
|
||||
assert.Equal(t, "partial", string(body))
|
||||
assert.Len(t, resp.Cookies(), 1)
|
||||
|
||||
record := probe.panicRecord(t)
|
||||
assert.Equal(t, panicMarker, record["panic"])
|
||||
|
||||
Reference in New Issue
Block a user