Repository: jhillyerd/enmime
Pull request #91 (2018-11-19 17:35:05)

Handled malformed Content-Type params
Handled an issue when a content-type header like this is encountered:

`Content-Type: text/html;iso-8859-1`

coveralls: 
[![Coverage Status](https://coveralls.io/builds/20184535/badge)](https://coveralls.io/builds/20184535)

Coverage increased (+0.1%) to 85.853% when pulling **7d84100ff66e79bec814676d40df66d3e092114b on dcormier:dc/params** into **383e124cdb6c260d015d00c6af6ae25ec269526a on jhillyerd:develop**.


Patch:
diff --git a/detect.go b/detect.go
index 955ba77..01e2ef3 100644
--- a/detect.go
+++ b/detect.go
@@ -9,7 +9,7 @@ import (
 func detectMultipartMessage(root *Part) bool {
 	// Parse top-level multipart
 	ctype := root.Header.Get(hnContentType)
-	mediatype, _, err := parseMediaType(ctype)
+	mediatype, _, _, err := parseMediaType(ctype)
 	if err != nil {
 		return false
 	}
@@ -29,13 +29,13 @@ func detectMultipartMessage(root *Part) bool {
 //  - Content-Disposition: inline; filename="frog.jpg"
 //  - Content-Type: attachment; filename="frog.jpg"
 func detectAttachmentHeader(header textproto.MIMEHeader) bool {
-	mediatype, params, _ := parseMediaType(header.Get(hnContentDisposition))
+	mediatype, params, _, _ := parseMediaType(header.Get(hnContentDisposition))
 	if strings.ToLower(mediatype) == cdAttachment ||
 		(strings.ToLower(mediatype) == cdInline && len(params) > 0) {
 		return true
 	}
 
-	mediatype, _, _ = parseMediaType(header.Get(hnContentType))
+	mediatype, _, _, _ = parseMediaType(header.Get(hnContentType))
 	return strings.ToLower(mediatype) == cdAttachment
 }
 
@@ -48,7 +48,7 @@ func detectTextHeader(header textproto.MIMEHeader, emptyContentTypeIsText bool)
 		return true
 	}
 
-	mediatype, _, err := parseMediaType(ctype)
+	mediatype, _, _, err := parseMediaType(ctype)
 	if err != nil {
 		return false
 	}
@@ -72,7 +72,7 @@ func detectBinaryBody(root *Part) bool {
 		// 'text/plain' or 'text/html'.
 		// Example:
 		// Content-Type: application/pdf; name="doc.pdf"
-		mediatype, _, _ := parseMediaType(root.Header.Get(hnContentType))
+		mediatype, _, _, _ := parseMediaType(root.Header.Get(hnContentType))
 		mediatype = strings.ToLower(mediatype)
 		if mediatype != ctTextPlain && mediatype != ctTextHTML {
 			return true
diff --git a/envelope.go b/envelope.go
index 16742ad..dede315 100644
--- a/envelope.go
+++ b/envelope.go
@@ -226,7 +226,7 @@ func parseTextOnlyBody(root *Part, e *Envelope) error {
 	var charset string
 	var isHTML bool
 	if ctype := root.Header.Get(hnContentType); ctype != "" {
-		if mediatype, mparams, err := parseMediaType(ctype); err == nil {
+		if mediatype, mparams, _, err := parseMediaType(ctype); err == nil {
 			isHTML = (mediatype == ctTextHTML)
 			if mparams[hpCharset] != "" {
 				charset = mparams[hpCharset]
@@ -265,7 +265,7 @@ func parseTextOnlyBody(root *Part, e *Envelope) error {
 func parseMultiPartBody(root *Part, e *Envelope) error {
 	// Parse top-level multipart
 	ctype := root.Header.Get(hnContentType)
-	mediatype, params, err := parseMediaType(ctype)
+	mediatype, params, _, err := parseMediaType(ctype)
 	if err != nil {
 		return fmt.Errorf("Unable to parse media type: %v", err)
 	}
diff --git a/header.go b/header.go
index 775ea73..0e94866 100644
--- a/header.go
+++ b/header.go
@@ -29,6 +29,10 @@ const (
 
 	// Used as a placeholder in case of malformed Content-Type headers
 	ctPlaceholder = "x-not-a-mime-type/x-not-a-mime-type"
+	// Used as a placeholder param value in case of malformed
+	// Content-Type/Content-Disposition parameters that lack values.
+	// E.g.: Content-Type: text/html;iso-8859-1
+	pvPlaceholder = "not-a-param-value"
 
 	// Standard Transfer encodings
 	cte7Bit            = "7bit"
@@ -191,7 +195,7 @@ func decodeToUTF8Base64Header(input string) string {
 }
 
 // parseMediaType is a more tolerant implementation of Go's mime.ParseMediaType function.
-func parseMediaType(ctype string) (mtype string, params map[string]string, err error) {
+func parseMediaType(ctype string) (mtype string, params map[string]string, invalidParams []string, err error) {
 	mtype, params, err = mime.ParseMediaType(ctype)
 	if err != nil {
 		// Small hack to remove harmless charset duplicate params.
@@ -208,7 +212,7 @@ func parseMediaType(ctype string) (mtype string, params map[string]string, err e
 				// If the media parameter has special characters, ensure that it is quoted.
 				mtype, params, err = mime.ParseMediaType(fixUnquotedSpecials(mctype))
 				if err != nil {
-					return "", nil, err
+					return "", nil, nil, err
 				}
 			}
 		}
@@ -216,7 +220,14 @@ func parseMediaType(ctype string) (mtype string, params map[string]string, err e
 	if mtype == ctPlaceholder {
 		mtype = ""
 	}
-	return mtype, params, err
+	for name, value := range params {
+		if value != pvPlaceholder {
+			continue
+		}
+		invalidParams = append(invalidParams, name)
+		delete(params, name)
+	}
+	return mtype, params, invalidParams, err
 }
 
 // fixMangledMediaType is used to insert ; separators into media type strings that lack them, and
@@ -235,12 +246,13 @@ func fixMangledMediaType(mtype, sep string) string {
 				p = ctPlaceholder
 			}
 		default:
-			if strings.Contains(p, "=") {
-				pair := strings.Split(p, "=")
-				if strings.Contains(mtype, pair[0]+"=") {
-					// Ignore repeated parameters.
-					continue
-				}
+			if !strings.Contains(p, "=") {
+				p = p + "=" + pvPlaceholder
+			}
+			pair := strings.Split(p, "=")
+			if strings.Contains(mtype, pair[0]+"=") {
+				// Ignore repeated parameters.
+				continue
 			}
 		}
 		mtype += p
diff --git a/part.go b/part.go
index 3959f0f..f4c5127 100644
--- a/part.go
+++ b/part.go
@@ -110,7 +110,7 @@ func (p *Part) setupHeaders(r *bufio.Reader, defaultContentType string) error {
 		ctype = defaultContentType
 	}
 	// Parse Content-Type header.
-	mtype, mparams, err := parseMediaType(ctype)
+	mtype, mparams, minvalidParams, err := parseMediaType(ctype)
 	if err != nil {
 		return err
 	}
@@ -119,6 +119,12 @@ func (p *Part) setupHeaders(r *bufio.Reader, defaultContentType string) error {
 			ErrorMissingContentType,
 			"Content-Type header has parameters but no content type")
 	}
+	for i := range minvalidParams {
+		p.addWarning(
+			ErrorMalformedHeader,
+			"Content-Type header has malformed parameter %q",
+			minvalidParams[i])
+	}
 	p.ContentType = mtype
 	// Set disposition, filename, charset if available.
 	p.setupContentHeaders(mparams)
@@ -131,7 +137,7 @@ func (p *Part) setupHeaders(r *bufio.Reader, defaultContentType string) error {
 // the disposition, filename, and charset fields.
 func (p *Part) setupContentHeaders(mediaParams map[string]string) {
 	// Determine content disposition, filename, character set.
-	disposition, dparams, err := parseMediaType(p.Header.Get(hnContentDisposition))
+	disposition, dparams, _, err := parseMediaType(p.Header.Get(hnContentDisposition))
 	if err == nil {
 		// Disposition is optional
 		p.Disposition = disposition


Test patch:
diff --git a/detect_test.go b/detect_test.go
index 4072937..11f86f1 100644
--- a/detect_test.go
+++ b/detect_test.go
@@ -97,6 +97,16 @@ func TestDetectAttachmentHeader(t *testing.T) {
 			header: textproto.MIMEHeader{
 				"Content-Disposition": []string{"inline"}},
 		},
+		{
+			want: false,
+			header: textproto.MIMEHeader{
+				"Content-Disposition": []string{"inline; broken"}},
+		},
+		{
+			want: true,
+			header: textproto.MIMEHeader{
+				"Content-Disposition": []string{"attachment; broken"}},
+		},
 		{
 			want: true,
 			header: textproto.MIMEHeader{
diff --git a/header_test.go b/header_test.go
index c170e15..bac1a27 100644
--- a/header_test.go
+++ b/header_test.go
@@ -181,6 +181,21 @@ func TestFixMangledMediaType(t *testing.T) {
 			sep:   " ",
 			want:  "one/two;name=\"file.two\"",
 		},
+		{
+			input: "; name=\"file.two\"",
+			sep:   ";",
+			want:  ctPlaceholder + "; name=\"file.two\"",
+		},
+		{
+			input: "one/two;iso-8859-1",
+			sep:   ";",
+			want:  "one/two;iso-8859-1=" + pvPlaceholder,
+		},
+		{
+			input: "one/two; name=\"file.two\"; iso-8859-1",
+			sep:   ";",
+			want:  "one/two; name=\"file.two\"; iso-8859-1=" + pvPlaceholder,
+		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.input, func(t *testing.T) {
diff --git a/part_test.go b/part_test.go
index 4436014..e76866f 100644
--- a/part_test.go
+++ b/part_test.go
@@ -784,3 +784,31 @@ func TestBarrenContentType(t *testing.T) {
 			p.Errors)
 	}
 }
+
+func TestMalformedContentTypeParams(t *testing.T) {
+	r := test.OpenTestData("parts", "malformed-content-type-params.raw")
+	p, err := enmime.ReadParts(r)
+	if err != nil {
+		t.Fatal(err)
+	}
+	wantp := &enmime.Part{
+		PartID:      "0",
+		ContentType: "text/html",
+	}
+	test.ComparePart(t, p, wantp)
+	satisfied := false
+	for _, perr := range p.Errors {
+		if perr.Name == enmime.ErrorMalformedHeader {
+			satisfied = true
+			if perr.Severe {
+				t.Errorf("Expected Severe to be false, got true for %q", perr.Name)
+			}
+		}
+	}
+	if !satisfied {
+		t.Errorf(
+			"Did not find expected error on part. Expected %q, but had: %v",
+			enmime.ErrorMalformedHeader,
+			p.Errors)
+	}
+}
diff --git a/testdata/parts/malformed-content-type-params.raw b/testdata/parts/malformed-content-type-params.raw
new file mode 100644
index 0000000..a6f8d67
--- /dev/null
+++ b/testdata/parts/malformed-content-type-params.raw
@@ -0,0 +1,2 @@
+Content-Type: text/html;iso-8859-1
+