net/http: Return ErrNotMultipart from ParseMultipartForm if content-type isn't multip...

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

diff --git a/src/pkg/net/http/request.go b/src/pkg/net/http/request.go
index 120ff857497bc1b1debaf842650a81d74a1211e9..feafc815772cab88aa727e11d7d70df22c1bc23c 100644 (file)
--- 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 223e02c294521d7985bff71b4e4be1bb55e51093..b9fa3c2bfc4f68fae9fb4df302325971dd356355 100644 (file)
--- 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) {