net/http: undeclared HTTP/2 trailers not sent when response body is empty and unflushed #54723

akshayjshah · 2022-08-29T04:14:31Z

What version of Go are you using (`go version`)?

$ go version
go version go1.19 darwin/arm64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (`go env`)?

go env Output

$ go env
GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/ashah/Library/Caches/go-build"
GOENV="/Users/ashah/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/ashah/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/ashah"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/ashah/sdk/go1.19"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/ashah/sdk/go1.19/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.19"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/ashah/src/github.com/akshayjshah/trailererr/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/0h/bt5cl84j5zs2t3sbyyy8vgzw0000gn/T/go-build2678483231=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

In a net/http handler, I wrote some HTTP trailers to the response writer using http.TrailerPrefix. The handler never writes to or flushes the response body. (This is admittedly odd, but trailers without a body are necessary in the gRPC HTTP/2 protocol.)

What did you expect to see?

I expected the client to receive the trailers regardless of the HTTP protocol version in use.

What did you see instead?

Everything works as expected when the server and client use HTTP/1.1. Regardless of the HTTP protocol version, everything also works as expected if I stop using http.TrailerPrefix and instead set the "Trailer" header.

When the client negotiates up to HTTP/2 and the handler uses http.TrailerPrefix, the client doesn't receive any trailers unless the handler flushes the response body. This is a little strange - it doesn't match the HTTP/1.x behavior, and it's not obvious why flushing a response without any buffered data should do anything at all. (I assume that something in x/net/http2 doesn't send trailers unless we've sent a DATA frame, and perhaps flushing always sends a frame? I haven't dug far enough to verify that hypothesis.)

Here's a reproduction:

package main

import (
	"io"
	"net/http"
	"net/http/httptest"
	"testing"
)

func testTrailers(tb testing.TB, flush bool) {
	const trailerKey = "Foo"
	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		w.Header().Set(http.TrailerPrefix+trailerKey, "Bar")
		if flusher, ok := w.(http.Flusher); ok && flush {
			flusher.Flush()
		}
		if _, err := io.Copy(io.Discard, r.Body); err != nil {
			tb.Errorf("discard request body: %v", err)
		}
		if err := r.Body.Close(); err != nil {
			tb.Errorf("close request body: %v", err)
		}
	})

	srv := httptest.NewUnstartedServer(handler)
	// Replacing the next two lines with srv.Start() allows both tests to pass.
	srv.EnableHTTP2 = true
	srv.StartTLS()
	defer srv.Close()

	res, err := srv.Client().Get(srv.URL)
	if err != nil {
		tb.Fatal(err)
	}
	tb.Logf("response protocol is %s", res.Proto)
	if _, err := io.Copy(io.Discard, res.Body); err != nil {
		tb.Fatalf("discard response body: %v", err)
	}
	if res.Trailer.Get(trailerKey) == "" {
		tb.Fatalf("missing trailer %q, got %v", trailerKey, res.Trailer)
	}
}

func TestTrailersFlush(t *testing.T) {
	testTrailers(t, true /* flush */)
}

func TestTrailersWithoutFlush(t *testing.T) {
	testTrailers(t, false /* flush */)
}

The text was updated successfully, but these errors were encountered:

seankhliao · 2022-08-29T18:08:24Z

cc @neild

gopherbot · 2022-08-30T14:46:48Z

Change https://go.dev/cl/426157 mentions this issue: http2: [wip] http.TrailerPrefix behavior depends on HTTP version

ianwoolf · 2022-08-30T14:49:08Z

