Repository: jhillyerd/enmime
Pull request #256 (2022-06-09 10:40:58)

fix(mediatype): quoted semicolon
This PR fixes splitting of Content-Type header, when parameter has semicolon inside quoted text`;`, e.g. `multipart/mixed; boundary="aaa;bbb;ccc"`. Previously, it was splitting wrongly, causing `fixUnescapedQuotes` function to break Content-Type header, making it `multipart/mixed; boundary=""`

Introduce new `FindQuoted` function. Reasoning: because for the splitting we still need to go through whole string, we can as well find all points where we need split. And then, use it for `IndexQuoted` or `CountQuoted` (analog of `strings.Index` and `strings.Count`), if we'll need it the future.


Patch:
diff --git a/internal/stringutil/find.go b/internal/stringutil/find.go
new file mode 100644
index 0000000..419716b
--- /dev/null
+++ b/internal/stringutil/find.go
@@ -0,0 +1,39 @@
+package stringutil
+
+// FindUnquoted returns the indexes of the instance of v in s, or empty slice if v is not present in s.
+// It ignores v present inside quoted runs.
+func FindUnquoted(s string, v rune, quote rune) []int {
+	escaped := false
+	quoted := false
+	indexes := make([]int, 0)
+	quotedIndexes := make([]int, 0)
+
+	for i := 0; i < len(s); i++ {
+		switch rune(s[i]) {
+		case escape:
+			escaped = !escaped // escape can escape itself.
+		case quote:
+			if escaped {
+				escaped = false
+				continue
+			}
+
+			quoted = !quoted
+			if !quoted {
+				quotedIndexes = quotedIndexes[:0] // drop possible indices inside quoted segment
+			}
+		case v:
+			escaped = false
+			if quoted {
+				quotedIndexes = append(quotedIndexes, i)
+			} else {
+				indexes = append(indexes, i)
+			}
+		default:
+			escaped = false
+		}
+
+	}
+
+	return append(indexes, quotedIndexes...)
+}
diff --git a/internal/stringutil/split.go b/internal/stringutil/split.go
index b0cf335..04051be 100644
--- a/internal/stringutil/split.go
+++ b/internal/stringutil/split.go
@@ -2,36 +2,44 @@ 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.
+// SplitUnquoted slices s into all substrings separated by sep and returns a slice of
+// the substrings between those separators.
 //
