qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-05 16:06:10 -07:00
Code Releases Activity

Revert "net/http: improve performance for parsePostForm"

Browse Source 
This reverts commit 59320c396e.

Reasons:
This CL was causing failures on a large regression test that we run
within Google. The issues arises from two bugs in the CL:
* The CL dropped support for ';' as a delimiter (see https://golang.org/issue/2210)
* The handling of an empty string caused an empty record to be added when
no record was added (see https://golang.org/cl/30454 for my attempted fix)

The logic being added is essentially a variation of url.ParseQuery,
but altered to accept an io.Reader instead of a string.
Since it is duplicated (but modified) logic, there needs to be good
tests to ensure that it's implementation doesn't drift in functionality
from url.ParseQuery. Fixing the above issues and adding the associated
regression tests leads to >100 lines of codes.
For a 4% reduction in CPU time, I think this complexity and duplicated
logic is not worth the effort.

As such, I am abandoning my efforts to fix the existing issues and
believe that reverting CL/20301 is the better course of action.

Updates #14655

Change-Id: Ibb5be0a5b48a16c46337e213b79467fcafee69df
Reviewed-on: https://go-review.googlesource.com/30470
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This commit is contained in:
Joe Tsai 2016-10-05 21:23:56 +00:00 committed by Joe Tsai
parent a9b4953777
commit f6b4c88941
2 changed files with 16 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
src/net/http/request.go
View File

@ -1015,8 +1015,18 @@ 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)
}
vs = make(url.Values)
e := parsePostFormURLEncoded(vs, reader, maxFormSize)
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))
if err == nil {
err = e
}
@ -1031,52 +1041,6 @@ 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

26
src/net/http/request_test.go
View File

@ -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&orphan=nope",
strings.NewReader("z=post&both=y&prio=2&orphan&empty="))
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.Header.Set("Content-Type", "application/x-www-form-urlencoded; param=value")
if q := req.FormValue("q"); q != "foo" {
@ -58,26 +58,11 @@ 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&orphan=nope",
strings.NewReader("z=post&both=y&prio=2&orphan&empty="))
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.Header.Set("Content-Type", "application/x-www-form-urlencoded; param=value")
if q := req.FormValue("q"); q != "foo" {
@ -104,9 +89,6 @@ 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