mime: ParseMediaType returns os.Error now, not a nil map

ParseMediaType previously documented that it always returned
a non-nil map, but also documented that it returned a nil map
to signal an error.

That is confusing, contradictory and not Go-like.

Now it returns (mediatype string, params map, os.Error).

R=golang-dev, r
CC=golang-dev
https://golang.org/cl/4867054

src/pkg/http/request.go

@@ -234,8 +234,8 @@ func (r *Request) multipartReader() (*multipart.Reader, os.Error) {
 	if v == "" {
 		return nil, ErrNotMultipart
 	}
-	d, params := mime.ParseMediaType(v)
-	if d != "multipart/form-data" {
+	d, params, err := mime.ParseMediaType(v)
+	if err != nil || d != "multipart/form-data" {
 		return nil, ErrNotMultipart
 	}
 	boundary, ok := params["boundary"]
@@ -625,8 +625,9 @@ func (r *Request) ParseForm() (err os.Error) {
 			return os.NewError("missing form body")
 		}
 		ct := r.Header.Get("Content-Type")
-		switch strings.SplitN(ct, ";", 2)[0] {
-		case "text/plain", "application/x-www-form-urlencoded", "":
+		ct, _, err := mime.ParseMediaType(ct)
+		switch {
+		case ct == "text/plain" || ct == "application/x-www-form-urlencoded" || ct == "":
 			const maxFormSize = int64(10 << 20) // 10 MB is a lot of text.
 			b, e := ioutil.ReadAll(io.LimitReader(r.Body, maxFormSize+1))
 			if e != nil {
@@ -652,8 +653,13 @@ func (r *Request) ParseForm() (err os.Error) {
 					r.Form.Add(k, value)
 				}
 			}
-		case "multipart/form-data":
-			// handled by ParseMultipartForm
+		case ct == "multipart/form-data":
+			// handled by ParseMultipartForm (which is calling us, or should be)
+			// TODO(bradfitz): there are too many possible
+			// orders to call too many functions here.
+			// Clean this up and write more tests.
+			// request_test.go contains the start of this,
+			// in TestRequestMultipartCallOrder.
 		default:
 			return &badStringError{"unknown Content-Type", ct}
 		}

src/pkg/mime/mediatype.go

@@ -12,40 +12,44 @@ import (
 	"unicode"
 )
 
-func validMediaTypeOrDisposition(s string) bool {
+func checkMediaTypeDisposition(s string) os.Error {
 	typ, rest := consumeToken(s)
 	if typ == "" {
-		return false
+		return os.NewError("mime: no media type")
 	}
 	if rest == "" {
-		return true
+		return nil
 	}
 	if !strings.HasPrefix(rest, "/") {
-		return false
+		return os.NewError("mime: expected slash after first token")
 	}
 	subtype, rest := consumeToken(rest[1:])
 	if subtype == "" {
-		return false
+		return os.NewError("mime: expected token after slash")
+	}
+	if rest != "" {
+		return os.NewError("mime: unexpected content after media subtype")
 	}
-	return rest == ""
+	return nil
 }
 
 // ParseMediaType parses a media type value and any optional
 // parameters, per RFC 1521.  Media types are the values in
 // Content-Type and Content-Disposition headers (RFC 2183).
 // On success, ParseMediaType returns the media type converted
-// to lowercase and trimmed of white space. The returned params
-// is always a non-nil map. Params maps from the lowercase
+// to lowercase and trimmed of white space and a non-nil map.
+// The returned map, params, maps from the lowercase
 // attribute to the attribute value with its case preserved.
-// On error, it returns an empty string and a nil params.
-func ParseMediaType(v string) (mediatype string, params map[string]string) {
+func ParseMediaType(v string) (mediatype string, params map[string]string, err os.Error) {
 	i := strings.Index(v, ";")
 	if i == -1 {
 		i = len(v)
 	}
 	mediatype = strings.TrimSpace(strings.ToLower(v[0:i]))
-	if !validMediaTypeOrDisposition(mediatype) {
-		return "", nil
+
+	err = checkMediaTypeDisposition(mediatype)
+	if err != nil {
+		return
 	}
 
 	params = make(map[string]string)
@@ -69,7 +73,7 @@ func ParseMediaType(v string) (mediatype string, params map[string]string) {
 				return
 			}
 			// Parse error.
-			return "", nil
+			return "", nil, os.NewError("mime: invalid media parameter")
 		}
 
 		pmap := params
@@ -86,7 +90,7 @@ func ParseMediaType(v string) (mediatype string, params map[string]string) {
 		}
 		if _, exists := pmap[key]; exists {
 			// Duplicate parameter name is bogus.
-			return "", nil
+			return "", nil, os.NewError("mime: duplicate parameter name")
 		}
 		pmap[key] = value
 		v = rest

src/pkg/mime/mediatype_test.go

@@ -219,7 +219,14 @@ func TestParseMediaType(t *testing.T) {
 			m("firstname", "Брэд", "lastname", "Фицпатрик")},
 	}
 	for _, test := range tests {
-		mt, params := ParseMediaType(test.in)
+		mt, params, err := ParseMediaType(test.in)
+		if err != nil {
+			if test.t != "" {
+				t.Errorf("for input %q, unexpected error: %v", test.in, err)
+				continue
+			}
+			continue
+		}
 		if g, e := mt, test.t; g != e {
 			t.Errorf("for input %q, expected type %q, got %q",
 				test.in, e, g)
@@ -238,11 +245,11 @@ func TestParseMediaType(t *testing.T) {
 }
 
 func TestParseMediaTypeBogus(t *testing.T) {
-	mt, params := ParseMediaType("bogus ;=========")
-	if mt != "" {
-		t.Error("expected empty type")
+	mt, params, err := ParseMediaType("bogus ;=========")
+	if err == nil {
+		t.Fatalf("expected an error parsing invalid media type; got type %q, params %#v", mt, params)
 	}
-	if params != nil {
-		t.Error("expected nil params")
+	if err.String() != "mime: invalid media parameter" {
+		t.Errorf("expected invalid media parameter; got error %q", err)
 	}
 }

src/pkg/mime/multipart/multipart.go

@@ -69,8 +69,9 @@ func (p *Part) FileName() string {
 
 func (p *Part) parseContentDisposition() {
 	v := p.Header.Get("Content-Disposition")
-	p.disposition, p.dispositionParams = mime.ParseMediaType(v)
-	if p.dispositionParams == nil {
+	var err os.Error
+	p.disposition, p.dispositionParams, err = mime.ParseMediaType(v)
+	if err != nil {
 		p.dispositionParams = emptyParams
 	}
 }