Skip to content

Commit

Permalink
Send gRPC error metadata only as HTTP trailers (#410)
Browse files Browse the repository at this point in the history
In #406, I was too cavalier about `net/http.ResponseWriter`'s API to
send HTTP trailers. Because we're not calling `WriteHeader` before
between predeclaring our trailers and actually setting the trailer
values, we end up sending the same data as both HTTP headers and
trailers.
  • Loading branch information
akshayjshah authored Dec 5, 2022
1 parent c16c8c9 commit 63d5a8d
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 1 deletion.
41 changes: 41 additions & 0 deletions connect_ext_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"compress/flate"
"compress/gzip"
"context"
"encoding/binary"
"errors"
"fmt"
"io"
Expand Down Expand Up @@ -1824,6 +1825,46 @@ func TestUnflushableResponseWriter(t *testing.T) {
}
}

func TestGRPCErrorMetadataIsTrailersOnly(t *testing.T) {
t.Parallel()
mux := http.NewServeMux()
mux.Handle(pingv1connect.NewPingServiceHandler(pingServer{}))
server := httptest.NewUnstartedServer(mux)
server.EnableHTTP2 = true
server.StartTLS()
t.Cleanup(server.Close)

protoBytes, err := proto.Marshal(&pingv1.FailRequest{Code: int32(connect.CodeInternal)})
assert.Nil(t, err)
// Manually construct a gRPC prefix. Data is uncompressed, so the first byte
// is 0. Set the last 4 bytes to the message length.
var prefix [5]byte
binary.BigEndian.PutUint32(prefix[1:5], uint32(len(protoBytes)))
body := append(prefix[:], protoBytes...)
// Manually send off a gRPC request.
req, err := http.NewRequestWithContext(
context.Background(),
http.MethodPost,
server.URL+"/connect.ping.v1.PingService/Fail",
bytes.NewReader(body),
)
assert.Nil(t, err)
req.Header.Set("Content-Type", "application/grpc")
res, err := server.Client().Do(req)
assert.Nil(t, err)
assert.Equal(t, res.StatusCode, http.StatusOK)
assert.Equal(t, res.Header.Get("Content-Type"), "application/grpc")
// pingServer.Fail adds handlerHeader and handlerTrailer to the error
// metadata. The gRPC protocol should send all error metadata as trailers.
assert.Zero(t, res.Header.Get(handlerHeader))
assert.Zero(t, res.Header.Get(handlerTrailer))
_, err = io.Copy(io.Discard, res.Body)
assert.Nil(t, err)
assert.Nil(t, res.Body.Close())
assert.NotZero(t, res.Trailer.Get(handlerHeader))
assert.NotZero(t, res.Trailer.Get(handlerTrailer))
}

func TestBidiOverHTTP1(t *testing.T) {
t.Parallel()
mux := http.NewServeMux()
Expand Down
5 changes: 4 additions & 1 deletion protocol_grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -528,8 +528,11 @@ func (hc *grpcHandlerConn) Close(err error) (retErr error) {
// has wrapped the response writer in net/http middleware that doesn't
// implement http.Flusher, we must pre-declare our HTTP trailers. We can
// remove this when Go 1.21 ships and we drop support for Go 1.19.
for key, values := range mergedTrailers {
for key := range mergedTrailers {
hc.responseWriter.Header().Add("Trailer", key)
}
hc.responseWriter.WriteHeader(http.StatusOK)
for key, values := range mergedTrailers {
for _, value := range values {
hc.responseWriter.Header().Add(key, value)
}
Expand Down

0 comments on commit 63d5a8d

Please sign in to comment.