i send a wip cl. i found rws.conn.writeHeaders return http2errStreamClosed (https://go.googlesource.com/go/+/refs/tags/go1.19/src/net/http/h2_bundle.go#6218), I am still researching. hope some help

akshayjshah · 2022-08-30T16:04:53Z

@ianwoolf Any changes to the code in h2_bundle.go would need to be made in golang.org/x/net/http2. They'll be vendored into net/http during the next release cycle.

ianwoolf · 2022-08-30T16:25:49Z

@ianwoolf Any changes to the code in h2_bundle.go would need to be made in golang.org/x/net/http2. They'll be vendored into net/http during the next release cycle.

sorry i missed that, i am sending a new cl to golang.org/x/net/http2

gopherbot · 2022-08-30T17:50:59Z

Change https://go.dev/cl/426754 mentions this issue: http2: [wip] add Trailer for writeChunk

gopherbot · 2022-08-30T20:53:14Z

Change https://go.dev/cl/426874 mentions this issue: http2: send undeclared trailers when body is not written

neild · 2022-08-30T20:56:40Z

The problem is that we're checking for undeclared trailers (with the magic Trailer: prefix) only after sending the response headers, which results in the headers being sent with an END_STREAM flag to finish the request. Flushing the ResponseWriter avoids the problem by sending the headers while the handler is still running.

You can probably also work around the problem by calling WriteHeader.

https://go.dev/cl/426874 contains a fix.

ianwoolf · 2022-08-31T14:42:13Z

The problem is that we're checking for undeclared trailers (with the magic Trailer: prefix) only after sending the response headers, which results in the headers being sent with an END_STREAM flag to finish the request. Flushing the ResponseWriter avoids the problem by sending the headers while the handler is still running.

You can probably also work around the problem by calling WriteHeader.

https://go.dev/cl/426874 contains a fix.

hmm.. i get it, thank you

akshayjshah · 2022-09-06T20:05:42Z

Thanks for the fix, @neild! The bit of code I was looking at can predeclare trailers, so I just set the Trailers header. It's nice to have this bit of surprising behavior polished away, though :)

Fix a bug wherein undeclared trailers (set with the "Trailer:" prefix) were not sent when the response body is neither written nor flushed. We were testing responseWriterState.hasTrailers before promoting undeclared trailers into rws.trailers, resulting in the response headers being sent with an END_STREAM flag. Promote undeclared headers earlier so that we leave the stream open for them to be sent. For golang/go#54723 Change-Id: Ic036925f4a7ec775282b6e474aa72249d6418b23 Reviewed-on: https://go-review.googlesource.com/c/net/+/426874 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Damien Neil <dneil@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>

odeke-em · 2024-01-25T07:16:37Z

Thank you everyone for the discourse, for reporting the issue @akshayjshah and @neild for the fix in CL https://go-review.googlesource.com/c/net/+/426874 in September 2022 for x/net/http2 which got bundled into the standard library(so fixed many Go cycles ago) and the repro from @akshayjshah now passes. With that I shall close this issue!

akshayjshah changed the title ~~net/http: behavior of http.TrailerPrefix depends on HTTP version in use~~ net/http: http.TrailerPrefix behavior depends on HTTP version Aug 29, 2022

seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 29, 2022

neild changed the title ~~net/http: http.TrailerPrefix behavior depends on HTTP version~~ net/http: undeclared HTTP/2 trailers not sent when response body is empty and unflushed Aug 30, 2022

seankhliao added this to the Go1.20 milestone Sep 3, 2022

gopherbot modified the milestones: Go1.20, Go1.21 Feb 1, 2023

gopherbot modified the milestones: Go1.21, Go1.22 Aug 8, 2023

odeke-em closed this as completed Jan 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

net/http: undeclared HTTP/2 trailers not sent when response body is empty and unflushed #54723

net/http: undeclared HTTP/2 trailers not sent when response body is empty and unflushed #54723

akshayjshah commented Aug 29, 2022 •

edited

seankhliao commented Aug 29, 2022

gopherbot commented Aug 30, 2022

ianwoolf commented Aug 30, 2022 •

edited

akshayjshah commented Aug 30, 2022

ianwoolf commented Aug 30, 2022

gopherbot commented Aug 30, 2022

gopherbot commented Aug 30, 2022

neild commented Aug 30, 2022

ianwoolf commented Aug 31, 2022

akshayjshah commented Sep 6, 2022

odeke-em commented Jan 25, 2024

net/http: undeclared HTTP/2 trailers not sent when response body is empty and unflushed #54723

net/http: undeclared HTTP/2 trailers not sent when response body is empty and unflushed #54723

Comments

akshayjshah commented Aug 29, 2022 • edited

What version of Go are you using (go version)?

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

What did you do?

What did you expect to see?

What did you see instead?

seankhliao commented Aug 29, 2022

gopherbot commented Aug 30, 2022

ianwoolf commented Aug 30, 2022 • edited

akshayjshah commented Aug 30, 2022

ianwoolf commented Aug 30, 2022

gopherbot commented Aug 30, 2022

gopherbot commented Aug 30, 2022

neild commented Aug 30, 2022

ianwoolf commented Aug 31, 2022

akshayjshah commented Sep 6, 2022

odeke-em commented Jan 25, 2024

akshayjshah commented Aug 29, 2022 •

edited

What version of Go are you using (`go version`)?

What operating system and processor architecture are you using (`go env`)?

ianwoolf commented Aug 30, 2022 •

edited