Skip to content

Commit

Permalink
StreamRequestBody shouldn't read more data than actual need. (#1819)
Browse files Browse the repository at this point in the history
* 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: #1816.

* Update server_test.go

Co-authored-by: Erik Dubbelboer <erik@dubbelboer.com>

* Update http.go

Co-authored-by: Erik Dubbelboer <erik@dubbelboer.com>

---------

Co-authored-by: Erik Dubbelboer <erik@dubbelboer.com>
  • Loading branch information
newacorn and erikdubbelboer authored Aug 11, 2024
1 parent 38a91cd commit a1db411
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 6 deletions.
13 changes: 7 additions & 6 deletions http.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
47 changes: 47 additions & 0 deletions server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

0 comments on commit a1db411

Please sign in to comment.