From 6e0d99a3860b0e5184a9b8b9f7a8f66035340f59 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 29 May 2022 19:51:08 +0200 Subject: [PATCH] remove the http3.DataStreamer --- http3/response_writer.go | 28 +++------------------------- http3/server.go | 10 +--------- http3/server_test.go | 39 --------------------------------------- 3 files changed, 4 insertions(+), 73 deletions(-) diff --git a/http3/response_writer.go b/http3/response_writer.go index 9f232e0f2bb..c0de2d4ec2a 100644 --- a/http3/response_writer.go +++ b/http3/response_writer.go @@ -12,25 +12,14 @@ import ( "github.com/marten-seemann/qpack" ) -// DataStreamer lets the caller take over the stream. After a call to DataStream -// the HTTP server library will not do anything else with the connection. -// -// It becomes the caller's responsibility to manage and close the stream. -// -// After a call to DataStream, the original Request.Body must not be used. -type DataStreamer interface { - DataStream() quic.Stream -} - type responseWriter struct { conn quic.Connection stream quic.Stream // needed for DataStream() bufferedStream *bufio.Writer - header http.Header - status int // status code passed to WriteHeader - headerWritten bool - dataStreamUsed bool // set when DataSteam() is called + header http.Header + status int // status code passed to WriteHeader + headerWritten bool logger utils.Logger } @@ -38,7 +27,6 @@ type responseWriter struct { var ( _ http.ResponseWriter = &responseWriter{} _ http.Flusher = &responseWriter{} - _ DataStreamer = &responseWriter{} _ Hijacker = &responseWriter{} ) @@ -112,16 +100,6 @@ func (w *responseWriter) Flush() { } } -func (w *responseWriter) usedDataStream() bool { - return w.dataStreamUsed -} - -func (w *responseWriter) DataStream() quic.Stream { - w.dataStreamUsed = true - w.Flush() - return w.stream -} - func (w *responseWriter) StreamID() quic.StreamID { return w.stream.StreamID() } diff --git a/http3/server.go b/http3/server.go index b52e546d182..45ca3f4c81b 100644 --- a/http3/server.go +++ b/http3/server.go @@ -562,11 +562,7 @@ func (s *Server) handleRequest(conn quic.Connection, str quic.Stream, decoder *q ctx = context.WithValue(ctx, http.LocalAddrContextKey, conn.LocalAddr()) req = req.WithContext(ctx) r := newResponseWriter(str, conn, s.logger) - defer func() { - if !r.usedDataStream() { - r.Flush() - } - }() + defer r.Flush() handler := s.Handler if handler == nil { handler = http.DefaultServeMux @@ -587,10 +583,6 @@ func (s *Server) handleRequest(conn quic.Connection, str quic.Stream, decoder *q handler.ServeHTTP(r, req) }() - if r.usedDataStream() { - return requestError{err: errHijacked} - } - if panicked { r.WriteHeader(500) } else { diff --git a/http3/server_test.go b/http3/server_test.go index e652072a9eb..064380c3d88 100644 --- a/http3/server_test.go +++ b/http3/server_test.go @@ -222,45 +222,6 @@ var _ = Describe("Server", func() { Expect(hfs).To(HaveKeyWithValue(":status", []string{"500"})) }) - It("doesn't close the stream if the handler called DataStream()", func() { - s.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - str := w.(DataStreamer).DataStream() - str.Write([]byte("foobar")) - }) - - rspWritten := make(chan struct{}) - setRequest(encodeRequest(exampleGetRequest)) - str.EXPECT().Context().Return(reqContext) - str.EXPECT().Write([]byte("foobar")).Do(func(b []byte) (int, error) { - close(rspWritten) - return len(b), nil - }) - // don't EXPECT CancelRead() - - 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 bidirectional streams", func() { var conn *mockquic.MockEarlyConnection testDone := make(chan struct{})