From a1db411cc28c8214776af8b6a62f4e60c9d2fea8 Mon Sep 17 00:00:00 2001 From: newacorn Date: Sun, 11 Aug 2024 21:35:28 +0800 Subject: [PATCH] StreamRequestBody shouldn't read more data than actual need. (#1819) * The StreamRequestBody should not read content beyond what is required. The StreamRequestBody feature on the server side should not read content that does not belong to the current request body.This is more logical and consistent with the result of not using the StreamRequestBody feature.Fixes: https://github.com/valyala/fasthttp/issues/1816. * Update server_test.go Co-authored-by: Erik Dubbelboer * Update http.go Co-authored-by: Erik Dubbelboer --------- Co-authored-by: Erik Dubbelboer --- http.go | 13 +++++++------ server_test.go | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/http.go b/http.go index 187f91d..ef6eafc 100644 --- a/http.go +++ b/http.go @@ -2283,12 +2283,13 @@ func readBodyWithStreaming(r *bufio.Reader, contentLength, maxBodySize int, dst readN = 8 * 1024 } - if contentLength >= 0 && maxBodySize >= contentLength { - b, err = appendBodyFixedSize(r, dst, readN) - } else { - b, err = readBodyIdentity(r, readN, dst) - } - + // A fixed-length pre-read function should be used here; otherwise, + // it may read content beyond the request body into areas outside + // the br buffer. This could affect the handling of the next request + // in the br buffer, if there is one. The original two branches can + // be handled with this single branch. by the way, + // fix issue: https://github.com/valyala/fasthttp/issues/1816 + b, err = appendBodyFixedSize(r, dst, readN) if err != nil { return b, err } diff --git a/server_test.go b/server_test.go index cd192ef..31ec188 100644 --- a/server_test.go +++ b/server_test.go @@ -4358,3 +4358,50 @@ func (cl *testLogger) Printf(format string, args ...any) { cl.out += fmt.Sprintf(format, args...)[6:] + "\n" cl.lock.Unlock() } + +func TestRequestBodyStreamReadIssue1816(t *testing.T) { + pcs := fasthttputil.NewPipeConns() + cliCon, serverCon := pcs.Conn1(), pcs.Conn2() + go func() { + req := AcquireRequest() + defer ReleaseRequest(req) + req.Header.SetContentLength(10) + req.Header.SetMethod("POST") + req.SetRequestURI("http://localhsot:8080") + req.SetBodyRaw(bytes.Repeat([]byte{'1'}, 10)) + var pipelineReqBody []byte + reqBody := req.String() + pipelineReqBody = append(pipelineReqBody, reqBody...) + pipelineReqBody = append(pipelineReqBody, reqBody...) + _, err := cliCon.Write(pipelineReqBody) + if err != nil { + t.Error(err) + } + resp := AcquireResponse() + err = resp.Read(bufio.NewReader(cliCon)) + if err != nil { + t.Error(err) + } + err = cliCon.Close() + if err != nil { + t.Error(err) + } + }() + server := Server{StreamRequestBody: true, MaxRequestBodySize: 5, Handler: func(ctx *RequestCtx) { + r := ctx.RequestBodyStream() + p := make([]byte, 1300) + for { + _, err := r.Read(p) + if err != nil { + if err != io.EOF { + t.Fatal(err) + } + break + } + } + }} + err := server.serveConn(serverCon) + if err != nil { + t.Fatal(err) + } +}