Skip to content
This repository was archived by the owner on Apr 30, 2025. It is now read-only.

Commit 1ac049c

Browse files
mariashgeofffranks
authored andcommitted
Add an option to enable concurrent reads and writes for HTTP/1
By default Go HTTP server consumes any unread request portion before writing the response for HTTP/1. This prevents handlers from reading request and writing response concurrently. This is set to false by default since it might be an unexpected behavior and cause deadlock for some handlers. See golang/go#15527 (comment)
1 parent f0b4d8e commit 1ac049c

File tree

4 files changed

+167
-89
lines changed

4 files changed

+167
-89
lines changed

config/config.go

Lines changed: 39 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -359,29 +359,30 @@ func InitClientCertMetadataRules(rules []VerifyClientCertificateMetadataRule, ce
359359
}
360360

361361
type Config struct {
362-
Status StatusConfig `yaml:"status,omitempty"`
363-
Nats NatsConfig `yaml:"nats,omitempty"`
364-
Logging LoggingConfig `yaml:"logging,omitempty"`
365-
Port uint16 `yaml:"port,omitempty"`
366-
Prometheus PrometheusConfig `yaml:"prometheus,omitempty"`
367-
Index uint `yaml:"index,omitempty"`
368-
Zone string `yaml:"zone,omitempty"`
369-
GoMaxProcs int `yaml:"go_max_procs,omitempty"`
370-
Tracing Tracing `yaml:"tracing,omitempty"`
371-
TraceKey string `yaml:"trace_key,omitempty"`
372-
AccessLog AccessLog `yaml:"access_log,omitempty"`
373-
DebugAddr string `yaml:"debug_addr,omitempty"`
374-
EnablePROXY bool `yaml:"enable_proxy,omitempty"`
375-
EnableSSL bool `yaml:"enable_ssl,omitempty"`
376-
SSLPort uint16 `yaml:"ssl_port,omitempty"`
377-
DisableHTTP bool `yaml:"disable_http,omitempty"`
378-
EnableHTTP2 bool `yaml:"enable_http2"`
379-
SSLCertificates []tls.Certificate `yaml:"-"`
380-
TLSPEM []TLSPem `yaml:"tls_pem,omitempty"`
381-
CACerts []string `yaml:"ca_certs,omitempty"`
382-
CAPool *x509.CertPool `yaml:"-"`
383-
ClientCACerts string `yaml:"client_ca_certs,omitempty"`
384-
ClientCAPool *x509.CertPool `yaml:"-"`
362+
Status StatusConfig `yaml:"status,omitempty"`
363+
Nats NatsConfig `yaml:"nats,omitempty"`
364+
Logging LoggingConfig `yaml:"logging,omitempty"`
365+
Port uint16 `yaml:"port,omitempty"`
366+
Prometheus PrometheusConfig `yaml:"prometheus,omitempty"`
367+
Index uint `yaml:"index,omitempty"`
368+
Zone string `yaml:"zone,omitempty"`
369+
GoMaxProcs int `yaml:"go_max_procs,omitempty"`
370+
Tracing Tracing `yaml:"tracing,omitempty"`
371+
TraceKey string `yaml:"trace_key,omitempty"`
372+
AccessLog AccessLog `yaml:"access_log,omitempty"`
373+
DebugAddr string `yaml:"debug_addr,omitempty"`
374+
EnablePROXY bool `yaml:"enable_proxy,omitempty"`
375+
EnableSSL bool `yaml:"enable_ssl,omitempty"`
376+
SSLPort uint16 `yaml:"ssl_port,omitempty"`
377+
DisableHTTP bool `yaml:"disable_http,omitempty"`
378+
EnableHTTP2 bool `yaml:"enable_http2"`
379+
EnableHTTP1ConcurrentReadWrite bool `yaml:"enable_http1_concurrent_read_write"`
380+
SSLCertificates []tls.Certificate `yaml:"-"`
381+
TLSPEM []TLSPem `yaml:"tls_pem,omitempty"`
382+
CACerts []string `yaml:"ca_certs,omitempty"`
383+
CAPool *x509.CertPool `yaml:"-"`
384+
ClientCACerts string `yaml:"client_ca_certs,omitempty"`
385+
ClientCAPool *x509.CertPool `yaml:"-"`
385386

386387
SkipSSLValidation bool `yaml:"skip_ssl_validation,omitempty"`
387388
ForwardedClientCert string `yaml:"forwarded_client_cert,omitempty"`
@@ -482,20 +483,21 @@ type Config struct {
482483
}
483484

484485
var defaultConfig = Config{
485-
Status: defaultStatusConfig,
486-
Nats: defaultNatsConfig,
487-
Logging: defaultLoggingConfig,
488-
Port: 8081,
489-
Index: 0,
490-
GoMaxProcs: -1,
491-
EnablePROXY: false,
492-
EnableSSL: false,
493-
SSLPort: 443,
494-
DisableHTTP: false,
495-
EnableHTTP2: true,
496-
MinTLSVersion: tls.VersionTLS12,
497-
MaxTLSVersion: tls.VersionTLS12,
498-
RouteServicesServerPort: 7070,
486+
Status: defaultStatusConfig,
487+
Nats: defaultNatsConfig,
488+
Logging: defaultLoggingConfig,
489+
Port: 8081,
490+
Index: 0,
491+
GoMaxProcs: -1,
492+
EnablePROXY: false,
493+
EnableSSL: false,
494+
SSLPort: 443,
495+
DisableHTTP: false,
496+
EnableHTTP2: true,
497+
EnableHTTP1ConcurrentReadWrite: false,
498+
MinTLSVersion: tls.VersionTLS12,
499+
MaxTLSVersion: tls.VersionTLS12,
500+
RouteServicesServerPort: 7070,
499501

500502
EndpointTimeout: 60 * time.Second,
501503
EndpointDialTimeout: 5 * time.Second,

proxy/proxy.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ func (p *proxy) ServeHTTP(responseWriter http.ResponseWriter, request *http.Requ
214214
logger := handlers.LoggerWithTraceInfo(p.logger, request)
215215
proxyWriter := responseWriter.(utils.ProxyResponseWriter)
216216

217-
if request.ProtoMajor < 2 {
217+
if p.config.EnableHTTP1ConcurrentReadWrite && request.ProtoMajor == 1 {
218218
rc := http.NewResponseController(proxyWriter)
219219

220220
err := rc.EnableFullDuplex()

proxy/proxy_test.go

Lines changed: 100 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -765,61 +765,119 @@ var _ = Describe("Proxy", func() {
765765
Expect(resp.StatusCode).To(Equal(http.StatusOK))
766766
})
767767

768-
It("can simultaneously read request and write response", func() {
769-
contentLength := len("message 0\n") * 10
770-
ln := test_util.RegisterConnHandler(r, "read-write", func(conn *test_util.HttpConn) {
771-
defer conn.Close()
768+
Context("when concurrent read write is not enabled", func() {
769+
BeforeEach(func() {
770+
conf.EnableHTTP1ConcurrentReadWrite = false
771+
})
772772

773-
req, err := http.ReadRequest(conn.Reader)
774-
Expect(err).NotTo(HaveOccurred())
775-
defer req.Body.Close()
773+
It("can not simultaneously read request and write response", func() {
774+
contentLength := len("message 0\n") * 10
775+
ln := test_util.RegisterConnHandler(r, "read-write", func(conn *test_util.HttpConn) {
776+
defer conn.Close()
777+
778+
req, err := http.ReadRequest(conn.Reader)
779+
Expect(err).NotTo(HaveOccurred())
780+
defer req.Body.Close()
781+
782+
conn.Writer.WriteString("HTTP/1.1 200 OK\r\n" +
783+
"Connection: keep-alive\r\n" +
784+
"Content-Type: text/plain\r\n" +
785+
fmt.Sprintf("Content-Length: %d\r\n", contentLength) +
786+
"\r\n")
787+
conn.Writer.Flush()
788+
reader := bufio.NewReader(req.Body)
776789

777-
conn.Writer.WriteString("HTTP/1.1 200 OK\r\n" +
790+
for i := 0; i < 10; i++ {
791+
// send back the received message
792+
line, err := reader.ReadString('\n')
793+
if err != nil {
794+
break
795+
}
796+
conn.Writer.Write([]byte(line))
797+
conn.Writer.Flush()
798+
}
799+
})
800+
defer ln.Close()
801+
802+
conn := dialProxy(proxyServer)
803+
// the test is hanging when fix is not implemented
804+
conn.SetReadDeadline(time.Now().Add(5 * time.Second))
805+
806+
conn.Writer.Write([]byte("GET / HTTP/1.1\r\n" +
807+
"Host: read-write\r\n" +
778808
"Connection: keep-alive\r\n" +
779809
"Content-Type: text/plain\r\n" +
780810
fmt.Sprintf("Content-Length: %d\r\n", contentLength) +
781-
"\r\n")
811+
"\r\n",
812+
))
782813
conn.Writer.Flush()
783-
reader := bufio.NewReader(req.Body)
784814

785-
for i := 0; i < 10; i++ {
786-
// send back the received message
787-
line, err := reader.ReadString('\n')
788-
if err != nil {
789-
break
790-
}
791-
conn.Writer.Write([]byte(line))
792-
conn.Writer.Flush()
793-
}
815+
_, err := http.ReadResponse(conn.Reader, &http.Request{})
816+
Expect(err).To(HaveOccurred())
794817
})
795-
defer ln.Close()
818+
})
796819

797-
conn := dialProxy(proxyServer)
798-
// the test is hanging when fix is not implemented
799-
conn.SetReadDeadline(time.Now().Add(5 * time.Second))
800-
801-
conn.Writer.Write([]byte("GET / HTTP/1.1\r\n" +
802-
"Host: read-write\r\n" +
803-
"Connection: keep-alive\r\n" +
804-
"Content-Type: text/plain\r\n" +
805-
fmt.Sprintf("Content-Length: %d\r\n", contentLength) +
806-
"\r\n",
807-
))
808-
conn.Writer.Flush()
820+
Context("when concurrent read write is enabled", func() {
821+
BeforeEach(func() {
822+
conf.EnableHTTP1ConcurrentReadWrite = true
823+
})
809824

810-
resp, err := http.ReadResponse(conn.Reader, &http.Request{})
811-
Expect(err).NotTo(HaveOccurred())
812-
defer resp.Body.Close()
813-
reader := bufio.NewReader(resp.Body)
825+
It("can simultaneously read request and write response", func() {
826+
contentLength := len("message 0\n") * 10
827+
ln := test_util.RegisterConnHandler(r, "read-write", func(conn *test_util.HttpConn) {
828+
defer conn.Close()
829+
830+
req, err := http.ReadRequest(conn.Reader)
831+
Expect(err).NotTo(HaveOccurred())
832+
defer req.Body.Close()
833+
834+
conn.Writer.WriteString("HTTP/1.1 200 OK\r\n" +
835+
"Connection: keep-alive\r\n" +
836+
"Content-Type: text/plain\r\n" +
837+
fmt.Sprintf("Content-Length: %d\r\n", contentLength) +
838+
"\r\n")
839+
conn.Writer.Flush()
840+
reader := bufio.NewReader(req.Body)
841+
842+
for i := 0; i < 10; i++ {
843+
// send back the received message
844+
line, err := reader.ReadString('\n')
845+
if err != nil {
846+
break
847+
}
848+
conn.Writer.Write([]byte(line))
849+
conn.Writer.Flush()
850+
}
851+
})
852+
defer ln.Close()
814853

815-
for i := 0; i < 10; i++ {
816-
message := fmt.Sprintf("message %d\n", i)
817-
conn.Writer.Write([]byte(message))
854+
conn := dialProxy(proxyServer)
855+
// the test is hanging when fix is not implemented
856+
conn.SetReadDeadline(time.Now().Add(5 * time.Second))
857+
858+
conn.Writer.Write([]byte("GET / HTTP/1.1\r\n" +
859+
"Host: read-write\r\n" +
860+
"Connection: keep-alive\r\n" +
861+
"Content-Type: text/plain\r\n" +
862+
fmt.Sprintf("Content-Length: %d\r\n", contentLength) +
863+
"\r\n",
864+
))
818865
conn.Writer.Flush()
819-
line, err := reader.ReadString('\n')
866+
867+
resp, err := http.ReadResponse(conn.Reader, &http.Request{})
820868
Expect(err).NotTo(HaveOccurred())
821-
Expect(line).To(Equal(message))
822-
}
869+
defer resp.Body.Close()
870+
reader := bufio.NewReader(resp.Body)
871+
872+
for i := 0; i < 10; i++ {
873+
message := fmt.Sprintf("message %d\n", i)
874+
conn.Writer.Write([]byte(message))
875+
conn.Writer.Flush()
876+
line, err := reader.ReadString('\n')
877+
Expect(err).NotTo(HaveOccurred())
878+
Expect(line).To(Equal(message))
879+
}
880+
})
823881
})
824882

825883
It("retries on POST requests if nothing was written", func() {

proxy/proxy_unit_test.go

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -97,19 +97,37 @@ var _ = Describe("Proxy Unit tests", func() {
9797

9898
Describe("full duplex", func() {
9999
Context("for HTTP/1.1 requests", func() {
100-
It("enables full duplex", func() {
101-
req := test_util.NewRequest("GET", "some-app", "/", bytes.NewReader([]byte("some-body")))
102-
proxyObj.ServeHTTP(resp, req)
103-
Expect(responseRecorder.EnableFullDuplexCallCount).To(Equal(1))
104-
})
100+
Context("when concurrent read write is enabled", func() {
101+
BeforeEach(func() {
102+
conf.EnableHTTP1ConcurrentReadWrite = true
103+
})
105104

106-
Context("when enabling duplex fails", func() {
107-
It("fails", func() {
108-
responseRecorder.EnableFullDuplexErr = errors.New("unsupported")
105+
It("enables full duplex", func() {
109106
req := test_util.NewRequest("GET", "some-app", "/", bytes.NewReader([]byte("some-body")))
110107
proxyObj.ServeHTTP(resp, req)
108+
Expect(responseRecorder.EnableFullDuplexCallCount).To(Equal(1))
109+
})
110+
111+
Context("when enabling duplex fails", func() {
112+
It("fails", func() {
113+
responseRecorder.EnableFullDuplexErr = errors.New("unsupported")
114+
req := test_util.NewRequest("GET", "some-app", "/", bytes.NewReader([]byte("some-body")))
115+
proxyObj.ServeHTTP(resp, req)
111116

112-
Eventually(fakeLogger).Should(Say("enable-full-duplex-err"))
117+
Eventually(fakeLogger).Should(Say("enable-full-duplex-err"))
118+
})
119+
})
120+
})
121+
122+
Context("when concurrent read write is not enabled", func() {
123+
BeforeEach(func() {
124+
conf.EnableHTTP1ConcurrentReadWrite = false
125+
})
126+
127+
It("does not enable full duplex", func() {
128+
req := test_util.NewRequest("GET", "some-app", "/", bytes.NewReader([]byte("some-body")))
129+
proxyObj.ServeHTTP(resp, req)
130+
Expect(responseRecorder.EnableFullDuplexCallCount).To(Equal(0))
113131
})
114132
})
115133
})

0 commit comments

Comments
 (0)