[go] net/http/httptest: check the HeaderCode when WriteHeader
ZiZhao Zhang (Gerrit)
ZiZhao Zhang has uploaded this change for review.
net/http/httptest: check the HeaderCode when WriteHeader
fixes #45353
Change-Id: Id5ed4af652e8c150ae86bf50402b800d935e2203
---
M src/net/http/httptest/recorder.go
1 file changed, 19 insertions(+), 0 deletions(-)
diff --git a/src/net/http/httptest/recorder.go b/src/net/http/httptest/recorder.go
index 2428482..1b712ef 100644
--- a/src/net/http/httptest/recorder.go
+++ b/src/net/http/httptest/recorder.go
@@ -122,11 +122,30 @@
return len(str), nil
}
+func checkWriteHeaderCode(code int) {
+ // Issue 22880: require valid WriteHeader status codes.
+ // For now we only enforce that it's three digits.
+ // In the future we might block things over 599 (600 and above aren't defined
+ // at https://httpwg.org/specs/rfc7231.html#status.codes)
+ // and we might block under 200 (once we have more mature 1xx support).
+ // But for now any three digits.
+ //
+ // We used to send "HTTP/1.1 000 0" on the wire in responses but there's
+ // no equivalent bogus thing we can realistically send in HTTP/2,
+ // so we'll consistently panic instead and help people find their bugs
+ // early. (We can't return an error from WriteHeader even if we wanted to.)
+ if code < 100 || code > 999 {
+ panic(fmt.Sprintf("invalid WriteHeader code %v", code))
+ }
+}
+
// WriteHeader implements http.ResponseWriter.
func (rw *ResponseRecorder) WriteHeader(code int) {
if rw.wroteHeader {
return
}
+
+ checkWriteHeaderCode(code)
rw.Code = code
rw.wroteHeader = true
if rw.HeaderMap == nil {
To view, visit change 308950. To unsubscribe, or for help writing mail filters, visit settings.
ZiZhao Zhang (Gerrit)
ZiZhao Zhang uploaded patch set #2 to this change.
net/http/httptest: check the HeaderCode when WriteHeader
Fixes #45353
Change-Id: Id5ed4af652e8c150ae86bf50402b800d935e2203
---
M src/net/http/httptest/recorder.go
1 file changed, 19 insertions(+), 0 deletions(-)
To view, visit change 308950. To unsubscribe, or for help writing mail filters, visit settings.
Emmanuel Odeke (Gerrit)
Attention is currently required from: Brad Fitzpatrick, ZiZhao Zhang, Ian Lance Taylor.
Patch set 2:Run-TryBot +1
2 comments:
Commit Message:
Patch Set #2, Line 7:
net/http/httptest: check the HeaderCode when WriteHeadernet/http/httptest: panic on non-3 digit (XXX) status code in Recorder.WriteHeader
This change conforms Recorder with net/http servers, to panic
when a handler writes a non-3 digit XXX status code.
Patchset:
Thank you ZiZhao for this change, and great to catch you here!
I have added some suggestions to improve the commit message, but also,
we need a test to prevent any regressions in case that code is removed.Please add this test:
// Ensure that httptest.Recorder panics when given a non-3 digit (XXX)
// status HTTP code. See https://golang.org/issues/45353
// Ensure that httptest.Recorder panics when given a non-3 digit (XXX)
// status HTTP code. See https://golang.org/issues/45353
func TestRecorderPanicsOnNonXXXStatusCode(t *testing.T) {
badCodes := []int{
-100, 0, 99, 1000, 20000,
}for _, badCode := range badCodes {
badCode := badCode
t.Run(fmt.Sprintf("Code=%d", badCode), func(t *testing.T) {
defer func() {
if r := recover(); r == nil {
t.Fatal("Expected a panic")
}
}()
handler := func(rw http.ResponseWriter, _ *http.Request) {
rw.WriteHeader(badCode)
}req := httptest.NewRequest("GET", "https://example.org/", nil)
rw := httptest.NewRecorder()
handler(rw, req)
})
}
}
To view, visit change 308950. To unsubscribe, or for help writing mail filters, visit settings.
ZiZhao Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, ZiZhao Zhang, Ian Lance Taylor.
ZiZhao Zhang uploaded patch set #3 to this change.
net/http/httptest: panic on non-3 digit (XXX) status code in Recorder.WriteHeader
This change conforms Recorder with net/http servers, to panic
when a handler writes a non-3 digit XXX status code.
fixes #45353
Change-Id: Id5ed4af652e8c150ae86bf50402b800d935e2203
---
M src/net/http/httptest/recorder.go
M src/net/http/httptest/recorder_test.go
2 files changed, 43 insertions(+), 0 deletions(-)
To view, visit change 308950. To unsubscribe, or for help writing mail filters, visit settings.
ZiZhao Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Ian Lance Taylor, Emmanuel Odeke.
2 comments:
Commit Message:
Patch Set #2, Line 7:
net/http/httptest: check the HeaderCode when WriteHeader
net/http/httptest: panic on non-3 digit (XXX) status code in Recorder.WriteHeader […]
Done
Patchset:
Thank you ZiZhao for this change, and great to catch you here! […]
Thank you for your review.
I wanted to add a test, but I forgot. thanks
To view, visit change 308950. To unsubscribe, or for help writing mail filters, visit settings.
Emmanuel Odeke (Gerrit)
Attention is currently required from: Brad Fitzpatrick, ZiZhao Zhang, Ian Lance Taylor.
1 comment:
Commit Message:
Please capitalize it, you had it capitalized before to "Fixes"
To view, visit change 308950. To unsubscribe, or for help writing mail filters, visit settings.
ZiZhao Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, ZiZhao Zhang, Ian Lance Taylor.
ZiZhao Zhang uploaded patch set #4 to this change.
net/http/httptest: panic on non-3 digit (XXX) status code in Recorder.WriteHeader
This change conforms Recorder with net/http servers, to panic
when a handler writes a non-3 digit XXX status code.
Fixes #45353
Change-Id: Id5ed4af652e8c150ae86bf50402b800d935e2203
---
M src/net/http/httptest/recorder.go
M src/net/http/httptest/recorder_test.go
2 files changed, 43 insertions(+), 0 deletions(-)
To view, visit change 308950. To unsubscribe, or for help writing mail filters, visit settings.
ZiZhao Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Ian Lance Taylor, Emmanuel Odeke.
1 comment:
Commit Message:
Please capitalize it, you had it capitalized before to "Fixes"
Done
To view, visit change 308950. To unsubscribe, or for help writing mail filters, visit settings.
Emmanuel Odeke (Gerrit)
Attention is currently required from: Brad Fitzpatrick, ZiZhao Zhang, Ian Lance Taylor.
Patch set 4:Run-TryBot +1Code-Review +2
1 comment:
Patchset:
Cool, thank you ZiZhao and LGTM!
To view, visit change 308950. To unsubscribe, or for help writing mail filters, visit settings.
Emmanuel Odeke (Gerrit)
Attention is currently required from: Brad Fitzpatrick, ZiZhao Zhang, Ian Lance Taylor.
1 comment:
Patchset:
Patch Set 4:
Build is still in progress...
This change failed on linux-arm64-aws:
See https://storage.googleapis.com/go-build-log/c5415166/linux-arm64-aws_e6de3524.logOther builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test *exactly* your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.
ZiZhao, please also run the tests locally too and adjust as needed :-) some imports
To view, visit change 308950. To unsubscribe, or for help writing mail filters, visit settings.
ZiZhao Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, ZiZhao Zhang, Ian Lance Taylor.
ZiZhao Zhang uploaded patch set #5 to this change.
net/http/httptest: panic on non-3 digit (XXX) status code in Recorder.WriteHeader
This change conforms Recorder with net/http servers, to panic
when a handler writes a non-3 digit XXX status code.
Fixes #45353
Change-Id: Id5ed4af652e8c150ae86bf50402b800d935e2203
---
M src/net/http/httptest/recorder.go
M src/net/http/httptest/recorder_test.go
2 files changed, 43 insertions(+), 0 deletions(-)
To view, visit change 308950. To unsubscribe, or for help writing mail filters, visit settings.
ZiZhao Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, ZiZhao Zhang, Ian Lance Taylor.
ZiZhao Zhang uploaded patch set #6 to this change.
net/http/httptest: panic on non-3 digit (XXX) status code in Recorder.WriteHeader
This change conforms Recorder with net/http servers, to panic
when a handler writes a non-3 digit XXX status code.
Fixes #45353
Change-Id: Id5ed4af652e8c150ae86bf50402b800d935e2203
---
M src/net/http/httptest/recorder.go
M src/net/http/httptest/recorder_test.go
2 files changed, 44 insertions(+), 0 deletions(-)
To view, visit change 308950. To unsubscribe, or for help writing mail filters, visit settings.
ZiZhao Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Ian Lance Taylor.
1 comment:
Patchset:
Patch Set 4:
Build is still in progress...
This change failed on linux-arm64-aws:
See https://storage.googleapis.com/go-build-log/c5415166/linux-arm64-aws_e6de3524.logOther builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test *exactly* your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.
ZiZhao, please also run the tests locally too and adjust as needed :-) some imports
My fault. The test is fixed.
To view, visit change 308950. To unsubscribe, or for help writing mail filters, visit settings.
Emmanuel Odeke (Gerrit)
Attention is currently required from: Brad Fitzpatrick, ZiZhao Zhang, Ian Lance Taylor.
Patch set 6:Run-TryBot +1Code-Review +2
1 comment:
Patchset:
Thank you!
To view, visit change 308950. To unsubscribe, or for help writing mail filters, visit settings.
ZiZhao Zhang (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Ian Lance Taylor.
1 comment:
Patchset:
Thank you!
Thank you for your careful review
To view, visit change 308950. To unsubscribe, or for help writing mail filters, visit settings.
Michael Knyszek (Gerrit)
Attention is currently required from: Brad Fitzpatrick, ZiZhao Zhang, Ian Lance Taylor.
Patch set 6:Trust +1
Emmanuel Odeke (Gerrit)
Attention is currently required from: Brad Fitzpatrick, ZiZhao Zhang, Ian Lance Taylor.
1 comment:
Patchset:
Thank you Michael for the Trust+1, thank you ZiZhao for the change!
To view, visit change 308950. To unsubscribe, or for help writing mail filters, visit settings.
Emmanuel Odeke (Gerrit)
Emmanuel Odeke submitted this change.
net/http/httptest: panic on non-3 digit (XXX) status code in Recorder.WriteHeader
This change conforms Recorder with net/http servers, to panic
when a handler writes a non-3 digit XXX status code.
Fixes #45353
Change-Id: Id5ed4af652e8c150ae86bf50402b800d935e2203
Reviewed-on: https://go-review.googlesource.com/c/go/+/308950
Reviewed-by: Emmanuel Odeke <emma...@orijtech.com>
Run-TryBot: Emmanuel Odeke <emma...@orijtech.com>
TryBot-Result: Go Bot <go...@golang.org>
Trust: Michael Knyszek <mkny...@google.com>
---
M src/net/http/httptest/recorder.go
M src/net/http/httptest/recorder_test.go
2 files changed, 44 insertions(+), 0 deletions(-)
diff --git a/src/net/http/httptest/recorder_test.go b/src/net/http/httptest/recorder_test.go
index a865e87..8cb32dd 100644
--- a/src/net/http/httptest/recorder_test.go
+++ b/src/net/http/httptest/recorder_test.go
@@ -345,3 +345,28 @@
}
}
}
+
+// Ensure that httptest.Recorder panics when given a non-3 digit (XXX)
+// status HTTP code. See https://golang.org/issues/45353
+func TestRecorderPanicsOnNonXXXStatusCode(t *testing.T) {
+ badCodes := []int{
+ -100, 0, 99, 1000, 20000,
+ }
+ for _, badCode := range badCodes {
+ badCode := badCode
+ t.Run(fmt.Sprintf("Code=%d", badCode), func(t *testing.T) {
+ defer func() {
+ if r := recover(); r == nil {
+ t.Fatal("Expected a panic")
+ }
+ }()
+
+ handler := func(rw http.ResponseWriter, _ *http.Request) {
+ rw.WriteHeader(badCode)
+ }
+ r, _ := http.NewRequest("GET", "http://example.org/", nil)
+ rw := NewRecorder()
+ handler(rw, r)
+ })
+ }
+}
To view, visit change 308950. To unsubscribe, or for help writing mail filters, visit settings.
Emmanuel Odeke (Gerrit)
1 comment:
Patchset:
RELNOTE=yes
To view, visit change 308950. To unsubscribe, or for help writing mail filters, visit settings.