diff --git a/http/httphi/bench_test.go b/http/httphi/bench_test.go index c7f25b7..5ea8fca 100644 --- a/http/httphi/bench_test.go +++ b/http/httphi/bench_test.go @@ -41,8 +41,9 @@ var benchBody = []byte("hello world") func benchExchange(b *testing.B, conn conn) *Exchange { b.Helper() const bufferSize = 1024 + const numHeaderCap = 2 exch := new(Exchange) - exch.Configure(make([]byte, 2*bufferSize), bufferSize, false) + exch.Configure(make([]byte, 2*bufferSize), bufferSize, numHeaderCap, false) if !exch.Acquire(conn) { b.Fatal("fresh exchange failed to acquire connection") } diff --git a/http/httphi/exchange.go b/http/httphi/exchange.go index 0dd2fda..49063a5 100644 --- a/http/httphi/exchange.go +++ b/http/httphi/exchange.go @@ -48,6 +48,13 @@ type Exchange struct { readErr error } +type ExchangeConfig struct { + RawBuf []byte + RequestBufferLim int + NumHeaderCap int + NormalizeOutgoingKeys bool +} + // HijackRaw is a low-level implementation of http.Hijacker interface. // A Hijack method is not exposed due to heap allocation implications and correctness concerns. // Below is what an actual implementation may look like: @@ -82,14 +89,15 @@ func (exch *Exchange) HijackRaw(dstBody []byte) (conn, []byte, error) { // of which the first requestLim bytes are reserved for the request header. // Panics if requestLim exceeds the buffer. Set normalizeKeys to normalize // outgoing header keys, i.e: "content-type" to "Content-Type". -func (exch *Exchange) Configure(rawbuf []byte, requestLim int, normalizeKeys bool) { - respSize := len(rawbuf) - requestLim +func (exch *Exchange) Configure(cfg ExchangeConfig) { + respSize := len(cfg.RawBuf) - cfg.RequestBufferLim if respSize < 0 { panic("request lim larger than buffer") } - exch.rawbuf = rawbuf - exch.reqHdr.Reset(rawbuf[:0:requestLim]) - exch.normalizeKeys = normalizeKeys + exch.rawbuf = cfg.RawBuf + exch.reqHdr.Reset(cfg.RawBuf[:0:cfg.RequestBufferLim], cfg.NumHeaderCap) + exch.reqHdr.ConfigBufferGrowth(false) + exch.normalizeKeys = cfg.NormalizeOutgoingKeys } // Acquire claims the exchange for conn and resets it to serve a new request, @@ -111,7 +119,7 @@ func (exch *Exchange) Acquire(conn conn) bool { exch.rw = conn exch.headerWritten = false exch.nextFree = nil - exch.reqHdr.Reset(nil) + exch.reqHdr.Reset(nil, 0) return true } @@ -350,13 +358,11 @@ func (exch *Exchange) ReadBody(dst []byte) (n int, _ error) { } n = copy(dst, toRead) exch.respRemains -= n - if len(dst) == n { - return n, nil - } - dst = dst[n:] + // hand over what already arrived since conn might have + // exhausted data and could block indefinetely. + return n, nil } - nr, err := exch.rw.Read(dst) - return nr + n, err + return exch.rw.Read(dst) } func (exch *Exchange) remainingSurplusBody() ([]byte, error) { diff --git a/http/httphi/exchange_test.go b/http/httphi/exchange_test.go index 8889fe3..6d1fa34 100644 --- a/http/httphi/exchange_test.go +++ b/http/httphi/exchange_test.go @@ -5,6 +5,7 @@ import ( "errors" "io" "net/http" + "strconv" "strings" "unsafe" @@ -19,10 +20,11 @@ import ( func nopBackoff(consecutiveBackoffs uint) time.Duration { return lneto.BackoffFlagNop } // newExchange returns an Exchange acquired on conn, ready to serve a request. -func newExchange(t *testing.T, conn conn, bufferSize int, normalizeKeys bool) *Exchange { +func newExchange(t *testing.T, conn conn, cfg ExchangeConfig) *Exchange { t.Helper() exch := new(Exchange) - exch.Configure(make([]byte, 2*bufferSize), bufferSize, normalizeKeys) + const numHeaderCap = 1 + exch.Configure(cfg) if !exch.Acquire(conn) { t.Fatal("fresh exchange failed to acquire connection") } @@ -46,6 +48,7 @@ func serve(t *testing.T, request string, mux Mux) *rwconn { // WriteHeader must emit a complete status line terminated in CRLF followed by // the end-of-headers CRLF, for every status code including the longest text. func TestExchangeWriteHeader(t *testing.T) { + var buf [128]byte for _, test := range []struct { code int want string @@ -57,7 +60,7 @@ func TestExchangeWriteHeader(t *testing.T) { {code: 511, want: "HTTP/1.1 511 Network Authentication Required\r\n\r\n"}, } { conn := newConn("") - exch := newExchange(t, conn, 128, false) + exch := newExchange(t, conn, ExchangeConfig{RawBuf: buf[:], RequestBufferLim: 64}) exch.WriteHeader(test.code) if got := conn.ViewWritten(); got != test.want { t.Errorf("code %d: want %q, got %q", test.code, test.want, got) @@ -67,8 +70,9 @@ func TestExchangeWriteHeader(t *testing.T) { // Status line is written once: a second WriteHeader must not reach the wire. func TestExchangeWriteHeaderOnce(t *testing.T) { + var buf [128]byte conn := newConn("") - exch := newExchange(t, conn, 128, false) + exch := newExchange(t, conn, ExchangeConfig{RawBuf: buf[:], RequestBufferLim: 64}) exch.WriteHeader(404) exch.WriteHeader(500) const want = "HTTP/1.1 404 Not Found\r\n\r\n" @@ -79,9 +83,10 @@ func TestExchangeWriteHeaderOnce(t *testing.T) { // Write with no prior WriteHeader must flush a 200 header ahead of the body. func TestExchangeWriteFlushesHeader(t *testing.T) { + var buf [128]byte const body = "hello" conn := newConn("") - exch := newExchange(t, conn, 128, false) + exch := newExchange(t, conn, ExchangeConfig{RawBuf: buf[:], RequestBufferLim: 64}) n, err := exch.WriteBody([]byte(body)) if err != nil { t.Fatal(err) @@ -124,7 +129,7 @@ func TestExchangeSetHeader(t *testing.T) { } { t.Run(test.name, func(t *testing.T) { conn := newConn("") - exch := newExchange(t, conn, 128, test.normalize) + exch := newExchange(t, conn, ExchangeConfig{RawBuf: make([]byte, 128), RequestBufferLim: 64, NormalizeOutgoingKeys: test.normalize}) for _, kv := range test.set { if !exch.StageHeader(kv[0], kv[1]) { t.Fatalf("SetHeader(%q,%q) reported insufficient memory", kv[0], kv[1]) @@ -147,7 +152,7 @@ func TestExchangeSetHeader(t *testing.T) { func TestExchangeSetHeaderOOM(t *testing.T) { const bufferSize = 32 conn := newConn("") - exch := newExchange(t, conn, bufferSize, false) + exch := newExchange(t, conn, ExchangeConfig{RawBuf: make([]byte, bufferSize), RequestBufferLim: 64}) if exch.StageHeader("X-Big", strings.Repeat("v", 4*bufferSize)) { t.Fatal("want insufficient memory reported for oversized header value") } @@ -346,10 +351,11 @@ func TestExchangeReadBody(t *testing.T) { func TestExchangeStageOKAndFail(t *testing.T) { const key, value = "K", "V" const field = len(key) + len(value) + len(":\r\n") + const numHeaderCap = 4 for _, bufLen := range []int{field + 2, field + 1, field} { conn := newConn("") exch := new(Exchange) - exch.Configure(make([]byte, bufLen), bufLen, false) + exch.Configure(make([]byte, bufLen), bufLen, numHeaderCap, false) if !exch.Acquire(conn) { t.Fatal("fresh exchange failed to acquire connection") } @@ -942,3 +948,144 @@ func TestExchangeRequestParseMultipartRejects(t *testing.T) { } } } + +// A request from a browser carries around twenty header fields. Serving one +// must not depend on how many fields the parser happens to have room for: the +// exchange's buffer is the memory the caller granted, and the field table comes +// out of it. +func TestHandleBrowserSizedRequest(t *testing.T) { + const wantMode = "navigate" + request := "GET /echo HTTP/1.1\r\nHost: lneto.test\r\n" + + "User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0 Safari/537.36\r\n" + + "Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8\r\n" + + "Accept-Language: en-US,en;q=0.5\r\nAccept-Encoding: gzip, deflate, br\r\n" + + "Upgrade-Insecure-Requests: 1\r\nSec-Fetch-Dest: document\r\nSec-Fetch-Site: none\r\n" + + "Sec-Ch-Ua: \"Chromium\";v=\"120\"\r\nCache-Control: max-age=0\r\nDnt: 1\r\n" + + "Referer: https://lneto.test/index.html\r\nCookie: session=abcdef0123456789; theme=dark\r\n" + + "X-Trace: 0123456789abcdef\r\nX-Client: bench\r\nX-Seq: 42\r\nX-Tag: alpha\r\n" + + "X-Nonce: cafebabe\r\nX-Mode: " + wantMode + "\r\n\r\n" + + var gotMode string + var fields int + var sm MuxSlice + sm.Reset(1) + sm.Handle("GET /echo", func(exch *Exchange) { + gotMode = string(exch.RequestHeader("X-Mode")) + exch.RequestHeaderRaw().ForEach(func(key, value []byte) error { + fields++ + return nil + }) + }) + conn := newConn(request) + conn.Hangup() + exch := newExchange(t, conn, 8192, false) + if err := Handle(exch, &sm, nopBackoff); err != nil { + t.Fatalf("serving a browser sized request: %s", err) + } + if gotMode != wantMode { + t.Errorf("last header field read back as %q, want %q", gotMode, wantMode) + } + const sent = 19 + if fields < sent { + t.Errorf("handler saw %d header fields, request carried %d", fields, sent) + } +} + +// A request with more header fields than the exchange has room for must be +// answered, not dropped: the peer learns its request was too large instead of +// seeing the connection go away. +func TestHandleTooManyHeaderFields(t *testing.T) { + request := "GET /echo HTTP/1.1\r\nHost: lneto.test\r\n" + for i := 0; i < 512; i++ { + request += "H" + strconv.Itoa(i) + ":v\r\n" + } + request += "\r\n" + + var served bool + var sm MuxSlice + sm.Reset(1) + sm.Handle("GET /echo", func(exch *Exchange) { served = true }) + conn := newConn(request) + conn.Hangup() + exch := newExchange(t, conn, 1024, false) + err := Handle(exch, &sm, nopBackoff) + if err != nil { + t.Fatal(err) + } + if served { + t.Fatal("handler ran on a request the parser could not hold") + } + got := conn.ViewWritten() + if !strings.HasPrefix(got, "HTTP/1.1 431 ") { + t.Errorf("want a 431 answer, got %q", firstLine(got)) + } +} + +func firstLine(s string) string { + if i := strings.Index(s, "\r\n"); i >= 0 { + return s[:i] + } + return s +} + +// A body that already arrived alongside the request header must be handed over +// without touching the connection again. A peer that sent a whole request and +// is waiting for its answer sends nothing more, so a read for bytes already in +// hand blocks until the connection's deadline, or forever without one. +func TestExchangeReadBodyDoesNotReadPastWhatArrived(t *testing.T) { + const body = "message body" + dst := make([]byte, 64) // Deliberately larger than the body. + var got string + var readErr error + var sm MuxSlice + sm.Reset(1) + sm.Handle("POST /", func(ex *Exchange) { + n, err := ex.ReadBody(dst) + got, readErr = string(dst[:n]), err + ex.WriteHeader(200) + }) + // The peer is still there, waiting to be answered: a read for bytes it is + // not going to send blocks, exactly as it does on a socket. + conn := &blockingConn{request: "POST / HTTP/1.1\r\nHost: h\r\nContent-Length: 12\r\n\r\n" + body} + exch := newExchange(t, conn, 1024, false) + done := make(chan struct{}) + go func() { + defer close(done) + Handle(exch, &sm, nopBackoff) + }() + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("ReadBody blocked waiting for a body that had already arrived") + } + if readErr != nil { + t.Fatal(readErr) + } + if got != body { + t.Errorf("want body %q, got %q", body, got) + } +} + +// blockingConn delivers a request and then blocks on reads, the way a peer +// awaiting its answer does. Writes are discarded. +type blockingConn struct { + request string + read int + blocked chan struct{} +} + +func (c *blockingConn) Read(b []byte) (int, error) { + if c.read >= len(c.request) { + if c.blocked == nil { + c.blocked = make(chan struct{}) + } + <-c.blocked // Nothing more is coming, and nothing unblocks this. + return 0, io.EOF + } + n := copy(b, c.request[c.read:]) + c.read += n + return n, nil +} + +func (c *blockingConn) Write(b []byte) (int, error) { return len(b), nil } +func (c *blockingConn) Close() error { return nil } diff --git a/http/httphi/mux.go b/http/httphi/mux.go index 0f1a09d..8d66a6e 100644 --- a/http/httphi/mux.go +++ b/http/httphi/mux.go @@ -5,6 +5,7 @@ import ( "unsafe" "github.com/soypat/lneto" + "github.com/soypat/lneto/http/httpraw" "github.com/soypat/lneto/internal" ) @@ -13,7 +14,7 @@ import ( // Handle does not close the connection on any outcome: the caller owns it. func Handle(exch *Exchange, mux Mux, backoff lneto.BackoffStrategy) error { reqhdr := &exch.reqHdr - reqhdr.Reset(nil) + reqhdr.Reset(nil, 0) // Assume exchange has been configured and reuse memory. var consecutiveBackoffs uint for { n, err := reqhdr.ReadFromLimited(exch.rw, reqhdr.BufferFree()) @@ -31,6 +32,12 @@ func Handle(exch *Exchange, mux Mux, backoff lneto.BackoffStrategy) error { if needMore { continue // Request header split across reads, accumulate the rest. } else if err != nil { + if err == httpraw.ErrHeaderTooMany || err == httpraw.ErrSmallHeaderBuffer { + // The peer is owed an answer: no larger buffer is coming, so + // say so instead of dropping the connection, RFC 6585 5. + exch.StageHeader("Content-Length", "0") + exch.WriteHeader(int(StatusRequestHeaderFieldsTooLarge)) + } return err } break // Done! diff --git a/http/httphi/router.go b/http/httphi/router.go index 0e25263..08a9f32 100644 --- a/http/httphi/router.go +++ b/http/httphi/router.go @@ -44,14 +44,15 @@ type conn = io.ReadWriteCloser // Methods are safe for concurrent use. The zero value is not usable: configure // it first. type Router struct { - mu sync.Mutex - gen atomic.Uint32 - numGoro int - reqBuf int - respBuf int - normalizeKeys bool - pendingConns chan job - mux Mux + mu sync.Mutex + gen atomic.Uint32 + numGoro int + reqBuf int + respBuf int + reqNumHeaderCap int + normalizeKeys bool + pendingConns chan job + mux Mux globbuf []byte exchs []Exchange @@ -79,6 +80,8 @@ type RouterConfig struct { // "HTTP/1.1 200 OK\r\n" does not count towards this memory, only actual Headers key/value pairs use this memory. // After memory is fully consumed [Exchange.StageHeader] will not append more headers. ResponseHeaderMinBufferSize int + // Number of request header key/value pairs to parse before failing and returning [StatusRequestHeaderFieldsTooLarge]. + RequestNumHeaderCap int // NormalizeOutgoingKeys normalizes response header field keys as they are // staged, i.e: "content-type" becomes "Content-Type". @@ -154,6 +157,7 @@ func (r *Router) Configure(cfg RouterConfig) error { gen := r.gen.Load() numgoro := cfg.FixedNumGoroutines workerMode := cfg.workerMode() + r.reqNumHeaderCap = cfg.RequestNumHeaderCap r.reqBuf = cfg.RequestHeaderBufferSize r.respBuf = cfg.ResponseHeaderMinBufferSize r.mux = cfg.Mux @@ -183,7 +187,13 @@ func (r *Router) Configure(cfg RouterConfig) error { for i := range numgoro { // TODO exchange buffer alloc goff := i * rawBuflen - r.exchs[i].Configure(r.globbuf[goff:goff+rawBuflen], cfg.RequestHeaderBufferSize, cfg.NormalizeOutgoingKeys) + // r.globbuf[goff:goff+rawBuflen], cfg.RequestHeaderBufferSize, cfg.RequestNumHeaderCap, cfg.NormalizeOutgoingKeys + r.exchs[i].Configure(ExchangeConfig{ + RawBuf: r.globbuf[goff : goff+rawBuflen], + RequestBufferLim: cfg.RequestHeaderBufferSize, + NumHeaderCap: cfg.RequestNumHeaderCap, + NormalizeOutgoingKeys: cfg.NormalizeOutgoingKeys, + }) go r.goroWorker(gen, jobqueue, cfg.Backoff, cfg.Mux) } r.pendingConns = jobqueue @@ -324,9 +334,14 @@ func (r *Router) getExchLocked(conn conn) (exch *Exchange) { } } - if r.numGoro == 0 { + if r.numGoro == -1 { exch := new(Exchange) - exch.Configure(make([]byte, r.respBuf+r.reqBuf), r.reqBuf, r.normalizeKeys) + exch.Configure(ExchangeConfig{ + RawBuf: make([]byte, r.respBuf+r.reqBuf), + RequestBufferLim: r.reqBuf, + NumHeaderCap: r.reqNumHeaderCap, + NormalizeOutgoingKeys: r.normalizeKeys, + }) exch.Acquire(conn) // Fresh exchange, CAS cannot fail. return exch } diff --git a/http/httpraw/header.go b/http/httpraw/header.go index c85d1dc..868b449 100644 --- a/http/httpraw/header.go +++ b/http/httpraw/header.go @@ -65,10 +65,12 @@ type Header struct { // Flags returns [Flags] to signal status code has been set, Connection:Close or other useful signals provided by flags. func (h *Header) Flags() Flags { return h.flags } -// EnableBufferGrowth disables buffer growth during parsing if b is false. Is enabled by default. -// Disabling buffer growth prevents allocations but methods may throw errors on insufficient memory. -func (h *Header) EnableBufferGrowth(b bool) { - if !b { +// ConfigBufferGrowth configures the memory the header may use. Setting +// outlives [Header.Reset]. Call before parsing/reading. +// +// enableBufferGrowth enables growing both the header buffer and the header key/value pair slice. +func (h *Header) ConfigBufferGrowth(enableBufferGrowth bool) { + if !enableBufferGrowth { h.flags |= flagNoBufferGrow } else { h.flags &^= flagNoBufferGrow @@ -77,7 +79,7 @@ func (h *Header) EnableBufferGrowth(b bool) { // ParseBytes copies the bytes into buffer and parses the HTTP header. It fails if HTTP header data is incomplete. func (h *Header) ParseBytes(asResponse bool, b []byte) error { - h.Reset(nil) + h.Reset(nil, 0) h.hbuf.readFromBytes(b) return h.parse(asResponse) } @@ -86,7 +88,7 @@ func (h *Header) ParseBytes(asResponse bool, b []byte) error { // It fails if HTTP data is incomplete. func (h *Header) Parse(asResponse bool) error { debuglog("http:parse:reset") - h.Reset(h.hbuf.buf) + h.Reset(h.hbuf.buf, 0) debuglog("http:parse:start") return h.parse(asResponse) } @@ -119,7 +121,7 @@ func (h *Header) TryParse(asResponse bool) (needMoreData bool, err error) { return err == ErrNeedMoreData, err } } - err = h.parseNextHeaders() + err = h.parseNextHeaders(h.flags) return err == ErrNeedMoreData, err } @@ -133,7 +135,7 @@ func (h *Header) ParsingSuccess() bool { // If read is successful (read length>0) and reader returns [io.EOF] then ReadFromLimited will return a nil error. func (h *Header) ReadFromLimited(r io.Reader, maxBytesToRead int) (int, error) { if maxBytesToRead <= 0 { - return 0, errSmallBuffer + return 0, ErrSmallHeaderBuffer } else if h.flags.HasAny(flagMangledBuffer) { return 0, errMangledBuffer } else if h.flags.HasAny(flagReaderEOF) { @@ -142,14 +144,14 @@ func (h *Header) ReadFromLimited(r io.Reader, maxBytesToRead int) (int, error) { free := h.BufferFree() if free < maxBytesToRead { if h.flags.HasAny(flagNoBufferGrow) { - return 0, errSmallBuffer + return 0, ErrSmallHeaderBuffer } h.hbuf.buf = slices.Grow(h.hbuf.buf, maxBytesToRead) } blen := len(h.hbuf.buf) b := h.hbuf.buf[blen:min(blen+maxBytesToRead, cap(h.hbuf.buf))] if len(b) == 0 { - return 0, errSmallBuffer + return 0, ErrSmallHeaderBuffer } n, err := r.Read(b) if err != nil && err == io.EOF { @@ -166,12 +168,12 @@ func (h *Header) ReadFromLimited(r io.Reader, maxBytesToRead int) (int, error) { // Used to accumulate HTTP header for later parsing with [Header.TryParse]. func (h *Header) ReadFromBytes(b []byte) (int, error) { if len(b) == 0 { - return 0, errSmallBuffer + return 0, ErrSmallHeaderBuffer } free := h.BufferFree() if free < len(b) { if h.flags.HasAny(flagNoBufferGrow) { - return 0, errSmallBuffer + return 0, ErrSmallHeaderBuffer } h.hbuf.buf = slices.Grow(h.hbuf.buf, len(b)) } @@ -253,13 +255,13 @@ func (hb *headerBuf) forEach(cb func(key, value []byte) error) error { // h.Reset(prealloc[:0]); h.ParseBytes(httpHeader) // Tell header to use a pre-allocated buffer capacity. // h.Reset(httpHeader); h.Parse() // Parse bytes in place with no copying. // h.Reset(nil) // Reuse buffer previously set in a call to Reset. -func (h *Header) Reset(buf []byte) { - if h.flags.HasAny(flagNoBufferGrow) && cap(buf) < 32 { - panic("small buffer and flagNoBufferGrow set") - } +func (h *Header) Reset(buf []byte, numHeaderCapacity int) { const persistentFlags = flagNoBufferGrow debuglog("http:reset:hbuf") - h.hbuf.reset(buf) + h.hbuf.reset(buf, numHeaderCapacity) + if h.flags.HasAny(flagNoBufferGrow) && cap(h.hbuf.buf) < 32 { + panic("small buffer and flagNoBufferGrow set") + } *h = Header{ hbuf: h.hbuf, flags: h.flags & persistentFlags, diff --git a/http/httpraw/header_test.go b/http/httpraw/header_test.go index edc8bd0..c7cf12f 100644 --- a/http/httpraw/header_test.go +++ b/http/httpraw/header_test.go @@ -2,6 +2,7 @@ package httpraw import ( "bytes" + "errors" "fmt" "net/http" "strconv" @@ -10,6 +11,8 @@ import ( "time" ) +const numHeaderCapacity = 16 + func TestHeaderParseRequest(t *testing.T) { const ( wantMethod = "GET" @@ -387,7 +390,7 @@ func TestCopyDecodedPercentURLInPlace(t *testing.T) { func TestHeaderSetOverwrite(t *testing.T) { var h Header - h.Reset(nil) + h.Reset(nil, numHeaderCapacity) h.SetMethod("GET") h.SetRequestTarget("/") h.SetProtocol("HTTP/1.1") @@ -410,7 +413,7 @@ func TestHeaderSetOverwrite(t *testing.T) { func TestHeaderSetBytesEmptyValue(t *testing.T) { var h Header - h.Reset(nil) + h.Reset(nil, numHeaderCapacity) h.SetBytes("X-Empty", nil) if got := h.Get("X-Empty"); len(got) != 0 { t.Errorf("want empty value, got %q", got) @@ -465,7 +468,7 @@ func TestHeader_SplitBeforeColonStillParses(t *testing.T) { const part2 = ": example.com\r\n\r\n" var h Header - h.Reset(nil) + h.Reset(nil, numHeaderCapacity) if _, err := h.ReadFromBytes([]byte(part1)); err != nil { t.Fatal(err) } @@ -498,7 +501,7 @@ func TestHeader_AppendHeaderExactCapNoPanic(t *testing.T) { const key, value = "K", "V" buf := make([]byte, 0, len(key)+len(value)) // exact cap, no slack. var h Header - h.Reset(buf) + h.Reset(buf, numHeaderCapacity) defer func() { if r := recover(); r != nil { t.Fatalf("appendHeader panicked on exact-cap buffer: %v", r) @@ -515,8 +518,8 @@ func TestHeader_AppendHeaderExactCapNoPanic(t *testing.T) { func TestHeader_AddFullBufferNoPanic(t *testing.T) { buf := make([]byte, 0, 40) // Small cap; enough for Reset (len 0) but not the field below. var h Header - h.Reset(buf) - h.EnableBufferGrowth(false) + h.Reset(buf, numHeaderCapacity) + h.ConfigBufferGrowth(false) h.SetMethod("GET") h.SetRequestTarget("/") h.SetProtocol("HTTP/1.1") @@ -550,7 +553,7 @@ func TestHeader_SetInt(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { var h Header - h.Reset(nil) + h.Reset(nil, numHeaderCapacity) h.SetInt("Content-Length", tc.value, tc.base) if got := string(h.Get("Content-Length")); got != tc.want { t.Fatalf("want %q, got %q", tc.want, got) @@ -562,7 +565,7 @@ func TestHeader_SetInt(t *testing.T) { // SetInt on an existing key must reuse the slot in place (single field, latest value). func TestHeader_SetIntOverwrite(t *testing.T) { var h Header - h.Reset(nil) + h.Reset(nil, numHeaderCapacity) h.SetMethod("GET") h.SetRequestTarget("/") h.SetProtocol("HTTP/1.1") @@ -586,8 +589,8 @@ func TestHeader_SetIntOverwrite(t *testing.T) { func TestHeader_SetIntNoAlloc(t *testing.T) { buf := make([]byte, 0, 256) var h Header - h.Reset(buf) - h.EnableBufferGrowth(false) + h.Reset(buf, numHeaderCapacity) + h.ConfigBufferGrowth(false) h.Add("Content-Length", "0000000000000000000000") // pre-size a reusable slot. allocs := testing.AllocsPerRun(100, func() { h.SetInt("Content-Length", 1234567890, 10) @@ -599,3 +602,43 @@ func TestHeader_SetIntNoAlloc(t *testing.T) { t.Fatalf("want %q, got %q", "1234567890", got) } } + +// A browser sends upwards of twenty header fields and an API client with a few +// custom fields is not far behind. The field table must be sized from the +// buffer the caller handed over, not fixed at a count that a real request +// exceeds. +func TestHeader_FieldTableSizedFromBuffer(t *testing.T) { + const wantVal = "the-canary-value" + raw := "GET / HTTP/1.1\r\nHost: lneto.test\r\n" + for i := 0; i < 40; i++ { + raw += "X-Field-" + strconv.Itoa(i) + ": value-of-a-realistic-length-here\r\n" + } + raw += "X-Canary: " + wantVal + "\r\n\r\n" + + var h Header + h.Reset(make([]byte, 0, 8192), numHeaderCapacity) // Room for the block with plenty to spare. + err := h.ParseBytes(false, []byte(raw)) + if err != nil { + t.Fatalf("parsing a 42 field request into an 8kB buffer: %s", err) + } + if got := string(h.Get("X-Canary")); got != wantVal { + t.Fatalf("want X-Canary %q, got %q", wantVal, got) + } +} + +// A buffer too small for the fields it is handed must be refused with an error +// the caller can act on, so a server answers 431 instead of dropping the peer. +func TestHeader_FieldTableFullIsReported(t *testing.T) { + raw := "GET / HTTP/1.1\r\n" + for i := 0; i < 64; i++ { + raw += "H" + strconv.Itoa(i) + ":v\r\n" // As short as a field gets. + } + raw += "\r\n" + var h Header + h.Reset(make([]byte, 0, 512), numHeaderCapacity) + h.ConfigBufferGrowth(false) + err := h.ParseBytes(false, []byte(raw)) + if !errors.Is(err, ErrHeaderTooMany) { + t.Fatalf("want ErrHeaderFieldsTooLarge, got %v", err) + } +} diff --git a/http/httpraw/parse.go b/http/httpraw/parse.go index 8ae5cf3..5752c59 100644 --- a/http/httpraw/parse.go +++ b/http/httpraw/parse.go @@ -14,12 +14,17 @@ var ( errNoProto = errors.New("missing protocol, HTTP/0.9 unsupported") // ErrNeedMoreData signals a parser was handed an incomplete buffer: append // more data to it and call again. - ErrNeedMoreData = errors.New("need more data: cannot find trailing lf/delimiter") - errNoBoundary = errors.New("httpraw: multipart boundary not set") - errUnparsed = errors.New("need to finish parsing") - errInvalidName = errors.New("invalid header name") - errSmallBuffer = errors.New("small read buffer. Increase ReadBufferSize") - errOOM = errors.New("httpraw: buffer out of memory") + ErrNeedMoreData = errors.New("need more data: cannot find trailing lf/delimiter") + errNoBoundary = errors.New("httpraw: multipart boundary not set") + errUnparsed = errors.New("need to finish parsing") + errInvalidName = errors.New("invalid header name") + ErrSmallHeaderBuffer = errors.New("httpraw: Header buffer exhausted, increase size") + errOOM = errors.New("httpraw: Header incomplete due to OOM") + // ErrHeaderTooMany signals a header block carrying more fields than + // the buffer it is parsed into has room for, see [Header.Reset]. A server + // answers it with 431, RFC 6585 5: no larger buffer is coming, so reading + // the rest of the block would only spend memory on a request already lost. + ErrHeaderTooMany = errors.New("httpraw: more header fields than buffer holds") // Header.Set and Header.Add mangles the buffer. // Call them after retrieving the Body. Do not call them before parsing the header (why would you even do that?). errMangledBuffer = errors.New("httpraw: mangled buffer") @@ -52,17 +57,21 @@ type headerBuf struct { headers []argsKV } -// reset sets the buffer data and discards all parsed data. -func (h *headerBuf) reset(buf []byte) { +// reset sets the buffer data and discards all parsed data. The field table is +// grown to match the new buffer's capacity and never shrinks, so a header +// reused across requests settles on its largest buffer and stops allocating. +func (h *headerBuf) reset(buf []byte, numHeaderCapacity int) { if buf == nil { buf = h.buf[:0] // Reuse buffer but discard raw data on nil input. } - if cap(h.headers) == 0 { - h.headers = make([]argsKV, 16) + if numHeaderCapacity != 0 { + internal.SliceReuse(&h.headers, numHeaderCapacity) + } else { + h.headers = h.headers[:0] } *h = headerBuf{ buf: buf, - headers: h.headers[:0], + headers: h.headers, } } @@ -100,7 +109,7 @@ func (h *Header) parse(asResponse bool) (err error) { return err } debuglog("http:firstline:done") - err = h.parseNextHeaders() + err = h.parseNextHeaders(h.flags) debuglog("http:headers:done") return err } @@ -117,9 +126,9 @@ func (h *Header) parseFirstLine(asResponse bool) (err error) { return err } -func (h *Header) parseNextHeaders() error { +func (h *Header) parseNextHeaders(flags Flags) error { var ss scannerState - h.hbuf.parseNextHeaders(&ss) + h.hbuf.parseNextHeaders(&ss, flags) if ss.err != nil { h.flags |= flagConnClose return ss.err @@ -134,13 +143,13 @@ func (hb *headerBuf) readFromBytes(b []byte) { func (hb *headerBuf) free() int { return cap(hb.buf) - len(hb.buf) } -func (hb *headerBuf) parseNextHeaders(ss *scannerState) { +func (hb *headerBuf) parseNextHeaders(ss *scannerState, flags Flags) { debuglog("http:nexthdr:loop") for kv := hb.next(ss); kv.isValid(); kv = hb.next(ss) { - if len(hb.headers) == cap(hb.headers) { - // Refuse to grow the headers slice: caller must pre-allocate - // sufficient capacity via reset or use a larger initial size. - ss.err = errOOM + if len(hb.headers) == cap(hb.headers) && flags.HasAny(flagNoBufferGrow) { + // Refuse to grow the headers slice: the caller granted this much + // memory and no more, see [Header.Reset]. + ss.err = ErrHeaderTooMany return } hb.headers = append(hb.headers, kv) diff --git a/http/httpraw/parse_test.go b/http/httpraw/parse_test.go index f0ef65c..ae66cc7 100644 --- a/http/httpraw/parse_test.go +++ b/http/httpraw/parse_test.go @@ -10,7 +10,7 @@ func TestTryParse_IncrementalRequest(t *testing.T) { // Full HTTP request split across multiple ReadFromBytes calls. full := "GET /index.html HTTP/1.1\r\nHost: example.com\r\nContent-Type: text/html\r\n\r\nbody here" var hdr Header - hdr.Reset(make([]byte, 0, 256)) + hdr.Reset(make([]byte, 0, 256), numHeaderCapacity) // Feed data in small chunks to exercise incremental parsing. chunks := splitInto(full, 10) @@ -85,7 +85,7 @@ func TestTryParse_IncrementalRequest(t *testing.T) { func TestTryParse_IncrementalResponse(t *testing.T) { full := "HTTP/1.1 200 OK\r\nContent-Length: 5\r\nServer: lneto\r\n\r\nhello" var hdr Header - hdr.Reset(make([]byte, 0, 256)) + hdr.Reset(make([]byte, 0, 256), numHeaderCapacity) chunks := splitInto(full, 8) var done bool @@ -137,7 +137,7 @@ func TestReadFromLimited(t *testing.T) { r := strings.NewReader(data) var hdr Header - hdr.Reset(make([]byte, 0, 256)) + hdr.Reset(make([]byte, 0, 256), numHeaderCapacity) // Read in one shot. n, err := hdr.ReadFromLimited(r, 256) @@ -163,7 +163,7 @@ func TestReadFromLimited(t *testing.T) { func TestReadFromLimited_MaxBytes(t *testing.T) { var hdr Header - hdr.Reset(make([]byte, 0, 256)) + hdr.Reset(make([]byte, 0, 256), numHeaderCapacity) // Zero maxBytesToRead should error. _, err := hdr.ReadFromLimited(strings.NewReader("data"), 0) @@ -174,7 +174,7 @@ func TestReadFromLimited_MaxBytes(t *testing.T) { func TestReadFromBytes_Empty(t *testing.T) { var hdr Header - hdr.Reset(make([]byte, 0, 256)) + hdr.Reset(make([]byte, 0, 256), numHeaderCapacity) _, err := hdr.ReadFromBytes(nil) if err == nil { @@ -184,7 +184,7 @@ func TestReadFromBytes_Empty(t *testing.T) { func TestBufferFreeAndCapacity(t *testing.T) { var hdr Header - hdr.Reset(make([]byte, 0, 100)) + hdr.Reset(make([]byte, 0, 100), numHeaderCapacity) if hdr.BufferCapacity() != 100 { t.Errorf("capacity = %d; want 100", hdr.BufferCapacity()) @@ -202,9 +202,8 @@ func TestBufferFreeAndCapacity(t *testing.T) { func TestEnableBufferGrowth(t *testing.T) { var hdr Header buf := make([]byte, 0, 64) - hdr.Reset(buf) - hdr.EnableBufferGrowth(false) - + hdr.Reset(buf, numHeaderCapacity) + hdr.ConfigBufferGrowth(false) // With growth disabled, reading more than capacity should fail. big := make([]byte, 128) for i := range big { @@ -389,7 +388,7 @@ func TestHeader_MultilineValue(t *testing.T) { func TestHeader_ResponseRoundTrip(t *testing.T) { var hdr Header - hdr.Reset(make([]byte, 0, 256)) + hdr.Reset(make([]byte, 0, 256), numHeaderCapacity) hdr.SetProtocol("HTTP/1.1") hdr.SetStatus("404", "Not Found") hdr.Add("Content-Type", "text/plain") @@ -429,7 +428,7 @@ func TestHeader_ResponseRoundTrip(t *testing.T) { func TestHeader_RequestRoundTrip(t *testing.T) { var hdr Header - hdr.Reset(make([]byte, 0, 256)) + hdr.Reset(make([]byte, 0, 256), numHeaderCapacity) hdr.SetProtocol("HTTP/1.1") hdr.SetMethod("POST") hdr.SetRequestTarget("/api/data") diff --git a/examples/http-linux/conn-linux.go b/internal/rawsock/rawsock_linux.go similarity index 100% rename from examples/http-linux/conn-linux.go rename to internal/rawsock/rawsock_linux.go