Skip to content

x/net/http2: return unexpected eof on empty response with non-zero content length #114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 23 additions & 13 deletions http2/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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 {
Expand Down
53 changes: 53 additions & 0 deletions http2/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}