diff --git a/header.go b/header.go index f489058..a28254d 100644 --- a/header.go +++ b/header.go @@ -2677,15 +2677,6 @@ func (h *RequestHeader) parse(buf []byte) (int, error) { } func parseTrailer(src []byte, dest []argsKV, disableNormalizing bool) ([]argsKV, int, error) { - // Skip any 0 length chunk. - if src[0] == '0' { - skip := len(strCRLF) + 1 - if len(src) < skip { - return dest, 0, io.EOF - } - src = src[skip:] - } - var s headerScanner s.b = src diff --git a/http.go b/http.go index 66cd8da..a3c3e02 100644 --- a/http.go +++ b/http.go @@ -1454,7 +1454,10 @@ func (req *Request) ContinueReadBody(r *bufio.Reader, maxBodySize int, preParseM if contentLength == -1 { err = req.Header.ReadTrailer(r) - if err != nil && err != io.EOF { + if err != nil { + if err == io.EOF { + return ErrBrokenChunk{error: io.ErrUnexpectedEOF} + } return err } } @@ -1592,7 +1595,10 @@ func (resp *Response) ReadLimitBody(r *bufio.Reader, maxBodySize int) error { // A response without a body can't have trailers. if resp.Header.ContentLength() == -1 && !resp.StreamBody && !resp.mustSkipBody() { err = resp.Header.ReadTrailer(r) - if err != nil && err != io.EOF { + if err != nil { + if err == io.EOF { + return ErrBrokenChunk{error: io.ErrUnexpectedEOF} + } return err } } @@ -2743,6 +2749,7 @@ func parseChunkSize(r *bufio.Reader) (int, error) { if err != nil { return -1, err } + inExt := false for { c, err := r.ReadByte() if err != nil { @@ -2750,24 +2757,35 @@ func parseChunkSize(r *bufio.Reader) (int, error) { error: fmt.Errorf("cannot read '\\r' char at the end of chunk size: %w", err), } } - // Skip chunk extension after chunk size. - // Add support later if anyone needs it. - if c != '\r' { - // Security: Don't allow newlines in chunk extensions. - // This can lead to request smuggling issues with some reverse proxies. - if c == '\n' { + if c == '\r' { + if err := r.UnreadByte(); err != nil { return -1, ErrBrokenChunk{ - error: errors.New("invalid character '\\n' after chunk size"), + error: fmt.Errorf("cannot unread '\\r' char at the end of chunk size: %w", err), } } - continue + break } - if err := r.UnreadByte(); err != nil { + // Security: Don't allow newlines in chunk extensions. + // This can lead to request smuggling issues with some reverse proxies. + if c == '\n' { return -1, ErrBrokenChunk{ - error: fmt.Errorf("cannot unread '\\r' char at the end of chunk size: %w", err), + error: errors.New("invalid character '\\n' after chunk size"), + } + } + if inExt { + continue + } + switch c { + case ' ', '\t': + continue + case ';': + inExt = true + continue + default: + return -1, ErrBrokenChunk{ + error: fmt.Errorf("invalid character %q after chunk size", c), } } - break } err = readCrLf(r) if err != nil { diff --git a/http_test.go b/http_test.go index 677cf93..276b1a7 100644 --- a/http_test.go +++ b/http_test.go @@ -3245,6 +3245,12 @@ func TestRequestMultipartFormPipeEmptyFormField(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } + if _, err = bw.Write(strCRLF); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if err = bw.Flush(); err != nil { + t.Fatalf("unexpected error: %v", err) + } for e := range errs { t.Fatalf("unexpected error in goroutine multiwriter: %v", e)