-// 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 s does not contain sep and sep is not empty, SplitUnquoted returns a
+// slice of length 1 whose only element is s.
+//
+// It ignores sep present inside quoted runs.
+func SplitUnquoted(s string, sep rune, quote rune) []string {
+	return splitUnquoted(s, sep, quote, false)
+}
+
+// SplitAfterUnquoted slices s into all substrings after each instance of sep and
+// returns a slice of those substrings.
+//
+// If s does not contain sep and sep is not empty, SplitAfterUnquoted returns
+// a slice of length 1 whose only element is s.
+//
+// It ignores sep present inside quoted runs.
+func SplitAfterUnquoted(s string, sep rune, quote rune) []string {
+	return splitUnquoted(s, sep, quote, true)
+}
+
+func splitUnquoted(s string, sep rune, quote rune, preserveSep bool) []string {
+	ixs := FindUnquoted(s, sep, quote)
+	if len(ixs) == 0 {
+		return []string{s}
 	}
 
-	if quoted && quote != 0 {
-		// s contained an unterminated quoted-run, re-split without quoting.
-		return SplitQuoted(s, sep, rune(0))
+	start := 0
+	result := make([]string, 0, len(ixs)+1)
+	for _, ix := range ixs {
+		end := ix
+		if preserveSep {
+			end++
+		}
+		result = append(result, s[start:end])
+		start = ix + 1
 	}
 
-	return append(a, s[p:])
+	return append(result, s[start:])
 }
diff --git a/mediatype/mediatype.go b/mediatype/mediatype.go
index ea64a73..af46731 100644
--- a/mediatype/mediatype.go
+++ b/mediatype/mediatype.go
@@ -69,7 +69,7 @@ func fixMangledMediaType(mtype string, sep rune) string {
 		return ""
 	}
 
-	parts := stringutil.SplitQuoted(mtype, sep, '"')
+	parts := stringutil.SplitUnquoted(mtype, sep, '"')
 	mtype = ""
 	if strings.Contains(parts[0], "=") {
 		// A parameter pair at this position indicates we are missing a content-type.
@@ -384,7 +384,7 @@ func fixUnquotedSpecials(s string) string {
 //  Input:  application/rtf; charset=iso-8859-1; name=""V047411.rtf".rtf"
 //  Output: application/rtf; charset=iso-8859-1; name="\"V047411.rtf\".rtf"
 func fixUnescapedQuotes(hvalue string) string {
-	params := strings.SplitAfter(hvalue, ";")
+	params := stringutil.SplitAfterUnquoted(hvalue, ';', '"')
 	sb := &strings.Builder{}
 
 	for i := 0; i < len(params); i++ {


Test patch:
diff --git a/internal/stringutil/find_test.go b/internal/stringutil/find_test.go
new file mode 100644
index 0000000..4a71df8
--- /dev/null
+++ b/internal/stringutil/find_test.go
@@ -0,0 +1,98 @@
+package stringutil
+
+import "testing"
+
+func TestFindUnquoted(t *testing.T) {
+	findRune := ';'
+	quoteRune := '"'
+	testCases := []struct {
+		input string
+		want  []int
+	}{
+		{
+			input: ``,
+			want:  []int{},
+		},
+		{
+			input: `;`,
+			want:  []int{0},
+		},
+		{
+			input: `\;`, // not escaped
+			want:  []int{1},
+		},
+		{
+			input: `a;b`, // single
+			want:  []int{1},
+		},
+		{
+			input: `a\;b;c`, // not escaped, multiple
+			want:  []int{2, 4},
+		},
+		{
+			input: `\`, // just escape
+			want:  []int{},
+		},
+		{
+			input: `\\;`, // escaped escape
+			want:  []int{2},
+		},
+		{
+			input: `"`, // just quote
+			want:  []int{},
+		},
+		{
+			input: `a";b"`, // inside quotes - ignored
+			want:  []int{},
+		},
+		{
+			input: `"a"";b"`, // inside quotes - ignored
+			want:  []int{},
+		},
+		{
+			input: `a\";b"`, // escaped quote - ignore quote
+			want:  []int{3},
+		},
+		{
+			input: `"a;b;c;d`, // unterminated quote at the beginning - ignore
+			want:  []int{2, 4, 6},
+		},
+		{
+			input: `a"`, // unterminated quote at the end - ignore
+			want:  []int{},
+		},
+		{
+			input: `ab";c`, // unterminated quote at the middle - ignore
+			want:  []int{3},
+		},
+		{
+			input: `"a;b""c;d`, // unterminated quote with properly terminated quote
+			want:  []int{7},
+		},
+		{
+			input: `a"b\";\"c";d`, // inside escaped quotes which should be ignored
+			want:  []int{10},
+		},
+		{
+			input: `a;"b";""`,
+			want:  []int{1, 5},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.input, func(t *testing.T) {
+			got := FindUnquoted(tc.input, findRune, quoteRune)
+
+			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: got %q, want %q", i, g, tc.want[i])
+				}
+			}
+		})
+	}
+}
diff --git a/internal/stringutil/split_test.go b/internal/stringutil/split_test.go
index 3e28cd1..6042fe2 100644
--- a/internal/stringutil/split_test.go
+++ b/internal/stringutil/split_test.go
@@ -4,7 +4,7 @@ import (
 	"testing"
 )
 
-func TestSplitQuoted(t *testing.T) {
+func TestSplitUnquoted(t *testing.T) {
 	testCases := []struct {
 		input string
 		want  []string
@@ -44,28 +44,99 @@ func TestSplitQuoted(t *testing.T) {
 			input: `"a;b;c;d`,
 			want:  []string{`"a`, `b`, `c`, `d`},
 		},
+		{
+			input: `"a;b";"c;d`,
+			want:  []string{`"a;b"`, `"c`, `d`},
+		},
+		{
+			input: `a;"b\";\"c";d`,
+			want:  []string{`a`, `"b\";\"c"`, `d`},
+		},
+		{
+			input: `a;"b";""`,
+			want:  []string{`a`, `"b"`, `""`},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.input, func(t *testing.T) {
+			got := SplitUnquoted(tc.input, ';', '"')
+
+			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: got %q, want %q", i, g, tc.want[i])
+				}
+			}
+		})
+	}
+}
+
+func TestSplitAfterUnquoted(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`},
+		},
+		{
 			input: `"a;b";"c;d`,
-			want:  []string{`"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`},
+			want:  []string{`a;`, `"b\";\"c";`, `d`},
 		},
 		{
 			input: `a;b\;c`,
-			want:  []string{`a`, `b\`, `c`},
+			want:  []string{`a;`, `b\;`, `c`},
+		},
+		{
+			input: `a;"b";""`,
+			want:  []string{`a;`, `"b";`, `""`},
 		},
 	}
 
 	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)
+			got := SplitAfterUnquoted(tc.input, ';', '"')
 
 			if len(got) != len(tc.want) {
 				t.Errorf("got len %v, want len %v", len(got), len(tc.want))
@@ -74,7 +145,7 @@ func TestSplitQuoted(t *testing.T) {
 
 			for i, g := range got {
 				if g != tc.want[i] {
-					t.Errorf("Element %v differs", i)
+					t.Errorf("element %v differs: got %q, want %q", i, g, tc.want[i])
 				}
 			}
 		})
diff --git a/mediatype/mediatype_test.go b/mediatype/mediatype_test.go
index 7100e91..94f6062 100644
--- a/mediatype/mediatype_test.go
+++ b/mediatype/mediatype_test.go
@@ -355,6 +355,10 @@ func TestFixUnEscapedQuotes(t *testing.T) {
 			input: `application/rtf; charset=utf-8; name=""žába".jpg"`,
 			want:  `application/rtf; charset=utf-8; name="\"žába\".jpg"`,
 		},
+		{
+			input: `multipart/mixed; boundary="aaa;bbb;ccc"`, // `;` inside quoted text, should ignore it
+			want:  `multipart/mixed; boundary="aaa;bbb;ccc"`,
+		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.input, func(t *testing.T) {