Skip to content

Commit 59320c3

Browse files
asticodebradfitz
authored andcommitted
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 <[email protected]> Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
1 parent e6051de commit 59320c3

File tree

2 files changed

+70
-16
lines changed

2 files changed

+70
-16
lines changed

src/net/http/request.go

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,18 +1015,8 @@ func parsePostForm(r *Request) (vs url.Values, err error) {
10151015
maxFormSize = int64(10 << 20) // 10 MB is a lot of text.
10161016
reader = io.LimitReader(r.Body, maxFormSize+1)
10171017
}
1018-
b, e := ioutil.ReadAll(reader)
1019-
if e != nil {
1020-
if err == nil {
1021-
err = e
1022-
}
1023-
break
1024-
}
1025-
if int64(len(b)) > maxFormSize {
1026-
err = errors.New("http: POST too large")
1027-
return
1028-
}
1029-
vs, e = url.ParseQuery(string(b))
1018+
vs = make(url.Values)
1019+
e := parsePostFormURLEncoded(vs, reader, maxFormSize)
10301020
if err == nil {
10311021
err = e
10321022
}
@@ -1041,6 +1031,52 @@ func parsePostForm(r *Request) (vs url.Values, err error) {
10411031
return
10421032
}
10431033

1034+
// parsePostFormURLEncoded reads from r, the reader of a POST form to populate vs which is a url-type values.
1035+
// maxFormSize indicates the maximum number of bytes that will be read from r.
1036+
func parsePostFormURLEncoded(vs url.Values, r io.Reader, maxFormSize int64) error {
1037+
br := newBufioReader(r)
1038+
defer putBufioReader(br)
1039+
1040+
var readSize int64
1041+
for {
1042+
// Read next "key=value&" or "justkey&".
1043+
// If this is the last pair, b will contain just "key=value" or "justkey".
1044+
b, err := br.ReadBytes('&')
1045+
if err != nil && err != io.EOF && err != bufio.ErrBufferFull {
1046+
return err
1047+
}
1048+
isEOF := err == io.EOF
1049+
readSize += int64(len(b))
1050+
if readSize >= maxFormSize {
1051+
return errors.New("http: POST too large")
1052+
}
1053+
1054+
// Remove last delimiter
1055+
if len(b) > 0 && b[len(b)-1] == '&' {
1056+
b = b[:len(b)-1]
1057+
}
1058+
1059+
// Parse key and value
1060+
k := string(b)
1061+
var v string
1062+
if i := strings.Index(k, "="); i > -1 {
1063+
k, v = k[:i], k[i+1:]
1064+
}
1065+
if k, err = url.QueryUnescape(k); err != nil {
1066+
return err
1067+
}
1068+
if v, err = url.QueryUnescape(v); err != nil {
1069+
return err
1070+
}
1071+
1072+
// Populate vs
1073+
vs[k] = append(vs[k], v)
1074+
if isEOF {
1075+
return nil
1076+
}
1077+
}
1078+
}
1079+
10441080
// ParseForm parses the raw query from the URL and updates r.Form.
10451081
//
10461082
// For POST or PUT requests, it also parses the request body as a form and

src/net/http/request_test.go

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ func TestQuery(t *testing.T) {
3030
}
3131

3232
func TestPostQuery(t *testing.T) {
33-
req, _ := NewRequest("POST", "http://www.google.com/search?q=foo&q=bar&both=x&prio=1&empty=not",
34-
strings.NewReader("z=post&both=y&prio=2&empty="))
33+
req, _ := NewRequest("POST", "http://www.google.com/search?q=foo&q=bar&both=x&prio=1&empty=not&orphan=nope",
34+
strings.NewReader("z=post&both=y&prio=2&orphan&empty="))
3535
req.Header.Set("Content-Type", "application/x-www-form-urlencoded; param=value")
3636

3737
if q := req.FormValue("q"); q != "foo" {
@@ -58,11 +58,26 @@ func TestPostQuery(t *testing.T) {
5858
if empty := req.FormValue("empty"); empty != "" {
5959
t.Errorf(`req.FormValue("empty") = %q, want "" (from body)`, empty)
6060
}
61+
if orphan := req.FormValue("orphan"); orphan != "" {
62+
t.Errorf(`req.FormValue("orphan") = %q, want "" (from body)`, orphan)
63+
}
64+
}
65+
66+
func BenchmarkPostQuery(b *testing.B) {
67+
req, _ := NewRequest("POST", "http://www.google.com/search?q=foo&q=bar&both=x&prio=1&empty=not&orphan=nope",
68+
strings.NewReader("z=post&both=y&prio=2&orphan&empty="))
69+
req.Header.Set("Content-Type", "application/x-www-form-urlencoded; param=value")
70+
b.ReportAllocs()
71+
b.ResetTimer()
72+
for i := 0; i < b.N; i++ {
73+
req.PostForm = nil
74+
req.ParseForm()
75+
}
6176
}
6277

6378
func TestPatchQuery(t *testing.T) {
64-
req, _ := NewRequest("PATCH", "http://www.google.com/search?q=foo&q=bar&both=x&prio=1&empty=not",
65-
strings.NewReader("z=post&both=y&prio=2&empty="))
79+
req, _ := NewRequest("PATCH", "http://www.google.com/search?q=foo&q=bar&both=x&prio=1&empty=not&orphan=nope",
80+
strings.NewReader("z=post&both=y&prio=2&orphan&empty="))
6681
req.Header.Set("Content-Type", "application/x-www-form-urlencoded; param=value")
6782

6883
if q := req.FormValue("q"); q != "foo" {
@@ -89,6 +104,9 @@ func TestPatchQuery(t *testing.T) {
89104
if empty := req.FormValue("empty"); empty != "" {
90105
t.Errorf(`req.FormValue("empty") = %q, want "" (from body)`, empty)
91106
}
107+
if orphan := req.FormValue("orphan"); orphan != "" {
108+
t.Errorf(`req.FormValue("orphan") = %q, want "" (from body)`, orphan)
109+
}
92110
}
93111

94112
type stringMap map[string][]string

0 commit comments

Comments
 (0)