diff --git a/http/httphi/exchange_test.go b/http/httphi/exchange_test.go index 3f4b045..b69fca2 100644 --- a/http/httphi/exchange_test.go +++ b/http/httphi/exchange_test.go @@ -4,6 +4,8 @@ import ( "context" "errors" "strings" + + "github.com/soypat/lneto/http/httpraw" "testing" "time" @@ -461,10 +463,11 @@ func TestHandleMuxOnPath(t *testing.T) { var gotPath, gotQuery string sm.Handle("GET /search", func(ex *Exchange) { gotPath = string(ex.RequestPath()) - ex.ForEachQueryRaw(func(rawkey, rawval []byte) bool { + rawkey, rawval, rest := httpraw.NextQueryPair(ex.RequestQuery()) + for rawkey != nil { gotQuery += string(rawkey) + "=" + string(rawval) + ";" - return true - }) + rawkey, rawval, rest = httpraw.NextQueryPair(rest) + } ex.WriteHeader(200) }) conn := serve(t, "GET /search?q=go&n=1 HTTP/1.1\r\nHost: h\r\n\r\n", &sm) @@ -490,9 +493,9 @@ func TestExchangeAppendQuery(t *testing.T) { }{ {uri: "/x?q=go", key: "q", want: "go", wantPresent: true}, {uri: "/x?q=go&n=1", key: "n", want: "1", wantPresent: true}, - {uri: "/x?a=1&a=2", key: "a", want: "1", wantPresent: true}, // First match wins. - {uri: "/x?q=go", key: "nope", want: "", wantPresent: false}, // Absent. - {uri: "/x", key: "q", want: "", wantPresent: false}, // No query at all. + {uri: "/x?a=1&a=2", key: "a", want: "1", wantPresent: true}, // First match wins. + {uri: "/x?q=go", key: "nope", want: "", wantPresent: false}, // Absent. + {uri: "/x", key: "q", want: "", wantPresent: false}, // No query at all. {uri: "/x?debug&q=go", key: "debug", want: "", wantPresent: true}, // Flag: present, no value. {uri: "/x?q=", key: "q", want: "", wantPresent: true}, // Present, empty. // Decoding is opt-in and applies to the value only. diff --git a/http/httphi/httphi.go b/http/httphi/httphi.go index 8a6b407..128e87a 100644 --- a/http/httphi/httphi.go +++ b/http/httphi/httphi.go @@ -596,10 +596,10 @@ func (exch *Exchange) RequestPath() []byte { return exch.RequestHeaderRaw().RequestPath() } -// ForEachQueryRaw iterates over the request's query string key-value pairs as -// they appear on the wire. See [httpraw.Header.ForEachQuery]. -func (exch *Exchange) ForEachQueryRaw(fn func(rawkey, rawval []byte) bool) { - exch.RequestHeaderRaw().ForEachQuery(fn) +// RequestQuery returns the request's query string as it appears on the wire. +// Iterate it with [httpraw.NextQueryPair]. See [httpraw.Header.RequestQuery]. +func (exch *Exchange) RequestQuery() []byte { + return exch.RequestHeaderRaw().RequestQuery() } // AppendQuery appends the value of the first query parameter matching key to @@ -617,39 +617,32 @@ func (exch *Exchange) ForEachQueryRaw(fn func(rawkey, rawval []byte) bool) { func (exch *Exchange) AppendQuery(dst []byte, key string, decoded bool) (valueAppended []byte, present bool) { const plusAsSpace = true // Query strings are form encoded, unlike paths. base := len(dst) - valueAppended = dst - exch.ForEachQueryRaw(func(rawkey, rawval []byte) bool { + rawkey, rawval, rest := httpraw.NextQueryPair(exch.RequestQuery()) + for ; rawkey != nil; rawkey, rawval, rest = httpraw.NextQueryPair(rest) { if b2s(rawkey) != key { // Key may be encoded: decode it over dst's free space and compare. // A decoded key cannot appear raw, so this cannot alias a real key. - valueAppended = slices.Grow(valueAppended, len(rawkey)) - scratch := valueAppended[base : base+len(rawkey)] + dst = slices.Grow(dst, len(rawkey)) + scratch := dst[base : base+len(rawkey)] n, err := httpraw.CopyDecodedPercentURL(scratch, rawkey, plusAsSpace) if err != nil || b2s(scratch[:n]) != key { - return true // Malformed or different key, keep looking. + continue // Malformed or different key, keep looking. } } - present = true if len(rawval) == 0 { - return false + return dst[:base], true // Flag or empty value, nothing to append. } - valueAppended = slices.Grow(valueAppended, len(rawval)) + dst = slices.Grow(dst, len(rawval)) if !decoded { - valueAppended = append(valueAppended[:base], rawval...) - return false + return append(dst[:base], rawval...), true } - n, err := httpraw.CopyDecodedPercentURL(valueAppended[base:base+len(rawval)], rawval, plusAsSpace) + n, err := httpraw.CopyDecodedPercentURL(dst[base:base+len(rawval)], rawval, plusAsSpace) if err != nil { - present = false // Malformed value, do not hand back half a decode. - return false + return dst[:base], false // Do not hand back half a decode. } - valueAppended = valueAppended[:base+n] - return false - }) - if !present { - valueAppended = valueAppended[:base] + return dst[:base+n], true } - return valueAppended, present + return dst[:base], false } func (exch *Exchange) RequestMethod() []byte { diff --git a/http/httphi/zz_scratch_bench_test.go b/http/httphi/zz_scratch_bench_test.go deleted file mode 100644 index 5284850..0000000 --- a/http/httphi/zz_scratch_bench_test.go +++ /dev/null @@ -1,145 +0,0 @@ -package httphi - -import "testing" - -// Scratch benchmarks isolating query handling. Delete after diagnosis. - -const scratchRequest = "GET /?abc=123 HTTP/1.1\r\nHost: tinygo.org\r\nUser-Agent: bench\r\nAccept: */*\r\nConnection: close\r\n\r\n" - -func scratchExchange(b *testing.B) (*Exchange, *benchConn, *MuxSlice) { - b.Helper() - conn := &benchConn{request: scratchRequest} - exch := benchExchange(b, conn) - mux := new(MuxSlice) - return exch, conn, mux -} - -// A: whole exchange, handler writes body only. No query touched. -func BenchmarkScratchBodyOnly(b *testing.B) { - exch, conn, mux := scratchExchange(b) - mux.Handle("GET /", func(ex *Exchange) { ex.Write(benchBody) }) - b.ReportAllocs() - for i := 0; i < b.N; i++ { - conn.rewind() - exch.Release() - exch.Acquire(conn) - Handle(exch, mux, nopBackoff) - } -} - -// B: same plus header writes. Isolates SetHeader/SetHeaderInt. -func BenchmarkScratchHeaders(b *testing.B) { - exch, conn, mux := scratchExchange(b) - mux.Handle("GET /", func(ex *Exchange) { - ex.SetHeader("Content-Type", "text/plain") - ex.SetHeaderInt("Content-Length", int64(len(benchBody)), 10) - ex.Write(benchBody) - }) - b.ReportAllocs() - for i := 0; i < b.N; i++ { - conn.rewind() - exch.Release() - exch.Acquire(conn) - Handle(exch, mux, nopBackoff) - } -} - -// C: iteration only, closure captures one counter. -func BenchmarkScratchForEachQuery(b *testing.B) { - exch, conn, mux := scratchExchange(b) - var seen int - mux.Handle("GET /", func(ex *Exchange) { - ex.ForEachQueryRaw(func(rawkey, rawval []byte) bool { - seen += len(rawkey) - return true - }) - }) - b.ReportAllocs() - for i := 0; i < b.N; i++ { - conn.rewind() - exch.Release() - exch.Acquire(conn) - Handle(exch, mux, nopBackoff) - } - _ = seen -} - -// C2: closure that reassigns a captured slice, exactly AppendQuery's shape. -func BenchmarkScratchClosureSlice(b *testing.B) { - exch, conn, mux := scratchExchange(b) - buf := make([]byte, 64) - mux.Handle("GET /", func(ex *Exchange) { - out := buf[:0] - found := false - ex.ForEachQueryRaw(func(rawkey, rawval []byte) bool { - if b2s(rawkey) != "abc" { - return true - } - out = append(out, rawval...) - found = true - return false - }) - _ = found - _ = out - }) - b.ReportAllocs() - for i := 0; i < b.N; i++ { - conn.rewind() - exch.Release() - exch.Acquire(conn) - Handle(exch, mux, nopBackoff) - } -} - -// C3: same but the captured slice is only read, never reassigned. -func BenchmarkScratchClosureNoReassign(b *testing.B) { - exch, conn, mux := scratchExchange(b) - buf := make([]byte, 64) - mux.Handle("GET /", func(ex *Exchange) { - n := 0 - ex.ForEachQueryRaw(func(rawkey, rawval []byte) bool { - n += copy(buf, rawval) - return false - }) - _ = n - }) - b.ReportAllocs() - for i := 0; i < b.N; i++ { - conn.rewind() - exch.Release() - exch.Acquire(conn) - Handle(exch, mux, nopBackoff) - } -} - -// D: AppendQuery raw, into a buffer with capacity. -func BenchmarkScratchAppendQueryRaw(b *testing.B) { - exch, conn, mux := scratchExchange(b) - buf := make([]byte, 64) - mux.Handle("GET /", func(ex *Exchange) { - ex.AppendQuery(buf[:0], "abc", false) - }) - b.ReportAllocs() - for i := 0; i < b.N; i++ { - conn.rewind() - exch.Release() - exch.Acquire(conn) - Handle(exch, mux, nopBackoff) - } -} - -// E: AppendQuery decoded. -func BenchmarkScratchAppendQueryDecoded(b *testing.B) { - exch, conn, mux := scratchExchange(b) - buf := make([]byte, 64) - mux.Handle("GET /", func(ex *Exchange) { - ex.AppendQuery(buf[:0], "abc", true) - }) - b.ReportAllocs() - for i := 0; i < b.N; i++ { - conn.rewind() - exch.Release() - exch.Acquire(conn) - Handle(exch, mux, nopBackoff) - } -} diff --git a/http/httpraw/header.go b/http/httpraw/header.go index 746e2ce..c32e984 100644 --- a/http/httpraw/header.go +++ b/http/httpraw/header.go @@ -372,21 +372,32 @@ func (h *Header) RequestPath() []byte { return uri[:query] } -// ForEachQuery iterates over the request URI's query string key-value pairs as -// they appear on the wire, percent-encoded and with '+' undecoded. fn returns -// true to continue iterating, false to stop. -// -// A pair with no '=' yields a nil value, i.e: "debug" in "?debug&q=go", which -// distinguishes it from "?debug=" where the value is present and empty. Empty -// sequences are skipped, so "?&&q=go&" yields a single pair. Only '&' separates -// pairs and only the first '=' splits a pair. -func (h *Header) ForEachQuery(fn func(rawkey, rawval []byte) bool) { +// RequestQuery returns the request URI's query string as it appears on the +// wire, percent-encoded and with '+' undecoded, i.e: "q=go" for "/search?q=go". +// Returns nil if the URI has no query string. Iterate it with [NextQueryPair]. +func (h *Header) RequestQuery() []byte { uri := h.RequestURI() start := bytes.IndexByte(uri, '?') if start < 0 { - return + return nil } - query := uri[start+1:] + return uri[start+1:] +} + +// NextQueryPair splits the leading key-value pair off a query string and returns +// what remains of it. Loop until rawkey is nil: +// +// rawkey, rawval, rest := httpraw.NextQueryPair(h.RequestQuery()) +// for rawkey != nil { +// // use rawkey, rawval. +// rawkey, rawval, rest = httpraw.NextQueryPair(rest) +// } +// +// A pair with no '=' yields a nil rawval, i.e: "debug" in "?debug&q=go", which +// distinguishes it from "?debug=" where the value is present and empty. Empty +// sequences are skipped, so "?&&q=go&" yields a single pair. Only '&' separates +// pairs and only the first '=' splits a pair. +func NextQueryPair(query []byte) (rawkey, rawval, rest []byte) { for len(query) > 0 { pair := query amp := bytes.IndexByte(query, '&') @@ -398,14 +409,12 @@ func (h *Header) ForEachQuery(fn func(rawkey, rawval []byte) bool) { if len(pair) == 0 { continue // Empty sequence, see WHATWG URL urlencoded parsing. } - key, value := pair, []byte(nil) if eq := bytes.IndexByte(pair, '='); eq >= 0 { - key, value = pair[:eq], pair[eq+1:] - } - if !fn(key, value) { - return + return pair[:eq], pair[eq+1:], query } + return pair, nil, query } + return nil, nil, nil } // Protocol returns the request header's HTTP protocol. Usually "HTTP/1.1". diff --git a/http/httpraw/header_test.go b/http/httpraw/header_test.go index 69319e6..eaebb89 100644 --- a/http/httpraw/header_test.go +++ b/http/httpraw/header_test.go @@ -173,7 +173,7 @@ func TestHeaderRequestPath(t *testing.T) { } } -func TestHeaderForEachQuery(t *testing.T) { +func TestNextQueryPair(t *testing.T) { for _, test := range []struct { uri string want string // "key=value" pairs joined by '|'; nil value shown as "key". @@ -196,7 +196,8 @@ func TestHeaderForEachQuery(t *testing.T) { t.Fatal(err) } var got []byte - h.ForEachQuery(func(rawkey, rawval []byte) bool { + rawkey, rawval, rest := NextQueryPair(h.RequestQuery()) + for rawkey != nil { if len(got) > 0 { got = append(got, '|') } @@ -205,28 +206,30 @@ func TestHeaderForEachQuery(t *testing.T) { got = append(got, '=') got = append(got, rawval...) } - return true - }) + rawkey, rawval, rest = NextQueryPair(rest) + } if string(got) != test.want { t.Errorf("uri %q: want %q, got %q", test.uri, test.want, got) } } } -// fn returning false stops iteration. -func TestHeaderForEachQueryStop(t *testing.T) { - var h Header - err := h.ParseBytes(false, []byte("GET /x?a=1&b=2&c=3 HTTP/1.1\r\nHost: h\r\n\r\n")) - if err != nil { - t.Fatal(err) +// A nil key ends iteration, and stopping early is just not looping again. +func TestNextQueryPairEnd(t *testing.T) { + rawkey, rawval, rest := NextQueryPair([]byte("a=1&b=2")) + if string(rawkey) != "a" || string(rawval) != "1" || string(rest) != "b=2" { + t.Fatalf("want a=1 with rest b=2, got %q=%q rest %q", rawkey, rawval, rest) } - visited := 0 - h.ForEachQuery(func(rawkey, rawval []byte) bool { - visited++ - return string(rawkey) != "b" - }) - if visited != 2 { - t.Errorf("want iteration stopped after 2 pairs, visited %d", visited) + rawkey, _, rest = NextQueryPair(rest) + if string(rawkey) != "b" || len(rest) != 0 { + t.Fatalf("want b with empty rest, got %q rest %q", rawkey, rest) + } + if rawkey, _, _ = NextQueryPair(rest); rawkey != nil { + t.Fatalf("want nil key at end of query, got %q", rawkey) + } + // Trailing separators yield no pair rather than an empty one. + if rawkey, _, _ = NextQueryPair([]byte("&&")); rawkey != nil { + t.Fatalf("want nil key for empty sequences, got %q", rawkey) } }