Repository: jhillyerd/enmime
Pull request #101 (2018-11-29 14:19:00)

Handle Content-Type parameter values with missing closing quotes

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

Coverage increased (+0.01%) to 86.844% when pulling **01d4b14c55ba25897d50863dfa8fe33e9985fb47 on dcormier:dc/quote** into **188078f1a35f6392ccd30b082d10ba9c9a5ab77d on jhillyerd:develop**.

jhillyerd: Looks good to me

Patch:
diff --git a/boundary.go b/boundary.go
index 98b9924..884f5ef 100644
--- a/boundary.go
+++ b/boundary.go
@@ -36,7 +36,7 @@ func newBoundaryReader(reader *bufio.Reader, boundary string) *boundaryReader {
 	}
 }
 
-// readUntilBoundary returns a buffer containing the content up until boundary
+// Read returns a buffer containing the content up until boundary
 func (b *boundaryReader) Read(dest []byte) (n int, err error) {
 	if b.buffer.Len() >= len(dest) {
 		// This read request can be satisfied entirely by the buffer
diff --git a/header.go b/header.go
index 30338b8..fae13e4 100644
--- a/header.go
+++ b/header.go
@@ -414,7 +414,11 @@ findValueEnd:
 		// There is a value that ends with the string. Capture it.
 		param.WriteString(value.String())
 
-		if !valueQuotedOriginally && valueQuoteNeeded {
+		if valueQuotedOriginally || valueQuoteNeeded {
+			// If valueQuotedOriginally is true and we got here,
+			// that means there was no closing quote. So we'll add one.
+			// Otherwise, we're here because it was an unquoted value
+			// with a special char in it, and we had to quote it.
 			param.WriteByte('"')
 		}
 	}
diff --git a/part.go b/part.go
index d210a44..f7b5d5b 100644
--- a/part.go
+++ b/part.go
@@ -192,26 +192,26 @@ func (p *Part) convertFromDetectedCharset(r io.Reader) (io.Reader, error) {
 	// Restore r.
 	r = bytes.NewReader(buf)
 
-	if cs != nil && cs.Confidence >= minCharsetConfidence {
-		// Confidence exceeded our threshold, use detected character set.
-		if p.Charset != "" && !strings.EqualFold(cs.Charset, p.Charset) {
-			p.addWarning(ErrorCharsetDeclaration,
-				"declared charset %q, detected %q, confidence %d",
-				p.Charset, cs.Charset, cs.Confidence)
-		}
+	if cs == nil || cs.Confidence < minCharsetConfidence {
+		// Low confidence, use declared character set.
+		return p.convertFromStatedCharset(r), nil
+	}
 
-		reader, err := coding.NewCharsetReader(cs.Charset, r)
-		if err != nil {
-			// Failed to get a conversion reader.
-			p.addWarning(ErrorCharsetConversion, err.Error())
-		} else {
-			r = reader
-			p.OrigCharset = p.Charset
-			p.Charset = cs.Charset
-		}
+	// Confidence exceeded our threshold, use detected character set.
+	if p.Charset != "" && !strings.EqualFold(cs.Charset, p.Charset) {
+		p.addWarning(ErrorCharsetDeclaration,
+			"declared charset %q, detected %q, confidence %d",
+			p.Charset, cs.Charset, cs.Confidence)
+	}
+
+	reader, err := coding.NewCharsetReader(cs.Charset, r)
+	if err != nil {
+		// Failed to get a conversion reader.
+		p.addWarning(ErrorCharsetConversion, err.Error())
 	} else {
-		// Low confidence, use declared character set.
-		r = p.convertFromStatedCharset(r)
+		r = reader
+		p.OrigCharset = p.Charset
+		p.Charset = cs.Charset
 	}
 
 	return r, nil


Test patch:
diff --git a/header_test.go b/header_test.go
index afa56a3..c893762 100644
--- a/header_test.go
+++ b/header_test.go
@@ -295,6 +295,10 @@ func TestFixUnquotedSpecials(t *testing.T) {
 			input: "application/octet-stream; param1=value/1",
 			want:  "application/octet-stream; param1=\"value/1\"",
 		},
+		{
+			input: `text/HTML; charset="UTF-8Return-Path: bounce-810_HTML-769869545-477063-1070564-43@bounce.email.oflce57578375.com`,
+			want:  `text/HTML; charset="UTF-8Return-Path: bounce-810_HTML-769869545-477063-1070564-43@bounce.email.oflce57578375.com"`,
+		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.input, func(t *testing.T) {
diff --git a/part_test.go b/part_test.go
index e93f1e5..6982471 100644
--- a/part_test.go
+++ b/part_test.go
@@ -818,6 +818,38 @@ func TestContentTypeParamUnquotedSpecial(t *testing.T) {
 	test.ComparePart(t, p, wantp)
 }
 
+func TestContentTypeParamMissingClosingQuote(t *testing.T) {
+	r := test.OpenTestData("parts", "missing-closing-param-quote.raw")
+	p, err := enmime.ReadParts(r)
+	if err != nil {
+		t.Fatalf("%+v", err)
+	}
+
+	wantp := &enmime.Part{
+		PartID:      "0",
+		ContentType: "text/html",
+		Charset:     "UTF-8Return-Path:;bounce-810_HTML-769869545-477063-1070564-43@bounce.email.oflce57578375.com=not-a-param-value",
+	}
+	test.ComparePart(t, p, wantp)
+
+	expected := enmime.ErrorCharsetConversion
+	satisfied := false
+	for _, perr := range p.Errors {
+		if perr.Name == expected {
+			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",
+			expected,
+			p.Errors)
+	}
+}
+
 func TestChardetFailure(t *testing.T) {
 	const expectedContent = "GIF89ad\x00\x04\x00\x80\x00\x00\x00f\xccf\xff\x99!\xf9\x04\x00\x00\x00\x00\x00,\x00\x00\x00\x00d\x00\x04\x00\x00\x02\x1a\x8c\x8f\xa9\xcb\xed\x0f\xa3\x9c\xb4\xda\xeb\x80\u07bc\xfb\x0f\x86\xe2H\x96扦\xea*\x16\x00;"
 
diff --git a/testdata/parts/missing-closing-param-quote.raw b/testdata/parts/missing-closing-param-quote.raw
new file mode 100644
index 0000000..b25d58d
--- /dev/null
+++ b/testdata/parts/missing-closing-param-quote.raw
@@ -0,0 +1,3 @@
+Content-type: text/HTML; charset="UTF-8Return-Path: bounce-810_HTML-769869545-477063-1070564-43@bounce.email.oflce57578375.com
+MIME-Version: 1.0
+