From 54cae90dfa1486714715a4e77555dec24f0b172f Mon Sep 17 00:00:00 2001 From: Akshay Shah Date: Wed, 30 Nov 2022 11:32:47 -0800 Subject: [PATCH] Close conn when HTTP/1.1 clients call bidi methods If an HTTP/1.1 client calls a bidi RPC method, the caller has likely written application-level code that expects a full-duplex stream. Unless the server closes the TCP connection, the client code will often end up blocked because it's trying to read the response before it closes the request body. --- connect_ext_test.go | 23 +++++++++++++++++++++++ handler.go | 4 ++++ 2 files changed, 27 insertions(+) diff --git a/connect_ext_test.go b/connect_ext_test.go index e415ee79..9c53ba2d 100644 --- a/connect_ext_test.go +++ b/connect_ext_test.go @@ -1824,6 +1824,29 @@ func TestUnflushableResponseWriter(t *testing.T) { } } +func TestBidiOverHTTP1(t *testing.T) { + t.Parallel() + mux := http.NewServeMux() + mux.Handle(pingv1connect.NewPingServiceHandler(pingServer{})) + server := httptest.NewServer(mux) + t.Cleanup(server.Close) + + // Clients expecting a full-duplex connection that end up with a simplex + // HTTP/1.1 connection shouldn't hang. Instead, the server should close the + // TCP connection. + client := pingv1connect.NewPingServiceClient(server.Client(), server.URL) + stream := client.CumSum(context.Background()) + if err := stream.Send(&pingv1.CumSumRequest{Number: 2}); err != nil { + assert.ErrorIs(t, err, io.EOF) + } + _, err := stream.Receive() + assert.NotNil(t, err) + assert.Equal(t, connect.CodeOf(err), connect.CodeUnknown) + assert.Equal(t, err.Error(), "unknown: HTTP status 505 HTTP Version Not Supported") + assert.Nil(t, stream.CloseRequest()) + assert.Nil(t, stream.CloseResponse()) +} + type unflushableWriter struct { w http.ResponseWriter } diff --git a/handler.go b/handler.go index f7942dbb..bc4a30a0 100644 --- a/handler.go +++ b/handler.go @@ -162,6 +162,10 @@ func (h *Handler) ServeHTTP(responseWriter http.ResponseWriter, request *http.Re // okay if we can't re-use the connection. isBidi := (h.spec.StreamType & StreamTypeBidi) == StreamTypeBidi if isBidi && request.ProtoMajor < 2 { + // Clients coded to expect full-duplex connections may hang if they've + // mistakenly negotiated HTTP/1.1. To unblock them, we must close the + // underlying TCP connection. + responseWriter.Header().Set("Connection", "close") responseWriter.WriteHeader(http.StatusHTTPVersionNotSupported) return }