net/http: improve performance for parsePostForm

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 <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/net/http/request.go b/src/net/http/request.go
index c29af7fbe52fa3adbf53327d89d6f8122c7caf10..82a918c22e5fef0bd30d7d2bfb69c975f7341dc2 100644 (file)
--- 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 a4c88c02915ccdf3ada229bda2b6524b1f1623c0..a6c90d09a423e9c786f61457dc7bfdf124ae1c49 100644 (file)
--- 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