From 7a7970478712cfa57384e5f0fd418288b9930e08 Mon Sep 17 00:00:00 2001
From: Akshay Shah <akshayjshah@users.noreply.github.com>
Date: Wed, 30 Nov 2022 12:12:48 -0800
Subject: [PATCH] Close conn when HTTP/1.1 clients call bidi methods (#408)

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
 	}