From f78f23f8e33f825be517dffc868d9e136f4efdea Mon Sep 17 00:00:00 2001 From: Alexander Yastrebov Date: Thu, 7 Oct 2021 20:43:08 +0000 Subject: [PATCH] http2: return unexpected eof on empty response with non-zero content length Fixes golang/go#46071 Change-Id: I8b8262ba8e28a129f6aacfca43b7b8d9e605a0ce GitHub-Last-Rev: 11b92cc8ba6e1d08716aac816d33b659563a893f GitHub-Pull-Request: golang/net#114 Reviewed-on: https://go-review.googlesource.com/c/net/+/354141 Reviewed-by: Damien Neil Trust: Damien Neil Trust: Alexander Rakoczy Run-TryBot: Alexander Rakoczy TryBot-Result: Go Bot --- transport.go | 36 ++++++++++++++++++++------------ transport_test.go | 53 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 13 deletions(-) diff --git a/transport.go b/transport.go index c88d2279..a5ba742d 100644 --- a/transport.go +++ b/transport.go @@ -2217,28 +2217,33 @@ func (rl *clientConnReadLoop) handleResponse(cs *clientStream, f *MetaHeadersFra return nil, nil } - streamEnded := f.StreamEnded() - isHead := cs.req.Method == "HEAD" - if !streamEnded || isHead { - res.ContentLength = -1 - if clens := res.Header["Content-Length"]; len(clens) == 1 { - if cl, err := strconv.ParseUint(clens[0], 10, 63); err == nil { - res.ContentLength = int64(cl) - } else { - // TODO: care? unlike http/1, it won't mess up our framing, so it's - // more safe smuggling-wise to ignore. - } - } else if len(clens) > 1 { + res.ContentLength = -1 + if clens := res.Header["Content-Length"]; len(clens) == 1 { + if cl, err := strconv.ParseUint(clens[0], 10, 63); err == nil { + res.ContentLength = int64(cl) + } else { // TODO: care? unlike http/1, it won't mess up our framing, so it's // more safe smuggling-wise to ignore. } + } else if len(clens) > 1 { + // TODO: care? unlike http/1, it won't mess up our framing, so it's + // more safe smuggling-wise to ignore. } - if streamEnded || isHead { + if cs.req.Method == "HEAD" { res.Body = noBody return res, nil } + if f.StreamEnded() { + if res.ContentLength > 0 { + res.Body = missingBody{} + } else { + res.Body = noBody + } + return res, nil + } + cs.bufPipe.setBuffer(&dataBuffer{expected: res.ContentLength}) cs.bytesRemain = res.ContentLength res.Body = transportResponseBody{cs} @@ -2786,6 +2791,11 @@ func (t *Transport) logf(format string, args ...interface{}) { var noBody io.ReadCloser = ioutil.NopCloser(bytes.NewReader(nil)) +type missingBody struct{} + +func (missingBody) Close() error { return nil } +func (missingBody) Read([]byte) (int, error) { return 0, io.ErrUnexpectedEOF } + func strSliceContains(ss []string, s string) bool { for _, v := range ss { if v == s { diff --git a/transport_test.go b/transport_test.go index 3c026953..b250738a 100644 --- a/transport_test.go +++ b/transport_test.go @@ -5623,3 +5623,56 @@ func TestTransportTimeoutServerHangs(t *testing.T) { } ct.run() } + +func TestTransportContentLengthWithoutBody(t *testing.T) { + contentLength := "" + st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Length", contentLength) + }, optOnlyServer) + defer st.Close() + tr := &Transport{TLSClientConfig: tlsConfigInsecure} + defer tr.CloseIdleConnections() + + for _, test := range []struct { + name string + contentLength string + wantBody string + wantErr error + wantContentLength int64 + }{ + { + name: "non-zero content length", + contentLength: "42", + wantErr: io.ErrUnexpectedEOF, + wantContentLength: 42, + }, + { + name: "zero content length", + contentLength: "0", + wantErr: nil, + wantContentLength: 0, + }, + } { + t.Run(test.name, func(t *testing.T) { + contentLength = test.contentLength + + req, _ := http.NewRequest("GET", st.ts.URL, nil) + res, err := tr.RoundTrip(req) + if err != nil { + t.Fatal(err) + } + defer res.Body.Close() + body, err := io.ReadAll(res.Body) + + if err != test.wantErr { + t.Errorf("Expected error %v, got: %v", test.wantErr, err) + } + if len(body) > 0 { + t.Errorf("Expected empty body, got: %v", body) + } + if res.ContentLength != test.wantContentLength { + t.Errorf("Expected content length %d, got: %d", test.wantContentLength, res.ContentLength) + } + }) + } +}