Repository: jhillyerd/enmime (go)

Issue:
Attachment having semicolon (;) in filenames are not downloading propery. 
What I did: attach a file having name like abc;xyz.doc, test;file.txt

What I expected: File should be properly downloaded with complete name.

What I got: File names are coming partially, it is getting broken  at semicolon (;). So I am only getting file-names as abc, test etc. Also extension is not coming. So files are not opening.

Release or branch I am using: version->v0.8.2, commit id->2a0b399

(Please attach a sample message if you feel it will help reproduce the issue)


Pull request:
ParseMediaType: Don't split quoted strings containing semicolons
- [x] Check RFCs for interaction w/ quoting rules

Fixes #174 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index f49914a..6e4ef97 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,12 @@ Change Log
 All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).
 
+## [Unreleased]
+
+### Fixed
+- Attachment file names containing semicolons are no longer truncated (#174)
+
+
 ## [0.8.3] - 2020-11-05
 
 ### Fixed
@@ -159,7 +165,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 ### Added
 - Initial implementation of MIME encoding, using `enmime.MailBuilder`
 
-[Unreleased]: https://github.com/jhillyerd/enmime/compare/master...develop
+[Unreleased]: https://github.com/jhillyerd/enmime/compare/v0.8.3...master
 [0.8.3]:      https://github.com/jhillyerd/enmime/compare/v0.8.2...v0.8.3
 [0.8.2]:      https://github.com/jhillyerd/enmime/compare/v0.8.1...v0.8.2
 [0.8.1]:      https://github.com/jhillyerd/enmime/compare/v0.8.0...v0.8.1
diff --git a/header.go b/header.go
index 9f588fb..9a4c942 100644
--- a/header.go
+++ b/header.go
@@ -9,6 +9,7 @@ import (
 	"strings"
 
 	"github.com/jhillyerd/enmime/internal/coding"
+	"github.com/jhillyerd/enmime/internal/stringutil"
 	"github.com/pkg/errors"
 )
 
@@ -221,7 +222,8 @@ func quotedDisplayName(s string) string {
 //   * Repeating media parameters
 //   * Unquoted values in media parameters containing 'tspecials' characters
 func ParseMediaType(ctype string) (mtype string, params map[string]string, invalidParams []string, err error) {
-	mtype, params, err = mime.ParseMediaType(fixUnescapedQuotes(fixUnquotedSpecials(fixMangledMediaType(ctype, ";"))))
+	mtype, params, err = mime.ParseMediaType(
+		fixUnescapedQuotes(fixUnquotedSpecials(fixMangledMediaType(ctype, ';'))))
 	if err != nil {
 		if err.Error() == "mime: no media type" {
 			return "", nil, nil, nil
@@ -243,16 +245,17 @@ func ParseMediaType(ctype string) (mtype string, params map[string]string, inval
 
 // fixMangledMediaType is used to insert ; separators into media type strings that lack them, and
 // remove repeated parameters.
-func fixMangledMediaType(mtype, sep string) string {
+func fixMangledMediaType(mtype string, sep rune) string {
+	strsep := string([]rune{sep})
 	if mtype == "" {
 		return ""
 	}
-	parts := strings.Split(mtype, sep)
+	parts := stringutil.SplitQuoted(mtype, sep, '"')
 	mtype = ""
 	if strings.Contains(parts[0], "=") {
 		// A parameter pair at this position indicates we are missing a content-type.
-		parts[0] = fmt.Sprintf("%s%s %s", ctAppOctetStream, sep, parts[0])
-		parts = strings.Split(strings.Join(parts, sep), sep)
+		parts[0] = fmt.Sprintf("%s%s %s", ctAppOctetStream, strsep, parts[0])
+		parts = strings.Split(strings.Join(parts, strsep), strsep)
 	}
 	for i, p := range parts {
 		switch i {
@@ -261,7 +264,7 @@ func fixMangledMediaType(mtype, sep string) string {
 				// The content type is completely missing. Put in a placeholder.
 				p = ctPlaceholder
 			}
-			// Check for missing token after slash
+			// Check for missing token after slash.
 			if strings.HasSuffix(p, "/") {
 				switch p {
 				case ctTextPrefix:
@@ -285,7 +288,7 @@ func fixMangledMediaType(mtype, sep string) string {
 				p = p + "=" + pvPlaceholder
 			}
 
-			// RFC-2047 encoded attribute name
+			// RFC-2047 encoded attribute name.
 			p = rfc2047decode(p)
 
 			pair := strings.SplitAfter(p, "=")
@@ -295,10 +298,9 @@ func fixMangledMediaType(mtype, sep string) string {
 			}
 
 			if strings.ContainsAny(pair[0], "()<>@,;:\"\\/[]?") {
-				// attribute is a strict token and cannot be a quoted-string
-				// if any of the above characters are present in a token it
-				// must be quoted and is therefor an invalid attribute.
-				// Discard the pair.
+				// Attribute is a strict token and cannot be a quoted-string.  If any of the above
+				// characters are present in a token it must be quoted and is therefor an invalid
+				// attribute.  Discard the pair.
 				continue
 			}
 		}
diff --git a/internal/stringutil/split.go b/internal/stringutil/split.go
new file mode 100644
index 0000000..b0cf335
--- /dev/null
+++ b/internal/stringutil/split.go
@@ -0,0 +1,37 @@
+package stringutil
+
+const escape = '\\'
+
+// SplitQuoted splits a string, ignoring separators present inside of quoted runs.  Separators
+// cannot be escaped outside of quoted runs, the escaping will be ignored.
+//
+// Quotes are preserved in result, but the separators are removed.
+func SplitQuoted(s string, sep rune, quote rune) []string {
+	a := make([]string, 0, 8)
+	quoted := false
+	escaped := false
+	p := 0
+	for i, c := range s {
+		if c == escape {
+			// Escape can escape itself.
+			escaped = !escaped
+			continue
+		}
+		if c == quote {
+			quoted = !quoted
+			continue
+		}
+		escaped = false
+		if !quoted && c == sep {
+			a = append(a, s[p:i])
+			p = i + 1
+		}
+	}
+
+	if quoted && quote != 0 {
+		// s contained an unterminated quoted-run, re-split without quoting.
+		return SplitQuoted(s, sep, rune(0))
+	}
+
+	return append(a, s[p:])
+}


Test patch:
diff --git a/header_test.go b/header_test.go
index bd2d5f7..4afe2fb 100644
--- a/header_test.go
+++ b/header_test.go
@@ -163,92 +163,94 @@ func TestDecodeToUTF8Base64Header(t *testing.T) {
 
 func TestFixMangledMediaType(t *testing.T) {
 	testCases := []struct {
-		input, sep, want string
+		input string
+		sep   rune
+		want  string
 	}{
 		{
 			input: "",
-			sep:   "",
+			sep:   ';',
 			want:  "",
 		},
 		{
 			input: `text/HTML; charset=UTF-8; format=flowed; content-transfer-encoding: 7bit=`,
-			sep:   ";",
+			sep:   ';',
 			want:  "text/HTML; charset=UTF-8; format=flowed",
 		},
 		{
 			input: "text/html;charset=",
-			sep:   ";",
+			sep:   ';',
 			want:  "text/html;charset=",
 		},
 		{
 			input: "text/;charset=",
-			sep:   ";",
+			sep:   ';',
 			want:  "text/plain;charset=",
 		},
 		{
 			input: "multipart/;charset=",
-			sep:   ";",
+			sep:   ';',
 			want:  "multipart/mixed;charset=",
 		},
 		{
 			input: "text/plain;",
-			sep:   ";",
+			sep:   ';',
 			want:  "text/plain",
 		},
 		{
 			input: "application/octet-stream;=?UTF-8?B?bmFtZT0iw7DCn8KUwoo=?=You've got a new voice miss call.msg",
-			sep:   ";",
+			sep:   ';',
 			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/; 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\"",
-			sep:   " ",
-			want:  "application/pdf;name=\"file.pdf\"",
+			input: `application/pdf name="file.pdf"`,
+			sep:   ' ',
+			want:  `application/pdf;name="file.pdf"`,
 		},
 		{
-			input: "one/two; name=\"file.two\"; name=\"file.two\"",
-			sep:   ";",
-			want:  "one/two; name=\"file.two\"",
+			input: `one/two; name="file.two"; name="file.two"`,
+			sep:   ';',
+			want:  `one/two; name="file.two"`,
 		},
 		{
-			input: "one/; name=\"file.two\"; name=\"file.two\"",
-			sep:   ";",
-			want:  "application/octet-stream; name=\"file.two\"",
+			input: `one/; name="file.two"; name="file.two"`,
+			sep:   ';',
+			want:  `application/octet-stream; name="file.two"`,
 		},
 		{
-			input: "application/octet-stream; =?UTF-8?B?bmFtZT3DsMKfwpTCii5tc2c=?=",
-			sep:   " ",
+			input: `application/octet-stream; =?UTF-8?B?bmFtZT3DsMKfwpTCii5tc2c=?=`,
+			sep:   ' ',
 			want:  "application/octet-stream;name=\"ð\u009f\u0094\u008a.msg\"",
 		},
 		{
-			input: "one/two name=\"file.two\" name=\"file.two\"",
-			sep:   " ",
-			want:  "one/two;name=\"file.two\"",
+			input: `one/two name="file.two" name="file.two"`,
+			sep:   ' ',
+			want:  `one/two;name="file.two"`,
 		},
 		{
-			input: "; name=\"file.two\"",
-			sep:   ";",
-			want:  ctPlaceholder + "; name=\"file.two\"",
+			input: `; name="file.two"`,
+			sep:   ';',
+			want:  ctPlaceholder + `; name="file.two"`,
 		},
 		{
-			input: "charset=binary; name=\"logoleft.jpg\"",
-			sep:   ";",
-			want:  "application/octet-stream; charset=binary; name=\"logoleft.jpg\"",
+			input: `charset=binary; name="logoleft.jpg"`,
+			sep:   ';',
+			want:  `application/octet-stream; charset=binary; name="logoleft.jpg"`,
 		},
 		{
-			input: "one/two;iso-8859-1",
-			sep:   ";",
-			want:  "one/two;iso-8859-1=" + pvPlaceholder,
+			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,
+			input: `one/two; name="file.two"; iso-8859-1`,
+			sep:   ';',
+			want:  `one/two; name="file.two"; iso-8859-1=` + pvPlaceholder,
 		},
 	}
 	for _, tc := range testCases {
@@ -317,6 +319,10 @@ func TestFixUnquotedSpecials(t *testing.T) {
 			input: "application/octet-stream; param1=\"value1;\"",
 			want:  "application/octet-stream; param1=\"value1;\"",
 		},
+		{
+			input: "application/octet-stream; param1=\"value1;2.txt\"",
+			want:  "application/octet-stream; param1=\"value1;2.txt\"",
+		},
 		{
 			input: "application/octet-stream; param1=\"value 1\"",
 			want:  "application/octet-stream; param1=\"value 1\"",
@@ -728,3 +734,80 @@ func TestCommaDelimitedAddressLists(t *testing.T) {
 		}
 	}
 }
+
+func TestParseMediaType(t *testing.T) {
+	testCases := []struct {
+		label  string            // Test case label.
+		input  string            // Content type to parse.
+		mtype  string            // Expected media type returned.
+		params map[string]string // Expected params returned.
+	}{
+		{
+			label:  "basic filename",
+			input:  "text/html; name=index.html",
+			mtype:  "text/html",
+			params: map[string]string{"name": "index.html"},
+		},
+		{
+			label:  "quoted filename",
+			input:  "text/html; name=\"index.html\"",
+			mtype:  "text/html",
+			params: map[string]string{"name": "index.html"},
+		},
+		{
+			label:  "basic filename trailing separator",
+			input:  "text/html; name=index.html;",
+			mtype:  "text/html",
+			params: map[string]string{"name": "index.html"},
+		},
+		{
+			label:  "quoted filename trailing separator",
+			input:  "text/html; name=\"index.html\";",
+			mtype:  "text/html",
+			params: map[string]string{"name": "index.html"},
+		},
+		{
+			label:  "unclosed quoted filename",
+			input:  "text/html; name=\"index.html",
+			mtype:  "text/html",
+			params: map[string]string{"name": "index.html"},
+		},
+		{
+			label:  "quoted filename with separator",
+			input:  "text/html; name=\"index;a.html\"",
+			mtype:  "text/html",
+			params: map[string]string{"name": "index;a.html"},
+		},
+		{
+			label:  "quoted separator mid-string",
+			input:  "text/html; name=\"index;a.html\"; hash=8675309",
+			mtype:  "text/html",
+			params: map[string]string{"name": "index;a.html", "hash": "8675309"},
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.label, func(t *testing.T) {
+			mtype, params, _, err := ParseMediaType(tc.input)
+
+			if err != nil {
+				t.Errorf("got err %v, want nil", err)
+				return
+			}
+
+			if mtype != tc.mtype {
+				t.Errorf("mtype got %q, want %q", mtype, tc.mtype)
+			}
+
+			for k, v := range tc.params {
+				if params[k] != v {
+					t.Errorf("params[%q] got %q, want %q", k, params[k], v)
+				}
+				// Delete param to allow check for unexpected below.
+				delete(params, k)
+			}
+			for pname := range params {
+				t.Errorf("Found unexpected param: %q=%q", pname, params[pname])
+			}
+		})
+	}
+}
diff --git a/internal/stringutil/split_test.go b/internal/stringutil/split_test.go
new file mode 100644
index 0000000..3e28cd1
--- /dev/null
+++ b/internal/stringutil/split_test.go
@@ -0,0 +1,82 @@
+package stringutil
+
+import (
+	"testing"
+)
+
+func TestSplitQuoted(t *testing.T) {
+	testCases := []struct {
+		input string
+		want  []string
+	}{
+		// All tests split on ; and treat " as quoting character.
+		{
+			input: ``,
+			want:  []string{``},
+		},
+		{
+			input: `;`,
+			want:  []string{``, ``},
+		},
+		{
+			input: `"`,
+			want:  []string{`"`},
+		},
+		{
+			input: `a;b`,
+			want:  []string{`a`, `b`},
+		},
+		{
+			input: `a;b;`,
+			want:  []string{`a`, `b`, ``},
+		},
+		{
+			input: `a;b;c`,
+			want:  []string{`a`, `b`, `c`},
+		},
+		{
+			// Separators are ignored within quoted-runs.
+			input: `a;"b;c";d`,
+			want:  []string{`a`, `"b;c"`, `d`},
+		},
+		{
+			// Unterminated quoted-run will cause quotes to be ignored from the start of the string.
+			input: `"a;b;c;d`,
+			want:  []string{`"a`, `b`, `c`, `d`},
+		},
+		{
+			// Unterminated quoted-run will cause quotes to be ignored from the start of the string.
+			input: `"a;b";"c;d`,
+			want:  []string{`"a`, `b"`, `"c`, `d`},
+		},
+		{
+			// Quotes must be escaped via RFC2047 encoding, not just a backslash.
+			// b through c below must not be treated as a single quoted-run.
+			input: `a;"b\";\"c";d`,
+			want:  []string{`a`, `"b\"`, `\"c"`, `d`},
+		},
+		{
+			input: `a;b\;c`,
+			want:  []string{`a`, `b\`, `c`},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.input, func(t *testing.T) {
+			got := SplitQuoted(tc.input, ';', '"')
+
+			t.Logf("\ngot : %q\nwant: %q\n", got, tc.want)
+
+			if len(got) != len(tc.want) {
+				t.Errorf("got len %v, want len %v", len(got), len(tc.want))
+				return
+			}
+
+			for i, g := range got {
+				if g != tc.want[i] {
+					t.Errorf("Element %v differs", i)
+				}
+			}
+		})
+	}
+}