Enable full duplex mode for HTTP/1 requests

mariash · geofffranks · commit f0b4d8effe24 · 2024-02-28T09:13:55.000-05:00
To allow concurrent reading from the request and writing the response. * Add Unwrap to ResponseWriter to satisfy http.ResponseController support (Go 1.20+) https://pkg.go.dev/net/http#NewResponseController https://www.pivotaltracker.com/story/show/182988664
diff --git a/handlers/access_log.go b/handlers/access_log.go
@@ -13,7 +13,7 @@ import (
 	"code.cloudfoundry.org/gorouter/proxy/utils"
 
 	"github.com/uber-go/zap"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 type accessLog struct {
diff --git a/handlers/access_log_test.go b/handlers/access_log_test.go
@@ -15,7 +15,7 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 var _ = Describe("AccessLog", func() {
diff --git a/handlers/clientcert.go b/handlers/clientcert.go
@@ -12,7 +12,7 @@ import (
 	"code.cloudfoundry.org/gorouter/logger"
 	"code.cloudfoundry.org/gorouter/routeservice"
 	"github.com/uber-go/zap"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 const xfcc = "X-Forwarded-Client-Cert"
diff --git a/handlers/clientcert_test.go b/handlers/clientcert_test.go
@@ -18,7 +18,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/uber-go/zap"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 var _ = Describe("Clientcert", func() {
diff --git a/handlers/http_rewrite.go b/handlers/http_rewrite.go
@@ -3,7 +3,7 @@ package handlers
 import (
 	"net/http"
 
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 
 	"code.cloudfoundry.org/gorouter/config"
 	"code.cloudfoundry.org/gorouter/proxy/utils"
diff --git a/handlers/http_rewrite_test.go b/handlers/http_rewrite_test.go
@@ -8,7 +8,7 @@ import (
 	"code.cloudfoundry.org/gorouter/handlers"
 	logger_fakes "code.cloudfoundry.org/gorouter/logger/fakes"
 
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
diff --git a/handlers/httplatencyprometheus.go b/handlers/httplatencyprometheus.go
@@ -5,7 +5,7 @@ import (
 	"time"
 
 	metrics "code.cloudfoundry.org/go-metric-registry"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 type Registry interface {
diff --git a/handlers/httplatencyprometheus_test.go b/handlers/httplatencyprometheus_test.go
@@ -14,7 +14,7 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 var _ = Describe("Http Prometheus Latency", func() {
diff --git a/handlers/httpstartstop.go b/handlers/httpstartstop.go
@@ -12,7 +12,7 @@ import (
 	"github.com/cloudfoundry/sonde-go/events"
 	uuid "github.com/nu7hatch/gouuid"
 	"github.com/uber-go/zap"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 	"google.golang.org/protobuf/proto"
 )
 
diff --git a/handlers/httpstartstop_test.go b/handlers/httpstartstop_test.go
@@ -21,7 +21,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/gbytes"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 func findEnvelope(fakeEmitter *fake.FakeEventEmitter, eventType events.Envelope_EventType) *events.Envelope {
diff --git a/handlers/lookup.go b/handlers/lookup.go
@@ -14,7 +14,7 @@ import (
 	"code.cloudfoundry.org/gorouter/registry"
 	"code.cloudfoundry.org/gorouter/route"
 	"github.com/uber-go/zap"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 const CfAppInstance = "X-CF-APP-INSTANCE"
diff --git a/handlers/lookup_test.go b/handlers/lookup_test.go
@@ -16,7 +16,7 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 var _ = Describe("Lookup", func() {
diff --git a/handlers/max_request_size_test.go b/handlers/max_request_size_test.go
@@ -14,7 +14,7 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 var _ = Describe("MaxRequestSize", func() {
diff --git a/handlers/paniccheck.go b/handlers/paniccheck.go
@@ -10,7 +10,7 @@ import (
 
 	"code.cloudfoundry.org/gorouter/logger"
 	"github.com/uber-go/zap"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 type panicCheck struct {
diff --git a/handlers/paniccheck_test.go b/handlers/paniccheck_test.go
@@ -16,7 +16,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/gbytes"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 var _ = Describe("Paniccheck", func() {
diff --git a/handlers/protocolcheck.go b/handlers/protocolcheck.go
@@ -10,7 +10,7 @@ import (
 
 	"code.cloudfoundry.org/gorouter/errorwriter"
 	"code.cloudfoundry.org/gorouter/logger"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 type protocolCheck struct {
diff --git a/handlers/protocolcheck_test.go b/handlers/protocolcheck_test.go
@@ -13,7 +13,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/ghttp"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 var _ = Describe("Protocolcheck", func() {
diff --git a/handlers/proxy_healthcheck.go b/handlers/proxy_healthcheck.go
@@ -4,7 +4,7 @@ import (
 	"net/http"
 
 	"code.cloudfoundry.org/gorouter/common/health"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 type proxyHealthcheck struct {
diff --git a/handlers/proxy_healthcheck_test.go b/handlers/proxy_healthcheck_test.go
@@ -11,7 +11,7 @@ import (
 	"code.cloudfoundry.org/gorouter/test_util"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 var _ = Describe("Proxy Healthcheck", func() {
diff --git a/handlers/proxywriter.go b/handlers/proxywriter.go
@@ -5,7 +5,7 @@ import (
 
 	"code.cloudfoundry.org/gorouter/logger"
 	"github.com/uber-go/zap"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 
 	"code.cloudfoundry.org/gorouter/proxy/utils"
 )
diff --git a/handlers/proxywriter_test.go b/handlers/proxywriter_test.go
@@ -13,7 +13,7 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 var _ = Describe("ProxyWriter", func() {
diff --git a/handlers/query_param.go b/handlers/query_param.go
@@ -8,7 +8,7 @@ import (
 	"code.cloudfoundry.org/gorouter/logger"
 
 	"github.com/uber-go/zap"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 type queryParam struct {
diff --git a/handlers/query_param_test.go b/handlers/query_param_test.go
@@ -16,7 +16,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/gbytes"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 var _ = Describe("QueryParamHandler", func() {
diff --git a/handlers/reporter.go b/handlers/reporter.go
@@ -9,7 +9,7 @@ import (
 
 	"code.cloudfoundry.org/gorouter/logger"
 	"github.com/uber-go/zap"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 type reporterHandler struct {
diff --git a/handlers/reporter_test.go b/handlers/reporter_test.go
@@ -16,7 +16,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/gbytes"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 var _ = Describe("Reporter Handler", func() {
diff --git a/handlers/request_id.go b/handlers/request_id.go
@@ -5,7 +5,7 @@ import (
 
 	"code.cloudfoundry.org/gorouter/logger"
 	"github.com/uber-go/zap"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 const (
diff --git a/handlers/request_id_test.go b/handlers/request_id_test.go
@@ -12,7 +12,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/gbytes"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 const UUIDRegex = "^(urn\\:uuid\\:)?\\{?([a-z0-9]{8})-([a-z0-9]{4})-([1-5][a-z0-9]{3})-([a-z0-9]{4})-([a-z0-9]{12})\\}?$"
diff --git a/handlers/requestinfo.go b/handlers/requestinfo.go
@@ -17,7 +17,7 @@ import (
 
 	"github.com/openzipkin/zipkin-go/idgenerator"
 	"github.com/openzipkin/zipkin-go/model"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 type key string
diff --git a/handlers/requestinfo_test.go b/handlers/requestinfo_test.go
@@ -18,7 +18,7 @@ import (
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/gbytes"
 	"github.com/uber-go/zap"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 var _ = Describe("RequestInfoHandler", func() {
diff --git a/handlers/routeservice.go b/handlers/routeservice.go
@@ -13,7 +13,7 @@ import (
 	"code.cloudfoundry.org/gorouter/registry"
 	"code.cloudfoundry.org/gorouter/routeservice"
 	"github.com/uber-go/zap"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 
 	"code.cloudfoundry.org/gorouter/route"
 )
diff --git a/handlers/routeservice_test.go b/handlers/routeservice_test.go
@@ -22,7 +22,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/gbytes"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 )
 
 var _ = Describe("Route Service Handler", func() {
diff --git a/handlers/w3c.go b/handlers/w3c.go
@@ -5,7 +5,7 @@ import (
 	"net/http"
 
 	"github.com/uber-go/zap"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 
 	"code.cloudfoundry.org/gorouter/logger"
 )
diff --git a/handlers/zipkin.go b/handlers/zipkin.go
@@ -5,7 +5,7 @@ import (
 
 	"github.com/openzipkin/zipkin-go/propagation/b3"
 	"github.com/uber-go/zap"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 
 	"code.cloudfoundry.org/gorouter/logger"
 )
diff --git a/proxy/proxy.go b/proxy/proxy.go
@@ -14,7 +14,7 @@ import (
 
 	"github.com/cloudfoundry/dropsonde"
 	"github.com/uber-go/zap"
-	"github.com/urfave/negroni"
+	"github.com/urfave/negroni/v3"
 
 	"code.cloudfoundry.org/gorouter/accesslog"
 	router_http "code.cloudfoundry.org/gorouter/common/http"
@@ -214,6 +214,15 @@ func (p *proxy) ServeHTTP(responseWriter http.ResponseWriter, request *http.Requ
 	logger := handlers.LoggerWithTraceInfo(p.logger, request)
 	proxyWriter := responseWriter.(utils.ProxyResponseWriter)
 
+	if request.ProtoMajor < 2 {
+		rc := http.NewResponseController(proxyWriter)
+
+		err := rc.EnableFullDuplex()
+		if err != nil {
+			logger.Panic("enable-full-duplex-err", zap.Error(err))
+		}
+	}
+
 	reqInfo, err := handlers.ContextRequestInfo(request)
 	if err != nil {
 		logger.Panic("request-info-err", zap.Error(err))
diff --git a/proxy/proxy_suite_test.go b/proxy/proxy_suite_test.go
@@ -83,6 +83,7 @@ var _ = BeforeEach(func() {
 	conf.EnableHTTP2 = false
 	conf.Backends.MaxAttempts = 3
 	conf.RouteServiceConfig.MaxAttempts = 3
+	conf.DisableKeepAlives = false
 	fakeReporter = &fakes.FakeCombinedReporter{}
 	fakeRegistry = fake_registry.NewMetricsRegistry()
 	skipSanitization = func(*http.Request) bool { return false }
diff --git a/proxy/proxy_test.go b/proxy/proxy_test.go
@@ -765,6 +765,63 @@ var _ = Describe("Proxy", func() {
 			Expect(resp.StatusCode).To(Equal(http.StatusOK))
 		})
 
+		It("can simultaneously read request and write response", func() {
+			contentLength := len("message 0\n") * 10
+			ln := test_util.RegisterConnHandler(r, "read-write", func(conn *test_util.HttpConn) {
+				defer conn.Close()
+
+				req, err := http.ReadRequest(conn.Reader)
+				Expect(err).NotTo(HaveOccurred())
+				defer req.Body.Close()
+
+				conn.Writer.WriteString("HTTP/1.1 200 OK\r\n" +
+					"Connection: keep-alive\r\n" +
+					"Content-Type: text/plain\r\n" +
+					fmt.Sprintf("Content-Length: %d\r\n", contentLength) +
+					"\r\n")
+				conn.Writer.Flush()
+				reader := bufio.NewReader(req.Body)
+
+				for i := 0; i < 10; i++ {
+					// send back the received message
+					line, err := reader.ReadString('\n')
+					if err != nil {
+						break
+					}
+					conn.Writer.Write([]byte(line))
+					conn.Writer.Flush()
+				}
+			})
+			defer ln.Close()
+
+			conn := dialProxy(proxyServer)
+			// the test is hanging when fix is not implemented
+			conn.SetReadDeadline(time.Now().Add(5 * time.Second))
+
+			conn.Writer.Write([]byte("GET / HTTP/1.1\r\n" +
+				"Host: read-write\r\n" +
+				"Connection: keep-alive\r\n" +
+				"Content-Type: text/plain\r\n" +
+				fmt.Sprintf("Content-Length: %d\r\n", contentLength) +
+				"\r\n",
+			))
+			conn.Writer.Flush()
+
+			resp, err := http.ReadResponse(conn.Reader, &http.Request{})
+			Expect(err).NotTo(HaveOccurred())
+			defer resp.Body.Close()
+			reader := bufio.NewReader(resp.Body)
+
+			for i := 0; i < 10; i++ {
+				message := fmt.Sprintf("message %d\n", i)
+				conn.Writer.Write([]byte(message))
+				conn.Writer.Flush()
+				line, err := reader.ReadString('\n')
+				Expect(err).NotTo(HaveOccurred())
+				Expect(line).To(Equal(message))
+			}
+		})
+
 		It("retries on POST requests if nothing was written", func() {
 			//FIXME: this test is flakey. we can't skip/pend this test since CI will fail on any skipped/pending
 			//       tests
diff --git a/proxy/proxy_unit_test.go b/proxy/proxy_unit_test.go
diff --git a/proxy/utils/responsewriter.go b/proxy/utils/responsewriter.go
diff --git a/proxy/utils/responsewriter_test.go b/proxy/utils/responsewriter_test.go

Original file line number	Diff line number	Diff line change
`@@ -13,7 +13,7 @@ import (`
`13`	`13`	`"code.cloudfoundry.org/gorouter/proxy/utils"`
`14`	`14`
`15`	`15`	`"github.com/uber-go/zap"`
`16`		`- "github.com/urfave/negroni"`
	`16`	`+ "github.com/urfave/negroni/v3"`
`17`	`17`	`)`
`18`	`18`
`19`	`19`	`type accessLog struct {`
Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,7 @@ import (`
`15`	`15`
`16`	`16`	`. "github.com/onsi/ginkgo/v2"`
`17`	`17`	`. "github.com/onsi/gomega"`
`18`		`- "github.com/urfave/negroni"`
	`18`	`+ "github.com/urfave/negroni/v3"`
`19`	`19`	`)`
`20`	`20`
`21`	`21`	`var _ = Describe("AccessLog", func() {`
Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@ import (`
`12`	`12`	`"code.cloudfoundry.org/gorouter/logger"`
`13`	`13`	`"code.cloudfoundry.org/gorouter/routeservice"`
`14`	`14`	`"github.com/uber-go/zap"`
`15`		`- "github.com/urfave/negroni"`
	`15`	`+ "github.com/urfave/negroni/v3"`
`16`	`16`	`)`
`17`	`17`
`18`	`18`	`const xfcc = "X-Forwarded-Client-Cert"`
Original file line number	Diff line number	Diff line change
`@@ -18,7 +18,7 @@ import (`
`18`	`18`	`. "github.com/onsi/ginkgo/v2"`
`19`	`19`	`. "github.com/onsi/gomega"`
`20`	`20`	`"github.com/uber-go/zap"`
`21`		`- "github.com/urfave/negroni"`
	`21`	`+ "github.com/urfave/negroni/v3"`
`22`	`22`	`)`
`23`	`23`
`24`	`24`	`var _ = Describe("Clientcert", func() {`
Original file line number	Diff line number	Diff line change
`@@ -5,7 +5,7 @@ import (`
`5`	`5`	`"time"`
`6`	`6`
`7`	`7`	`metrics "code.cloudfoundry.org/go-metric-registry"`
`8`		`- "github.com/urfave/negroni"`
	`8`	`+ "github.com/urfave/negroni/v3"`
`9`	`9`	`)`
`10`	`10`
`11`	`11`	`type Registry interface {`
Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,7 @@ import (`
`14`	`14`
`15`	`15`	`. "github.com/onsi/ginkgo/v2"`
`16`	`16`	`. "github.com/onsi/gomega"`
`17`		`- "github.com/urfave/negroni"`
	`17`	`+ "github.com/urfave/negroni/v3"`
`18`	`18`	`)`
`19`	`19`
`20`	`20`	`var _ = Describe("Http Prometheus Latency", func() {`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ import (`
`21`	`21`	`. "github.com/onsi/ginkgo/v2"`
`22`	`22`	`. "github.com/onsi/gomega"`
`23`	`23`	`"github.com/onsi/gomega/gbytes"`
`24`		`- "github.com/urfave/negroni"`
	`24`	`+ "github.com/urfave/negroni/v3"`
`25`	`25`	`)`
`26`	`26`
`27`	`27`	`func findEnvelope(fakeEmitter fake.FakeEventEmitter, eventType events.Envelope_EventType) events.Envelope {`