From 3b0b415c694780af654f9cc0de375979bed920ca Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Fri, 20 May 2022 11:53:44 +0200 Subject: [PATCH] don't close the request stream when http3.DataStreamer was used (#3413) --- http3/server.go | 18 ++++++++++-------- http3/server_test.go | 30 +++++++++++++++++++++++++++--- 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/http3/server.go b/http3/server.go index efb0a530894..645b2b3ea27 100644 --- a/http3/server.go +++ b/http3/server.go @@ -582,15 +582,17 @@ func (s *Server) handleRequest(conn quic.Connection, str quic.Stream, decoder *q handler.ServeHTTP(r, req) }() - if !r.usedDataStream() { - if panicked { - r.WriteHeader(500) - } else { - r.WriteHeader(200) - } - // If the EOF was read by the handler, CancelRead() is a no-op. - str.CancelRead(quic.StreamErrorCode(errorNoError)) + if r.usedDataStream() { + return requestError{err: errHijacked} + } + + if panicked { + r.WriteHeader(500) + } else { + r.WriteHeader(200) } + // If the EOF was read by the handler, CancelRead() is a no-op. + str.CancelRead(quic.StreamErrorCode(errorNoError)) return requestError{} } diff --git a/http3/server_test.go b/http3/server_test.go index bcf50268cd0..d3a360afd32 100644 --- a/http3/server_test.go +++ b/http3/server_test.go @@ -228,13 +228,37 @@ var _ = Describe("Server", func() { str.Write([]byte("foobar")) }) + rspWritten := make(chan struct{}) setRequest(encodeRequest(exampleGetRequest)) str.EXPECT().Context().Return(reqContext) - str.EXPECT().Write([]byte("foobar")) + str.EXPECT().Write([]byte("foobar")).Do(func(b []byte) (int, error) { + close(rspWritten) + return len(b), nil + }) // don't EXPECT CancelRead() - serr := s.handleRequest(conn, str, qpackDecoder, nil) - Expect(serr.err).ToNot(HaveOccurred()) + ctrlStr := mockquic.NewMockStream(mockCtrl) + ctrlStr.EXPECT().Write(gomock.Any()).AnyTimes() + conn.EXPECT().OpenUniStream().Return(ctrlStr, nil) + conn.EXPECT().AcceptUniStream(gomock.Any()).DoAndReturn(func(context.Context) (quic.ReceiveStream, error) { + <-rspWritten + return nil, errors.New("done") + }) + conn.EXPECT().AcceptStream(gomock.Any()).Return(str, nil) + conn.EXPECT().AcceptStream(gomock.Any()).DoAndReturn(func(context.Context) (quic.Stream, error) { + <-rspWritten + return nil, errors.New("done") + }) + + done := make(chan struct{}) + go func() { + defer GinkgoRecover() + defer close(done) + s.handleConn(conn) + }() + Eventually(rspWritten).Should(BeClosed()) + time.Sleep(50 * time.Millisecond) // make sure that after str.Write there are no further calls to stream methods + Eventually(done).Should(BeClosed()) }) Context("hijacking unidirectional streams", func() {