From 59320c396e6448132a52cb5a5d96491eee1e0ad8 Mon Sep 17 00:00:00 2001 From: Quentin Renard Date: Sun, 6 Mar 2016 17:27:50 +0100 Subject: [PATCH] net/http: improve performance for parsePostForm MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Remove the use of io.ReadAll in http.parsePostForm to avoid converting the whole input from []byte to string and not performing well space-allocated-wise. Instead a new function called parsePostFormURLEncoded is used and is fed directly an io.Reader that is parsed using a bufio.Reader. Benchmark: name old time/op new time/op delta PostQuery-4 2.90µs ± 6% 2.82µs ± 4% ~ (p=0.094 n=9+9) name old alloc/op new alloc/op delta PostQuery-4 1.05kB ± 0% 0.90kB ± 0% -14.49% (p=0.000 n=10+10) name old allocs/op new allocs/op delta PostQuery-4 6.00 ± 0% 7.00 ± 0% +16.67% (p=0.000 n=10+10) Fixes #14655 Change-Id: I112c263d4221d959ed6153cfe88bc57a2aa8ea73 Reviewed-on: https://go-review.googlesource.com/20301 Reviewed-by: Brad Fitzpatrick Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/net/http/request.go | 60 ++++++++++++++++++++++++++++-------- src/net/http/request_test.go | 26 +++++++++++++--- 2 files changed, 70 insertions(+), 16 deletions(-) diff --git a/src/net/http/request.go b/src/net/http/request.go index c29af7fbe5..82a918c22e 100644 --- a/src/net/http/request.go +++ b/src/net/http/request.go @@ -1015,18 +1015,8 @@ func parsePostForm(r *Request) (vs url.Values, err error) { maxFormSize = int64(10 << 20) // 10 MB is a lot of text. reader = io.LimitReader(r.Body, maxFormSize+1) } - b, e := ioutil.ReadAll(reader) - if e != nil { - if err == nil { - err = e - } - break - } - if int64(len(b)) > maxFormSize { - err = errors.New("http: POST too large") - return - } - vs, e = url.ParseQuery(string(b)) + vs = make(url.Values) + e := parsePostFormURLEncoded(vs, reader, maxFormSize) if err == nil { err = e } @@ -1041,6 +1031,52 @@ func parsePostForm(r *Request) (vs url.Values, err error) { return } +// parsePostFormURLEncoded reads from r, the reader of a POST form to populate vs which is a url-type values. +// maxFormSize indicates the maximum number of bytes that will be read from r. +func parsePostFormURLEncoded(vs url.Values, r io.Reader, maxFormSize int64) error { + br := newBufioReader(r) + defer putBufioReader(br) + + var readSize int64 + for { + // Read next "key=value&" or "justkey&". + // If this is the last pair, b will contain just "key=value" or "justkey". + b, err := br.ReadBytes('&') + if err != nil && err != io.EOF && err != bufio.ErrBufferFull { + return err + } + isEOF := err == io.EOF + readSize += int64(len(b)) + if readSize >= maxFormSize { + return errors.New("http: POST too large") + } + + // Remove last delimiter + if len(b) > 0 && b[len(b)-1] == '&' { + b = b[:len(b)-1] + } + + // Parse key and value + k := string(b) + var v string + if i := strings.Index(k, "="); i > -1 { + k, v = k[:i], k[i+1:] + } + if k, err = url.QueryUnescape(k); err != nil { + return err + } + if v, err = url.QueryUnescape(v); err != nil { + return err + } + + // Populate vs + vs[k] = append(vs[k], v) + if isEOF { + return nil + } + } +} + // ParseForm parses the raw query from the URL and updates r.Form. // // For POST or PUT requests, it also parses the request body as a form and diff --git a/src/net/http/request_test.go b/src/net/http/request_test.go index a4c88c0291..a6c90d09a4 100644 --- a/src/net/http/request_test.go +++ b/src/net/http/request_test.go @@ -30,8 +30,8 @@ func TestQuery(t *testing.T) { } func TestPostQuery(t *testing.T) { - req, _ := NewRequest("POST", "http://www.google.com/search?q=foo&q=bar&both=x&prio=1&empty=not", - strings.NewReader("z=post&both=y&prio=2&empty=")) + req, _ := NewRequest("POST", "http://www.google.com/search?q=foo&q=bar&both=x&prio=1&empty=not&orphan=nope", + strings.NewReader("z=post&both=y&prio=2&orphan&empty=")) req.Header.Set("Content-Type", "application/x-www-form-urlencoded; param=value") if q := req.FormValue("q"); q != "foo" { @@ -58,11 +58,26 @@ func TestPostQuery(t *testing.T) { if empty := req.FormValue("empty"); empty != "" { t.Errorf(`req.FormValue("empty") = %q, want "" (from body)`, empty) } + if orphan := req.FormValue("orphan"); orphan != "" { + t.Errorf(`req.FormValue("orphan") = %q, want "" (from body)`, orphan) + } +} + +func BenchmarkPostQuery(b *testing.B) { + req, _ := NewRequest("POST", "http://www.google.com/search?q=foo&q=bar&both=x&prio=1&empty=not&orphan=nope", + strings.NewReader("z=post&both=y&prio=2&orphan&empty=")) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded; param=value") + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + req.PostForm = nil + req.ParseForm() + } } func TestPatchQuery(t *testing.T) { - req, _ := NewRequest("PATCH", "http://www.google.com/search?q=foo&q=bar&both=x&prio=1&empty=not", - strings.NewReader("z=post&both=y&prio=2&empty=")) + req, _ := NewRequest("PATCH", "http://www.google.com/search?q=foo&q=bar&both=x&prio=1&empty=not&orphan=nope", + strings.NewReader("z=post&both=y&prio=2&orphan&empty=")) req.Header.Set("Content-Type", "application/x-www-form-urlencoded; param=value") if q := req.FormValue("q"); q != "foo" { @@ -89,6 +104,9 @@ func TestPatchQuery(t *testing.T) { if empty := req.FormValue("empty"); empty != "" { t.Errorf(`req.FormValue("empty") = %q, want "" (from body)`, empty) } + if orphan := req.FormValue("orphan"); orphan != "" { + t.Errorf(`req.FormValue("orphan") = %q, want "" (from body)`, orphan) + } } type stringMap map[string][]string -- 2.48.1