http2: don't hang a stream if trailers values are not provided
Pre-declared trailers are written after a server's request handler
returns, in which case the trailer header frame is also responsible
for closing the stream. However, if the server handler does not
define a trailer header value before returning, the headers are empty
and no header frame is written. In this case, the stream would simply
hang.
This change fixes this by closing the stream if there are no header
values but endStream is true. Some other options to consider:
1. Reset the stream with an error; the user didn't define a trailer value
as they said they would. From my brief reading of the http/2 RFC,
this isn't actually required.
2. Send an explicitly empty header value.
Change-Id: I07e01250df60ac33fa6d34beb81ec2fa7e43c146
GitHub-Last-Rev: dfc532d7857822d507b218e4cc2511bf0b6dfcbd
GitHub-Pull-Request: golang/net#31
Reviewed-on: https://go-review.googlesource.com/c/net/+/161958
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/http2/server.go b/http2/server.go
index 8f17019..d4abeb2 100644
--- a/http2/server.go
+++ b/http2/server.go
@@ -2307,7 +2307,16 @@
func (cw chunkWriter) Write(p []byte) (n int, err error) { return cw.rws.writeChunk(p) }
-func (rws *responseWriterState) hasTrailers() bool { return len(rws.trailers) != 0 }
+func (rws *responseWriterState) hasTrailers() bool { return len(rws.trailers) > 0 }
+
+func (rws *responseWriterState) hasNonemptyTrailers() bool {
+ for _, trailer := range rws.trailers {
+ if _, ok := rws.handlerHeader[trailer]; ok {
+ return true
+ }
+ }
+ return false
+}
// declareTrailer is called for each Trailer header when the
// response header is written. It notes that a header will need to be
@@ -2407,7 +2416,10 @@
rws.promoteUndeclaredTrailers()
}
- endStream := rws.handlerDone && !rws.hasTrailers()
+ // only send trailers if they have actually been defined by the
+ // server handler.
+ hasNonemptyTrailers := rws.hasNonemptyTrailers()
+ endStream := rws.handlerDone && !hasNonemptyTrailers
if len(p) > 0 || endStream {
// only send a 0 byte DATA frame if we're ending the stream.
if err := rws.conn.writeDataFromHandler(rws.stream, p, endStream); err != nil {
@@ -2416,7 +2428,7 @@
}
}
- if rws.handlerDone && rws.hasTrailers() {
+ if rws.handlerDone && hasNonemptyTrailers {
err = rws.conn.writeHeaders(rws.stream, &writeResHeaders{
streamID: rws.stream.id,
h: rws.handlerHeader,
diff --git a/http2/server_test.go b/http2/server_test.go
index c543143..6e71db9 100644
--- a/http2/server_test.go
+++ b/http2/server_test.go
@@ -2708,6 +2708,26 @@
}
}
+func TestServer_Response_Stream_With_Missing_Trailer(t *testing.T) {
+ testServerResponse(t, func(w http.ResponseWriter, r *http.Request) error {
+ w.Header().Set("Trailer", "test-trailer")
+ return nil
+ }, func(st *serverTester) {
+ getSlash(st)
+ hf := st.wantHeaders()
+ if !hf.HeadersEnded() {
+ t.Fatal("want END_HEADERS flag")
+ }
+ df := st.wantData()
+ if len(df.data) != 0 {
+ t.Fatal("did not want data")
+ }
+ if !df.StreamEnded() {
+ t.Fatal("want END_STREAM flag")
+ }
+ })
+}
+
func TestCompressionErrorOnWrite(t *testing.T) {
const maxStrLen = 8 << 10
var serverConfig *http.Server
