Repository: jhillyerd/enmime
Pull request #121 (2019-01-25 20:02:31)

Implementation which does not depend on successive function calls
Only quoting the head of the attribute value and leaving the end unquoted was a hack. This will now properly tokenize the string, handle multiple RFC2047 chunks if present and quote both ends of the attribute value as needed.

requaos: Force-pushed for rebase on develop
coveralls: 
[![Coverage Status](https://coveralls.io/builds/21281874/badge)](https://coveralls.io/builds/21281874)

Coverage decreased (-0.7%) to 86.792% when pulling **d22bbe765a1255bb4fb5ab6c1b7316b54e8245ae on requaos:requaos/morebetter** into **32384ef9fb4bf35ba74c859d7aeea172f62db744 on jhillyerd:develop**.


Patch:
diff --git a/header.go b/header.go
index f8c5ccb..a4a946c 100644
--- a/header.go
+++ b/header.go
@@ -20,11 +20,13 @@ const (
 	cdInline     = "inline"
 
 	// Standard MIME content types
+	ctAppPrefix        = "application/"
 	ctAppOctetStream   = "application/octet-stream"
 	ctMultipartAltern  = "multipart/alternative"
 	ctMultipartMixed   = "multipart/mixed"
 	ctMultipartPrefix  = "multipart/"
 	ctMultipartRelated = "multipart/related"
+	ctTextPrefix       = "text/"
 	ctTextPlain        = "text/plain"
 	ctTextHTML         = "text/html"
 
@@ -269,6 +271,20 @@ func fixMangledMediaType(mtype, sep string) string {
 				// The content type is completely missing. Put in a placeholder.
 				p = ctPlaceholder
 			}
+			// Check for missing token after slash
+			if strings.HasSuffix(p, "/") {
+				switch p {
+				case ctTextPrefix:
+					p = ctTextPlain
+				case ctAppPrefix:
+					p = ctAppOctetStream
+				case ctMultipartPrefix:
+					p = ctMultipartMixed
+				default:
+					// Safe default
+					p = ctAppOctetStream
+				}
+			}
 		default:
 			if !strings.Contains(p, "=") {
 				p = p + "=" + pvPlaceholder
@@ -277,8 +293,8 @@ func fixMangledMediaType(mtype, sep string) string {
 			// RFC-2047 encoded attribute name
 			p = rfc2047AttributeName(p)
 
-			pair := strings.Split(p, "=")
-			if strings.Contains(mtype, pair[0]+"=") {
+			pair := strings.SplitAfter(p, "=")
+			if strings.Contains(mtype, pair[0]) {
 				// Ignore repeated parameters.
 				continue
 			}
@@ -505,20 +521,51 @@ func whiteSpaceRune(r rune) bool {
 // rfc2047AttributeName checks if the attribute name is encoded in RFC2047 format
 // RFC2047 Example:
 //     `=?UTF-8?B?bmFtZT0iw7DCn8KUwoo=?=`
-func rfc2047AttributeName(s string) string {
-	if !strings.Contains(s, "?=") {
-		return s
+func rfc2047AttributeName(name string) string {
+	if !strings.Contains(name, "?=") {
+		return name
 	}
-	pair := strings.SplitAfter(s, "?=")
-	// lets assume that the attribute was encoded because of unicode characters being present
-	// then the attribute value should be quoted
-	keyValuePair := strings.SplitAfter(decodeHeader(pair[0]), "=")
-	// only quote the parameter value if it isn't already quoted
-	if len(keyValuePair) > 1 {
-		if !strings.HasPrefix(keyValuePair[1], "\"") {
-			keyValuePair[1] = fmt.Sprintf("\"%s", keyValuePair[1])
+	// copy the string so we can return the original if we encounter any issues
+	s := name
+
+	// handle n-number of RFC2047 chunk occurrences
+	count := strings.Count(name, "?=")
+	result := &strings.Builder{}
+	var beginning, ending int
+	for i := 0; i < count; i++ {
+		beginning = strings.Index(s, "=?")
+		ending = strings.Index(s, "?=")
+
+		if beginning == -1 || ending == -1 {
+			// the RFC2047 chunk is either malformed or is not an RFC2047 chunk
+			return name
+		}
+
+		_, err := result.WriteString(s[:beginning])
+		if err != nil {
+			return name
+		}
+		_, err = result.WriteString(decodeHeader(s[beginning : ending+2]))
+		if err != nil {
+			return name
 		}
+
+		s = s[ending+2:]
+	}
+	_, err := result.WriteString(s)
+	if err != nil {
+		return name
+	}
+	keyValuePair := strings.SplitAfter(result.String(), "=")
+	if len(keyValuePair) < 2 {
+		return result.String()
+	}
+	// Add quotes as needed
+	if !strings.HasPrefix(keyValuePair[1], "\"") {
+		keyValuePair[1] = fmt.Sprintf("\"%s", keyValuePair[1])
+	}
+	if !strings.HasSuffix(keyValuePair[1], "\"") {
+		keyValuePair[1] = fmt.Sprintf("%s\"", keyValuePair[1])
 	}
-	pair[0] = strings.Join(keyValuePair, "")
-	return strings.Join(pair, "")
+	return strings.Join(keyValuePair, "")
 }


Test patch:
diff --git a/cmd/example_test.go b/cmd/example_test.go
index 91a33cb..acd7678 100644
--- a/cmd/example_test.go
+++ b/cmd/example_test.go
@@ -24,6 +24,34 @@ Text section.
 Content-Type: text/html
 
 <em>HTML</em> section.
+--Enmime-Test-100
+Content-Transfer-Encoding: base64
+Content-Disposition: inline;
+	filename=favicon.png
+Content-Type: image/png;
+	x-unix-mode=0644;
+	name="favicon.png"
+Content-Id: <8B8481A2-25CA-4886-9B5A-8EB9115DD064@skynet>
+
+iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAAGXRFWHRTb2Z0d2FyZQBBZG9iZSBJ
+bWFnZVJlYWR5ccllPAAAAlFJREFUeNqUU8tOFEEUPVVdNV3dPe8xYRBnjGhmBgKjKzCIiQvBoIaN
+bly5Z+PSv3Aj7DSiP2B0rwkLGVdGgxITSCRIJGSMEQWZR3eVt5sEFBgTb/dN1yvnnHtPNTPG4Pqd
+HgCMXnPRSZrpSuH8vUJu4DE4rYHDGAZDX62BZttHqTiIayM3gGiXQsgYLEvATaqxU+dy1U13YXap
+XptpNHY8iwn8KyIAzm1KBdtRZWErpI5lEWTXp5Z/vHpZ3/wyKKwYGGOdAYwR0EZwoezTYApBEIOb
+yELl/aE1/83cp40Pt5mxqCKrE4Ck+mVWKKcI5tA8BLEhRBKJLjez6a7MLq7XZtp+yyOawwCBtkiB
+VZDKzRk4NN7NQBMYPHiZDFhXY+p9ff7F961vVcnl4R5I2ykJ5XFN7Ab7Gc61VoipNBKF+PDyztu5
+lfrSLT/wIwCxq0CAGtXHZTzqR2jtwQiXONma6hHpj9sLT7YaPxfTXuZdBGA02Wi7FS48YiTfj+i2
+NhqtdhP5RC8mh2/Op7y0v6eAcWVLFT8D7kWX5S9mepp+C450MV6aWL1cGnvkxbwHtLW2B9AOkLeU
+d9KEDuh9fl/7CEj7YH5g+3r/lWfF9In7tPz6T4IIwBJOr1SJyIGQMZQbsh5P9uBq5VJtqHh2mo49
+pdw5WFoEwKWqWHacaWOjQXWGcifKo6vj5RGS6zykI587XeUIQDqJSmAp+lE4qt19W5P9o8+Lma5D
+cjsC8JiT607lMVkdqQ0Vyh3lHhmh52tfNy78ajXv0rgYzv8nfwswANuk+7sD/Q0aAAAAAElFTkSu
+QmCC
+--Enmime-Test-100
+Content-Transfer-Encoding: base64
+Content-Type: text/html; name="test.html"
+Content-Disposition: attachment; filename=test.html
+
+PGh0bWw+Cg==
 --Enmime-Test-100--
 `
 	// Convert MIME text to Envelope
@@ -66,14 +94,19 @@ Content-Type: text/html
 	// <em>HTML</em> section.
 	//
 	// ## Attachment List
+	// - test.html (text/html)
 	//
 	// ## Inline List
+	// - favicon.png (image/png)
+	//   Content-ID: 8B8481A2-25CA-4886-9B5A-8EB9115DD064@skynet
 	//
 	// ## Other Part List
 	//
 	// ## MIME Part Tree
 	//     multipart/mixed
 	//     |-- text/plain
-	//     `-- text/html
+	//     |-- text/html
+	//     |-- image/png, disposition: inline, filename: "favicon.png"
+	//     `-- text/html, disposition: attachment, filename: "test.html"
 	//
 }
diff --git a/header_test.go b/header_test.go
index 0e8d484..d6abcd2 100644
--- a/header_test.go
+++ b/header_test.go
@@ -182,7 +182,12 @@ func TestFixMangledMediaType(t *testing.T) {
 		{
 			input: "application/octet-stream;=?UTF-8?B?bmFtZT0iw7DCn8KUwoo=?=You've got a new voice miss call.msg",
 			sep:   ";",
-			want:  "application/octet-stream;name=\"ð\u009f\u0094\u008aYou've got a new voice miss call.msg",
+			want:  "application/octet-stream;name=\"ð\u009f\u0094\u008aYou've got a new voice miss call.msg\"",
+		},
+		{
+			input: "application/; name=\"Voice message from =?UTF-8?B?4piOICsxIDI1MS0yNDUtODA0NC5tc2c=?=\";",
+			sep:   ";",
+			want:  "application/octet-stream; name=\"Voice message from ☎ +1 251-245-8044.msg\"",
 		},
 		{
 			input: "application/pdf name=\"file.pdf\"",
@@ -197,7 +202,7 @@ func TestFixMangledMediaType(t *testing.T) {
 		{
 			input: "application/octet-stream; =?UTF-8?B?bmFtZT3DsMKfwpTCii5tc2c=?=",
 			sep:   " ",
-			want:  "application/octet-stream;name=\"ð.msg",
+			want:  "application/octet-stream;name=\"ð.msg\"",
 		},
 		{
 			input: "one/two name=\"file.two\" name=\"file.two\"",
diff --git a/internal/coding/charsets_test.go b/internal/coding/charsets_test.go
index f06055a..57a4ad2 100644
--- a/internal/coding/charsets_test.go
+++ b/internal/coding/charsets_test.go
@@ -69,3 +69,30 @@ func TestFindCharsetInHTML(t *testing.T) {
 		}
 	}
 }
+
+func TestConvertToUTF8String(t *testing.T) {
+	var testTable = []struct {
+		charset string
+		input   []byte
+		want    string
+	}{
+		{"utf-8", []byte("abcABC\u2014"), "abcABC\u2014"},
+		{"windows-1250", []byte{'a', 'Z', 0x96}, "aZ\u2013"},
+		{"big5", []byte{0xa1, 0x5d, 0xa1, 0x61, 0xa1, 0x71}, "\uff08\uff5b\u3008"},
+	}
+	// Success Conditions
+	for _, v := range testTable {
+		s, err := coding.ConvertToUTF8String(v.charset, v.input)
+		if err != nil {
+			t.Error("UTF-8 conversion failed")
+		}
+		if s != v.want {
+			t.Errorf("Got %s, but wanted %s", s, v.want)
+		}
+	}
+	// Fail for unsupported charset
+	_, err := coding.ConvertToUTF8String("123", []byte("there is no 123 charset"))
+	if err == nil {
+		t.Error("Charset 123 should not exist")
+	}
+}
diff --git a/internal/test/testing_test.go b/internal/test/testing_test.go
index a0599fb..c921af0 100644
--- a/internal/test/testing_test.go
+++ b/internal/test/testing_test.go
@@ -1,6 +1,8 @@
 package test
 
 import (
+	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/jhillyerd/enmime"
@@ -102,3 +104,154 @@ func TestHelperComparePartsInequal(t *testing.T) {
 		})
 	}
 }
+
+// TestOpenTestDataPanic verifies that this function will panic as predicted
+func TestOpenTestDataPanic(t *testing.T) {
+	defer func() {
+		if r := recover(); r == nil {
+			t.Error("OpenTestData did not panic")
+		}
+	}()
+	_ = OpenTestData("invalidDir", "invalidFile")
+}
+
+// TestOpenTestData ensures that the returned io.Reader has the correct underlying type, that
+// the file descriptor referenced is a directory and that we have permission to read it
+func TestOpenTestData(t *testing.T) {
+	// this will open a handle to the "testdata" directory
+	r := OpenTestData("", "")
+	if r == nil {
+		t.Error("The returned io.Reader should not be nil")
+	}
+
+	osFilePtr, ok := r.(*os.File)
+	if !ok {
+		t.Errorf("Underlying type should be *os.File, but got %T instead", r)
+	}
+
+	info, err := osFilePtr.Stat()
+	if err != nil {
+		t.Error("We should have read permission for \"testdata\" directory")
+	}
+
+	if !info.IsDir() {
+		t.Error("File descriptor labeled \"testdata\" should be a directory")
+	}
+}
+
+// TestContentContainsString checks if the string contains a provided sub-string
+func TestContentContainsString(t *testing.T) {
+	// Success
+	ContentContainsString(t, []byte("someString"), "some")
+	// Failure
+	ContentContainsString(&testing.T{}, []byte("someString"), "nope")
+}
+
+// TestContentEqualsString checks if the strings are equal
+func TestContentEqualsString(t *testing.T) {
+	// Success
+	ContentEqualsString(t, []byte("someString"), "someString")
+	// Failure
+	ContentEqualsString(&testing.T{}, []byte("someString"), "nope")
+}
+
+// TestContentEqualsBytes checks if the slices of bytes are equal
+func TestContentEqualsBytes(t *testing.T) {
+	// Success
+	ContentEqualsBytes(t, []byte("someString"), []byte("someString"))
+	// Failure
+	ContentEqualsBytes(&testing.T{}, []byte("someString"), []byte("nope"))
+}
+
+// TestCompareEnvelope checks all publicly accessible members of an envelope for differences
+func TestCompareEnvelope(t *testing.T) {
+	fileA, err := os.Open(filepath.Join("..", "..", "testdata", "mail", "attachment.raw"))
+	if err != nil {
+		t.Error(err)
+	}
+	envelopeA, err := enmime.ReadEnvelope(fileA)
+	if err != nil {
+		t.Error(err)
+	}
+
+	// Success
+	success := CompareEnvelope(t, envelopeA, envelopeA)
+	if !success {
+		t.Error("Same file should have identical envelopes")
+	}
+
+	// Success on "want" and "got" nil
+	success = CompareEnvelope(t, nil, nil)
+	if !success {
+		t.Error("Comparing nil to nil should result in true")
+	}
+
+	// Fail on "got" nil
+	success = CompareEnvelope(&testing.T{}, nil, envelopeA)
+	if success {
+		t.Error("Got is nil, envelopeA should not be the same")
+	}
+
+	// Fail on "want" nil
+	success = CompareEnvelope(&testing.T{}, envelopeA, nil)
+	if success {
+		t.Error("Want is nil, envelopeA should not be the same")
+	}
+
+	// Fail on root Part mismatch nil
+	envelopeB := *envelopeA
+	envelopeB.Root = nil
+	success = CompareEnvelope(&testing.T{}, envelopeA, &envelopeB)
+	if success {
+		t.Error("Envelope Root parts should not be the same")
+	}
+	envelopeB.Root = envelopeA.Root
+
+	// Fail on Text mismatch
+	envelopeB.Text = "mismatch"
+	success = CompareEnvelope(&testing.T{}, envelopeA, &envelopeB)
+	if success {
+		t.Error("Envelope Text parts should not be the same")
+	}
+	envelopeB.Text = envelopeA.Text
+
+	// Fail on HTML mismatch
+	envelopeB.HTML = "mismatch"
+	success = CompareEnvelope(&testing.T{}, envelopeA, &envelopeB)
+	if success {
+		t.Error("Envelope HTML parts should not be the same")
+	}
+	envelopeB.HTML = envelopeA.HTML
+
+	// Fail on Attachment count mismatch
+	envelopeB.Attachments = append(envelopeB.Attachments, &enmime.Part{})
+	success = CompareEnvelope(&testing.T{}, envelopeA, &envelopeB)
+	if success {
+		t.Error("Envelope Attachment slices should not be the same")
+	}
+	envelopeB.Attachments = envelopeA.Attachments
+
+	// Fail on Inlines count mismatch
+	envelopeB.Inlines = append(envelopeB.Inlines, &enmime.Part{})
+	success = CompareEnvelope(&testing.T{}, envelopeA, &envelopeB)
+	if success {
+		t.Error("Envelope Inlines slices should not be the same")
+	}
+	envelopeB.Inlines = envelopeA.Inlines
+
+	// Fail on OtherParts count mismatch
+	envelopeB.OtherParts = append(envelopeB.OtherParts, &enmime.Part{})
+	success = CompareEnvelope(&testing.T{}, envelopeA, &envelopeB)
+	if success {
+		t.Error("Envelope OtherParts slices should not be the same")
+	}
+	envelopeB.OtherParts = envelopeA.OtherParts
+
+	// Fail on Errors count mismatch
+	envelopeB.Errors = append(envelopeB.Errors, &enmime.Error{})
+	success = CompareEnvelope(&testing.T{}, envelopeA, &envelopeB)
+	if success {
+		t.Error("Envelope Errors slices should not be the same")
+	}
+	envelopeB.Errors = envelopeA.Errors
+}