diff --git a/http/httphi/bench_test.go b/http/httphi/bench_test.go index 4a15e01..4bf5dff 100644 --- a/http/httphi/bench_test.go +++ b/http/httphi/bench_test.go @@ -112,10 +112,13 @@ func BenchmarkRequestParseForm(b *testing.B) { const request = "POST /f HTTP/1.1\r\nHost: tinygo.org\r\n" + "Content-Type: application/x-www-form-urlencoded\r\nContent-Length: 27\r\n\r\n" + "user=gopher&msg=hello+world" - buf := make([]byte, 64) + // The form owns its memory now, so pre-size it and forbid growth: an + // allocation on this path is the failure the benchmark is watching for. + benchForm.Reset(make([]byte, 0, 64), 2) + benchForm.EnableBufferGrowth(false) var mux MuxSlice mux.Handle("POST /f", func(ex *Exchange) { - err := ex.RequestParseForm(&benchForm, buf) + err := ex.RequestParseForm(&benchForm, false, false) if err != nil || benchForm.Len() != 2 { panic("invalid result") } diff --git a/http/httphi/example_test.go b/http/httphi/example_test.go index 1094469..ea3958f 100644 --- a/http/httphi/example_test.go +++ b/http/httphi/example_test.go @@ -77,10 +77,12 @@ func ExampleMuxSlice_query_forms_multipart() { }) mux.Handle("GET /form", func(ex *httphi.Exchange) { - // Request Body Form. - formbuf := make([]byte, 1024) + // Request Body Form. The form owns the memory: hand it a buffer and + // forbid growth to bound what a request may spend. var form httpraw.Form - err := ex.RequestParseForm(&form, formbuf) + form.Reset(make([]byte, 0, 1024), 8) + form.EnableBufferGrowth(false) + err := ex.RequestParseForm(&form, false, false) if err != nil { ex.WriteHeader(httphi.StatusInternalServerError) return diff --git a/http/httphi/exchange.go b/http/httphi/exchange.go index 94caab8..a414769 100644 --- a/http/httphi/exchange.go +++ b/http/httphi/exchange.go @@ -36,6 +36,9 @@ type Exchange struct { respHeaderLen uint16 reqHdr httpraw.Header pathValues []PathValue + // bodyRW is the reader handed to [httpraw.Form.ReadLimited], kept here so + // boxing it into an io.Reader allocates nothing per request. + bodyRW ExchangeRW hijacked bool rw conn @@ -506,83 +509,64 @@ func (exch *Exchange) RequestContentLength() (_ int64, present bool, _ error) { return exch.RequestHeaderRaw().ContentLength() } -func (exch *Exchange) RequestParseForm(dst *httpraw.Form, parseURL, prioritizeURL bool) (err error) { - dst.Reset(nil, 0) - if parseURL { - err = dst.ReadFromBytes(exch.RequestQuery()) - - } - if !parseURL { - cl, ok, err := exch.RequestContentLength() - if cl == 0 { - return nil +// RequestParseForm parses "application/x-www-form-urlencoded" pairs into dst +// from the request body and, when parseURL is set, from the query string as +// well. Pairs are stored as they arrived, call [httpraw.Form.Decode] to decode +// them in place. +// +// dst owns the memory: both sources are read into its buffer and parsed together +// once. Hand it a preallocated buffer with [httpraw.Form.Reset] and turn growth +// off with [httpraw.Form.EnableBufferGrowth] to bound it, which then reports +// [httpraw.ErrBufferExhausted] instead of allocating. It grows by default. +// +// prioritizeURL reads the query ahead of the body, so a key carried by both +// resolves to the query's value: [httpraw.Form.Get] answers with the first pair +// holding a key. Both stay readable in wire order through [httpraw.Form.Pair]. +// The body is consumed, so call this before [Exchange.ReadBody]. +// +// A request with no Content-Length has no body, RFC 9112 6.3, and one with no +// Content-Type declares no encoding to parse, RFC 9110 8.3. Neither is an error, +// a bodiless POST being legal, and the query is still parsed when asked for. A +// Content-Type that is present and not form encoded is [errNotFormEncoded]. +func (exch *Exchange) RequestParseForm(dst *httpraw.Form, parseURL, prioritizeURL bool) error { + dst.Reset(nil, 0) // Reuse whatever buffer dst holds, discarding old pairs. + if parseURL && prioritizeURL { + if err := exch.readQueryForm(dst); err != nil { + return err } - var rw ExchangeRW - exch.ReadWriter(&rw) - _, err = dst.ReadLimited(&rw, int(cl)) - } - + if err := exch.readBodyForm(dst); err != nil { + return err + } + if parseURL && !prioritizeURL { + if err := exch.readQueryForm(dst); err != nil { + return err + } + } + return dst.Parse() } -func (exch *Exchange) readQueryForm(dst *httpraw.Form) (n int, err error) { +// formSeparator joins two sources inside one form buffer. Shared so appending it +// converts no literal per call. +var formSeparator = []byte{'&'} + +// readQueryForm appends the request's query string to dst's buffer. +func (exch *Exchange) readQueryForm(dst *httpraw.Form) error { query := exch.RequestQuery() if len(query) == 0 { - return 0, nil - } else if dst.Len() > 0 { - err = dst.ReadFromBytes([]byte{'&'}) // Add separator. - if err != nil { - return 0, err - } + return nil + } else if err := separateForm(dst); err != nil { + return err } - err = dst.ReadFromBytes(query) - if err != nil { - return 0, err - } - return len(query), nil + return dst.ReadFromBytes(query) } -func (exch *Exchange) readBodyForm(dst *httpraw.Form) (n int, err error) { - cl, _, err := exch.RequestContentLength() - if cl <= 0 { - return 0, nil - } - if dst.Len() > 0 { - err = dst.ReadFromBytes([]byte{'&'}) // Add separator. - if err != nil { - return 0, err - } - } - var rw ExchangeRW - exch.ReadWriter(&rw) - n, err = dst.ReadLimited(&rw, int(cl)) - if err != nil { - return n, err - } - return n, nil -} - -// RequestParseForm reads the request body into buf and parses it as -// "application/x-www-form-urlencoded" into dst. buf is the only storage used and -// the only limit: a body longer than buf is refused with [lneto.ErrBufferFull] -// before a single byte is read, leaving the caller free to answer 413. Pairs are -// left as they arrived, call [httpraw.Form.Decode] to decode them in place. -// -// Unlike http.Request.ParseForm the query string is not folded in, reach it with -// [Exchange.RequestQuery] or [Exchange.RequestQueryAppend]. The body is consumed, so -// call this before [Exchange.ReadBody]. -// -// A request with no Content-Length has no body, RFC 9112 6.3, and a request with -// no Content-Type declares no encoding to parse, RFC 9110 8.3. Both yield an -// empty form and a nil error, a bodiless POST being legal. Use -// [Exchange.RequestContentLength] and [Exchange.RequestContentType] to tell -// either apart from a body that arrived empty. A Content-Type that is present -// and not form encoded is [errNotFormEncoded], an absent one never is. -func (exch *Exchange) RequestParseForm(dst *httpraw.Form, buf []byte) error { +// readBodyForm appends the request body to dst's buffer, reading until +// Content-Length bytes have arrived. +func (exch *Exchange) readBodyForm(dst *httpraw.Form) error { contentType := exch.RequestContentType() if contentType == nil { - dst.Reset(nil, 0) - return nil // No declared encoding is no form, as no length is no body. + return nil // No declared encoding is no form, RFC 9110 8.3. } else if !httpraw.MediaTypeIs(contentType, "application/x-www-form-urlencoded") { return errNotFormEncoded } else if exch.RequestHeaderRaw().GetFold("Transfer-Encoding") != nil { @@ -590,31 +574,44 @@ func (exch *Exchange) RequestParseForm(dst *httpraw.Form, buf []byte) error { // wire would parse chunk sizes as form data. httpraw does not decode them. return errUnsupportedTransferCoding } - length, present, err := exch.RequestContentLength() - if !present { - dst.Reset(nil, 0) - return nil // No length is no body, RFC 9112 6.3. - } else if err != nil { + if err != nil { return err - } else if length > int64(len(buf)) { - return lneto.ErrShortBuffer // Refuse before reading, caller may answer 413. + } else if !present || length == 0 { + return nil // No length is no body, RFC 9112 6.3. } - buf = buf[:length] - for read := 0; read < len(buf); { - n, err := exch.ReadBody(buf[read:]) + if err = separateForm(dst); err != nil { + return err + } + // Reuse the exchange's own handle: a local would escape when boxed into the + // io.Reader [httpraw.Form.ReadLimited] takes, costing an allocation a request. + exch.ReadWriter(&exch.bodyRW) + // A single read may fall short of the limit, the body arriving a TCP segment + // at a time, so read until the declared length is in hand. + for read := 0; read < int(length); { + n, err := dst.ReadLimited(&exch.bodyRW, int(length)-read) read += n if n == 0 { if err == nil { err = io.ErrNoProgress } else if err == io.EOF { - break + break // Peer sent less than it declared. } return err + } else if err != nil && err != io.EOF { + return err } } - dst.Reset(buf, 0) - return dst.Parse() + return nil +} + +// separateForm appends the '&' keeping two sources from merging into one pair, +// doing nothing while dst holds no bytes yet. +func separateForm(dst *httpraw.Form) error { + if dst.BufferUsed() == 0 { + return nil + } + return dst.ReadFromBytes(formSeparator) } // RequestMultipart returns a parser prepared from the boundary parameter of the diff --git a/http/httphi/exchange_test.go b/http/httphi/exchange_test.go index d53f793..585f5d9 100644 --- a/http/httphi/exchange_test.go +++ b/http/httphi/exchange_test.go @@ -824,10 +824,12 @@ func TestExchangeRequestParseForm(t *testing.T) { extraHeaders: "Transfer-Encoding: chunked\r\n", wantErr: errUnsupportedTransferCoding, }, { + // The form bounds itself now, so an oversized body is the form + // refusing to grow rather than a short buffer handed in. name: "body larger than buffer", formVals: []formPair{{key: "a", value: "1"}, {key: "b", value: "2"}, {key: "c", value: "3"}}, bufsize: 4, - wantErr: lneto.ErrShortBuffer, + wantErr: httpraw.ErrBufferExhausted, }, } { t.Run(test.name, func(t *testing.T) { @@ -867,12 +869,16 @@ func TestExchangeRequestParseForm(t *testing.T) { builder.WriteString("\r\n") builder.Write(body) + // The form owns the memory: bufSize bounds it here, growth off so an + // oversized body is reported rather than allocated for. var form httpraw.Form + form.Reset(make([]byte, 0, bufSize), 8) + form.EnableBufferGrowth(false) var gotErr error var sm MuxSlice sm.Reset(1) sm.Handle("/f", func(exch *Exchange) { - gotErr = exch.RequestParseForm(&form, make([]byte, bufSize)) + gotErr = exch.RequestParseForm(&form, false, false) if gotErr == nil && test.callDecode { gotErr = form.Decode() } @@ -915,7 +921,7 @@ func TestExchangeRequestParseFormSplit(t *testing.T) { var sm MuxSlice sm.Reset(1) sm.Handle("/f", func(exch *Exchange) { - gotErr = exch.RequestParseForm(&form, make([]byte, 64)) + gotErr = exch.RequestParseForm(&form, false, false) }) exch := newExchange(t, conn, ExchangeConfig{RawBuf: make([]byte, 2*1024), RequestBufferLim: 1024}) if err := Handle(exch, &sm, nopBackoff); err != nil { @@ -935,7 +941,7 @@ func TestExchangeRequestParseFormDecode(t *testing.T) { var sm MuxSlice sm.Reset(1) sm.Handle("/f", func(exch *Exchange) { - if err := exch.RequestParseForm(&form, make([]byte, 64)); err != nil { + if err := exch.RequestParseForm(&form, false, false); err != nil { t.Error(err) } else if err = form.Decode(); err != nil { t.Error(err) @@ -1435,7 +1441,7 @@ func TestExchangeRequestContentTypeFolded(t *testing.T) { sm.Reset(1) sm.Handle("/f", func(exch *Exchange) { gotType = string(exch.RequestContentType()) - gotErr = exch.RequestParseForm(&form, make([]byte, 64)) + gotErr = exch.RequestParseForm(&form, false, false) }) serve(t, "POST /f HTTP/1.1\r\nHost: h\r\n"+name+": "+formType+"\r\nContent-Length: 3\r\n\r\na=1", &sm) @@ -1464,7 +1470,7 @@ func TestExchangeRequestParseFormFoldedTransferEncoding(t *testing.T) { var sm MuxSlice sm.Reset(1) sm.Handle("/f", func(exch *Exchange) { - gotErr = exch.RequestParseForm(&form, make([]byte, 64)) + gotErr = exch.RequestParseForm(&form, false, false) }) serve(t, "POST /f HTTP/1.1\r\nHost: h\r\nContent-Type: application/x-www-form-urlencoded\r\n"+ name+": chunked\r\nContent-Length: "+strconv.Itoa(len(body))+"\r\n\r\n"+body, &sm) @@ -1576,3 +1582,95 @@ func TestExchangeRespondReportsOverflow(t *testing.T) { t.Error("want the failure recorded on the exchange too") } } + +// Query and body are read into one form buffer and parsed together, so both +// sources are present at once and read order decides which value a key resolves +// to. A key carried by both keeps both pairs, in wire order. +func TestExchangeRequestParseFormFoldsQuery(t *testing.T) { + const body = "cnt=body&only=b" + const target = "/f?cnt=query&page=2" + for _, test := range []struct { + name string + parseURL, prioritizeURL bool + wantCnt string + wantPage string + wantRendered string + }{ + { + name: "body only", parseURL: false, + wantCnt: "body", wantPage: "", wantRendered: "cnt=body|only=b", + }, + { + name: "query first wins", parseURL: true, prioritizeURL: true, + wantCnt: "query", wantPage: "2", wantRendered: "cnt=query|page=2|cnt=body|only=b", + }, + { + name: "body first wins", parseURL: true, prioritizeURL: false, + wantCnt: "body", wantPage: "2", wantRendered: "cnt=body|only=b|cnt=query|page=2", + }, + } { + t.Run(test.name, func(t *testing.T) { + var form httpraw.Form + var gotErr error + var sm MuxSlice + sm.Reset(1) + sm.Handle("POST /f", func(exch *Exchange) { + gotErr = exch.RequestParseForm(&form, test.parseURL, test.prioritizeURL) + }) + serve(t, "POST "+target+" HTTP/1.1\r\nHost: h\r\n"+ + "Content-Type: application/x-www-form-urlencoded\r\n"+ + "Content-Length: "+strconv.Itoa(len(body))+"\r\n\r\n"+body, &sm) + if gotErr != nil { + t.Fatalf("RequestParseForm: %s", gotErr) + } + if got := string(form.Get("cnt")); got != test.wantCnt { + t.Errorf("want cnt=%q, got %q", test.wantCnt, got) + } + if got := string(form.Get("page")); got != test.wantPage { + t.Errorf("want page=%q, got %q", test.wantPage, got) + } + if got := formString(&form); got != test.wantRendered { + t.Errorf("want pairs %q, got %q", test.wantRendered, got) + } + }) + } +} + +// A GET with a query and no body must fold the query alone: no Content-Type +// means no body to parse, which is not an error. +func TestExchangeRequestParseFormQueryWithoutBody(t *testing.T) { + var form httpraw.Form + var gotErr error + var sm MuxSlice + sm.Reset(1) + sm.Handle("GET /f", func(exch *Exchange) { + gotErr = exch.RequestParseForm(&form, true, true) + }) + serve(t, "GET /f?a=1&b=2 HTTP/1.1\r\nHost: h\r\n\r\n", &sm) + if gotErr != nil { + t.Fatalf("want the query parsed with no body, got %s", gotErr) + } + if got := formString(&form); got != "a=1|b=2" { + t.Errorf("want a=1|b=2, got %q", got) + } +} + +// The separator must not merge the two sources into one pair: without it the +// last query pair and the first body pair run together. +func TestExchangeRequestParseFormSourcesNotMerged(t *testing.T) { + var form httpraw.Form + var sm MuxSlice + sm.Reset(1) + sm.Handle("POST /f", func(exch *Exchange) { + if err := exch.RequestParseForm(&form, true, true); err != nil { + t.Fatal(err) + } + }) + const body = "second=2" + serve(t, "POST /f?first=1 HTTP/1.1\r\nHost: h\r\n"+ + "Content-Type: application/x-www-form-urlencoded\r\n"+ + "Content-Length: "+strconv.Itoa(len(body))+"\r\n\r\n"+body, &sm) + if got := formString(&form); got != "first=1|second=2" { + t.Errorf("want first=1|second=2, got %q", got) + } +} diff --git a/http/httphi/httphi_fuzz_test.go b/http/httphi/httphi_fuzz_test.go index a4c07b9..99c2e06 100644 --- a/http/httphi/httphi_fuzz_test.go +++ b/http/httphi/httphi_fuzz_test.go @@ -251,7 +251,7 @@ func FuzzQueryAndForm(f *testing.F) { var form httpraw.Form buf := make([]byte, scratchLen) - if err := exch.RequestParseForm(&form, buf); err != nil { + if err := exch.RequestParseForm(&form, false, false); err != nil { return } total := 0 diff --git a/http/httpraw/form.go b/http/httpraw/form.go index 5082a2e..f186427 100644 --- a/http/httpraw/form.go +++ b/http/httpraw/form.go @@ -22,6 +22,10 @@ func (f *Form) EnableBufferGrowth(enableGrowth bool) { f.kv.EnableBufferGrowth(e // Reset discards parsed pairs and sets the buffer to parse in place. // If buf is nil the current buffer is reused. +// +// capKV sizes the pair table. With growth disabled it is a hard limit, so a +// capKV of 0 leaves no room for a single pair and [Form.Parse] answers +// [ErrBufferExhausted]; size it to the pairs expected. func (f *Form) Reset(buf []byte, capKV int) { f.kv.Reset(buf, capKV) } @@ -29,6 +33,10 @@ func (f *Form) Reset(buf []byte, capKV int) { // ReadFromBytes appends buf to the underlying buffer, accumulating data to parse. Returns ErrBufferExhausted when buf does not fit and growth is disabled. func (f *Form) ReadFromBytes(b []byte) error { return f.kv.ReadFromBytes(b) } +// BufferUsed returns bytes accumulated by the Read* methods and awaiting a +// [Form.Parse]. See [kvBuffer.BufferUsed]. +func (f *Form) BufferUsed() int { return f.kv.BufferUsed() } + // ReadLimited appends at most limit bytes read from r to the underlying buffer. A read returning data alongside io.EOF reports a nil error, later ones io.EOF. func (f *Form) ReadLimited(r io.Reader, limit int) (int, error) { return f.kv.ReadLimited(r, limit) } diff --git a/http/httpraw/form_test.go b/http/httpraw/form_test.go index 7545118..7a920b0 100644 --- a/http/httpraw/form_test.go +++ b/http/httpraw/form_test.go @@ -155,3 +155,44 @@ func TestFormParseReuseNoAlloc(t *testing.T) { t.Errorf("reused Form allocated %v times, want 0", allocs) } } + +// BufferUsed reports buffered bytes, not parsed pairs, so a caller appending +// from several sources can tell whether a separator is needed before the next +// one. Form.Len is zero until Parse runs and cannot answer that. +func TestFormBufferUsed(t *testing.T) { + var f Form + f.Reset(nil, 0) + if got := f.BufferUsed(); got != 0 { + t.Errorf("want 0 on a fresh form, got %d", got) + } + if err := f.ReadFromBytes([]byte("a=1")); err != nil { + t.Fatal(err) + } + if got := f.BufferUsed(); got != 3 { + t.Errorf("want 3 buffered, got %d", got) + } + if got := f.Len(); got != 0 { + t.Errorf("Len must stay 0 until Parse, got %d", got) + } + // A second source appended behind a separator. + if err := f.ReadFromBytes([]byte("&b=2")); err != nil { + t.Fatal(err) + } + if got := f.BufferUsed(); got != 7 { + t.Errorf("want 7 buffered, got %d", got) + } + if err := f.Parse(); err != nil { + t.Fatal(err) + } + if got := render(&f); got != "a=1|b=2" { + t.Errorf("want a=1|b=2, got %q", got) + } + if got := f.BufferUsed(); got != 7 { + t.Errorf("BufferUsed must not change on Parse, got %d", got) + } + // Reset discards the pairs and the buffered bytes with them. + f.Reset(nil, 0) + if got := f.BufferUsed(); got != 0 { + t.Errorf("want 0 after Reset, got %d", got) + } +} diff --git a/http/httpraw/kvbuffer.go b/http/httpraw/kvbuffer.go index 260929c..01c2da2 100644 --- a/http/httpraw/kvbuffer.go +++ b/http/httpraw/kvbuffer.go @@ -23,6 +23,11 @@ func (kvb *kvBuffer) free() int { return cap(kvb.buf) - len(kvb.buf) } // Stored pairs alias it, so writing to it mangles them. func (kvb *kvBuffer) BufferRaw() []byte { return kvb.buf } +// BufferUsed returns the raw memory used, which is what a caller appending from +// several sources checks to know whether a separator is needed. Counts buffered +// bytes and not parsed pairs, so it is set before a Parse and unchanged by one. +func (kvb *kvBuffer) BufferUsed() int { return len(kvb.buf) } + // EnableBufferGrowth allows the buffer to grow past the memory [kvBuffer.Reset] // was handed. The setting outlives Reset; with growth off callers get [ErrBufferExhausted]. func (kvb *kvBuffer) EnableBufferGrowth(enableGrowth bool) {