Skip to content
Open
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
3 changes: 3 additions & 0 deletions internal/http3/headers.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ func parseHeaders(decodeFn qpack.DecodeFunc, isRequest bool, sizeLimit int, head
if ds.ShouldDump() && len(hdr.Headers) > 0 {
ds.DumpResponseHeader([]byte("\r\n"))
}
if isRequest && (hdr.Scheme == "http" || hdr.Scheme == "https") && strings.Contains(hdr.Authority, "@") {
return header{}, errors.New("userinfo is not allowed in :authority")
}
hdr.ContentLength = -1
if len(contentLengthStr) > 0 {
// use ParseUint instead of ParseInt, so that parsing fails on negative values
Expand Down
26 changes: 26 additions & 0 deletions internal/http3/headers_test.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,36 @@
package http3

import (
"io"
"net/http"
"testing"

"github.com/imroc/req/v3/internal/dump"
"github.com/quic-go/qpack"
)

func TestParseHeadersRejectsUserinfoInAuthority(t *testing.T) {
fields := []qpack.HeaderField{
{Name: ":method", Value: "GET"},
{Name: ":scheme", Value: "https"},
{Name: ":authority", Value: "user@example.com"},
{Name: ":path", Value: "/"},
}
decode := func() (qpack.HeaderField, error) {
if len(fields) == 0 {
return qpack.HeaderField{}, io.EOF
}
field := fields[0]
fields = fields[1:]
return field, nil
}

_, err := parseHeaders(decode, true, 1<<20, nil, dump.Dumpers{})
if err == nil || err.Error() != "userinfo is not allowed in :authority" {
t.Fatalf("got error %v, want userinfo rejection", err)
}
}

func TestExtractAnnouncedTrailers(t *testing.T) {
tests := []struct {
name string
Expand Down
15 changes: 10 additions & 5 deletions transfer.go
Original file line number Diff line number Diff line change
Expand Up @@ -827,19 +827,24 @@ var (
doubleCRLF = []byte("\r\n\r\n")
)

func seeUpcomingDoubleCRLF(r *bufio.Reader) bool {
func seeUpcomingDoubleCRLF(r *bufio.Reader) error {
for peekSize := 4; ; peekSize++ {
// This loop stops when Peek returns an error,
// which it does when r's buffer has been filled.
buf, err := r.Peek(peekSize)
for i, b := range buf {
if b == '\n' && (i == 0 || buf[i-1] != '\r') {
return errors.New("http: invalid trailer")
}
}
if bytes.HasSuffix(buf, doubleCRLF) {
return true
return nil
}
if err != nil {
break
}
}
return false
return errors.New("http: suspiciously long trailer after chunked body")
}

var errTrailerEOF = errors.New("http: unexpected EOF reading trailer")
Expand All @@ -866,8 +871,8 @@ func (b *body) readTrailer() error {
// this bufio.textprotoReader. Instead, a hack: we iteratively Peek up
// to the bufio.textprotoReader's max size, looking for a double CRLF.
// This limits the trailer to the underlying buffer size, typically 4kB.
if !seeUpcomingDoubleCRLF(b.r) {
return errors.New("http: suspiciously long trailer after chunked body")
if err := seeUpcomingDoubleCRLF(b.r); err != nil {
return err
}

hdr, err := textproto.NewReader(b.r).ReadMIMEHeader()
Expand Down
64 changes: 64 additions & 0 deletions transfer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@ package req

import (
"bufio"
"fmt"
"io"
"net"
"net/http"
"strings"
"sync/atomic"
"testing"
)

Expand All @@ -26,3 +30,63 @@ func TestReadTransferClosesResponseWithTransferEncodingAndContentLength(t *testi
t.Fatal("response with both Transfer-Encoding and Content-Length did not set Close")
}
}

func TestResponseWithBareLFInTrailerClosesConnection(t *testing.T) {
ln, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
t.Fatal(err)
}
defer ln.Close()

var connections, requests atomic.Int32
go func() {
for {
conn, err := ln.Accept()
if err != nil {
return
}
connections.Add(1)
go func() {
defer conn.Close()
r := bufio.NewReader(conn)
for {
req, err := http.ReadRequest(r)
if err != nil {
return
}
req.Body.Close()
if requests.Add(1) == 1 {
fmt.Fprint(conn, "HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\nTrailer: X-One, X-Two\r\n\r\n0\r\nX-One: a\nX-Two: b\r\n\r\n")
} else {
fmt.Fprint(conn, "HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\nok")
}
}
}()
}
}()

tr := NewTransport()
defer tr.CloseIdleConnections()
client := &http.Client{Transport: tr}
resp, err := client.Get("http://" + ln.Addr().String())
if err != nil {
t.Fatal(err)
}
_, err = io.ReadAll(resp.Body)
resp.Body.Close()
if err == nil || err.Error() != "http: invalid trailer" {
t.Fatalf("got body read error %v, want http: invalid trailer", err)
}

resp, err = client.Get("http://" + ln.Addr().String())
if err != nil {
t.Fatal(err)
}
defer resp.Body.Close()
if _, err := io.ReadAll(resp.Body); err != nil {
t.Fatal(err)
}
if got := connections.Load(); got != 2 {
t.Fatalf("got %d connections, want 2; invalid trailer connection was reused", got)
}
}
Loading