From 8d1b63abff2c948741e94c92d2a9a60069a71ddb Mon Sep 17 00:00:00 2001 From: Matthew Cottingham Date: Thu, 10 Apr 2014 22:50:04 -0700 Subject: [PATCH] net/http: Return ErrNotMultipart from ParseMultipartForm if content-type isn't multipart/form-data. Add test for multipart form requests with an invalid content-type to ensure ErrNotMultipart is returned. Change ParseMultipartForm to return ErrNotMultipart when it is returned by multipartReader. Modify test for empty multipart request handling to use POST so that the body is checked. Fixes #6334. This is the first changeset working on multipart request handling. Further changesets could add more tests and clean up the TODO. LGTM=bradfitz R=golang-codereviews, gobot, bradfitz, rsc CC=golang-codereviews https://golang.org/cl/44040043 --- src/pkg/net/http/request.go | 4 +-- src/pkg/net/http/request_test.go | 56 +++++++++++++++++++++++++++----- 2 files changed, 49 insertions(+), 11 deletions(-) diff --git a/src/pkg/net/http/request.go b/src/pkg/net/http/request.go index 120ff857497bc..feafc815772ca 100644 --- a/src/pkg/net/http/request.go +++ b/src/pkg/net/http/request.go @@ -789,9 +789,7 @@ func (r *Request) ParseMultipartForm(maxMemory int64) error { } mr, err := r.multipartReader() - if err == ErrNotMultipart { - return nil - } else if err != nil { + if err != nil { return err } diff --git a/src/pkg/net/http/request_test.go b/src/pkg/net/http/request_test.go index 223e02c294521..b9fa3c2bfc4f6 100644 --- a/src/pkg/net/http/request_test.go +++ b/src/pkg/net/http/request_test.go @@ -154,7 +154,25 @@ func TestMultipartReader(t *testing.T) { req.Header = Header{"Content-Type": {"text/plain"}} multipart, err = req.MultipartReader() if multipart != nil { - t.Errorf("unexpected multipart for text/plain") + t.Error("unexpected multipart for text/plain") + } +} + +func TestParseMultipartForm(t *testing.T) { + req := &Request{ + Method: "POST", + Header: Header{"Content-Type": {`multipart/form-data; boundary="foo123"`}}, + Body: ioutil.NopCloser(new(bytes.Buffer)), + } + err := req.ParseMultipartForm(25) + if err == nil { + t.Error("expected multipart EOF, got nil") + } + + req.Header = Header{"Content-Type": {"text/plain"}} + err = req.ParseMultipartForm(25) + if err != ErrNotMultipart { + t.Error("expected ErrNotMultipart for text/plain") } } @@ -220,16 +238,38 @@ func TestMultipartRequestAuto(t *testing.T) { validateTestMultipartContents(t, req, true) } -func TestEmptyMultipartRequest(t *testing.T) { - // Test that FormValue and FormFile automatically invoke - // ParseMultipartForm and return the right values. - req, err := NewRequest("GET", "/", nil) - if err != nil { - t.Errorf("NewRequest err = %q", err) - } +func TestMissingFileMultipartRequest(t *testing.T) { + // Test that FormFile returns an error if + // the named file is missing. + req := newTestMultipartRequest(t) testMissingFile(t, req) } +// Test that FormValue invokes ParseMultipartForm. +func TestFormValueCallsParseMultipartForm(t *testing.T) { + req, _ := NewRequest("POST", "http://www.google.com/", strings.NewReader("z=post")) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded; param=value") + if req.Form != nil { + t.Fatal("Unexpected request Form, want nil") + } + req.FormValue("z") + if req.Form == nil { + t.Fatal("ParseMultipartForm not called by FormValue") + } +} + +// Test that FormFile invokes ParseMultipartForm. +func TestFormFileCallsParseMultipartForm(t *testing.T) { + req := newTestMultipartRequest(t) + if req.Form != nil { + t.Fatal("Unexpected request Form, want nil") + } + req.FormFile("") + if req.Form == nil { + t.Fatal("ParseMultipartForm not called by FormFile") + } +} + // Test that ParseMultipartForm errors if called // after MultipartReader on the same request. func TestParseMultipartFormOrder(t *testing.T) {