mime/multipart: allow nested boundary with outer boundary+dash prefix

Fixes #46042

Change-Id: Icd243eb12c6e260aeead04710f12340048a0e859
Reviewed-on: https://go-review.googlesource.com/c/go/+/338549
Trust: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Trust: Cherry Mui <cherryyz@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>

diff --git a/src/mime/multipart/multipart.go b/src/mime/multipart/multipart.go
index 1054e7a4ce16e53a741d6e3f3e2188c30fa82a56..c7bcb4d121971bf209ff4b36169b5c8eba178459 100644 (file)
--- a/src/mime/multipart/multipart.go
+++ b/src/mime/multipart/multipart.go
@@ -264,7 +264,8 @@ func scanUntilBoundary(buf, dashBoundary, nlDashBoundary []byte, total int64, re
 // and the caller has verified already that bytes.HasPrefix(buf, prefix) is true.
 //
 // matchAfterPrefix returns +1 if the buffer does match the boundary,
-// meaning the prefix is followed by a dash, space, tab, cr, nl, or end of input.
+// meaning the prefix is followed by a double dash, space, tab, cr, nl,
+// or end of input.
 // It returns -1 if the buffer definitely does NOT match the boundary,
 // meaning the prefix is followed by some other character.
 // For example, "--foobar" does not match "--foo".
@@ -278,9 +279,25 @@ func matchAfterPrefix(buf, prefix []byte, readErr error) int {
                return 0
        }
        c := buf[len(prefix)]
-       if c == ' ' || c == '\t' || c == '\r' || c == '\n' || c == '-' {
+
+       if c == ' ' || c == '\t' || c == '\r' || c == '\n' {
                return +1
        }
+
+       // Try to detect boundaryDash
+       if c == '-' {
+               if len(buf) == len(prefix)+1 {
+                       if readErr != nil {
+                               // Prefix + "-" does not match
+                               return -1
+                       }
+                       return 0
+               }
+               if buf[len(prefix)+1] == '-' {
+                       return +1
+               }
+       }
+
        return -1
 }
 

diff --git a/src/mime/multipart/multipart_test.go b/src/mime/multipart/multipart_test.go
index 741d2304ed45b332fa84eb8fa2f6f55f480540c0..e043e36ef7ff762530700f04e5f9b4356b6c65c9 100644 (file)
--- a/src/mime/multipart/multipart_test.go
+++ b/src/mime/multipart/multipart_test.go
@@ -291,24 +291,34 @@ func TestLineLimit(t *testing.T) {
 }
 
 func TestMultipartTruncated(t *testing.T) {
-       testBody := `
+       for _, body := range []string{
+               `
 This is a multi-part message.  This line is ignored.
 --MyBoundary
 foo-bar: baz
 
 Oh no, premature EOF!
-`
-       body := strings.ReplaceAll(testBody, "\n", "\r\n")
-       bodyReader := strings.NewReader(body)
-       r := NewReader(bodyReader, "MyBoundary")
+`,
+               `
+This is a multi-part message.  This line is ignored.
+--MyBoundary
+foo-bar: baz
 
-       part, err := r.NextPart()
-       if err != nil {
-               t.Fatalf("didn't get a part")
-       }
-       _, err = io.Copy(io.Discard, part)
-       if err != io.ErrUnexpectedEOF {
-               t.Fatalf("expected error io.ErrUnexpectedEOF; got %v", err)
+Oh no, premature EOF!
+--MyBoundary-`,
+       } {
+               body = strings.ReplaceAll(body, "\n", "\r\n")
+               bodyReader := strings.NewReader(body)
+               r := NewReader(bodyReader, "MyBoundary")
+
+               part, err := r.NextPart()
+               if err != nil {
+                       t.Fatalf("didn't get a part")
+               }
+               _, err = io.Copy(io.Discard, part)
+               if err != io.ErrUnexpectedEOF {
+                       t.Fatalf("expected error io.ErrUnexpectedEOF; got %v", err)
+               }
        }
 }
 
@@ -751,6 +761,7 @@ html things
                        },
                },
        },
+
        // Issue 12662: Check that we don't consume the leading \r if the peekBuffer
        // ends in '\r\n--separator-'
        {
@@ -767,6 +778,7 @@ Content-Type: application/octet-stream
                        },
                },
        },
+
        // Issue 12662: Same test as above with \r\n at the end
        {
                name: "peek buffer boundary condition",
@@ -782,6 +794,7 @@ Content-Type: application/octet-stream
                        },
                },
        },
+
        // Issue 12662v2: We want to make sure that for short buffers that end with
        // '\r\n--separator-' we always consume at least one (valid) symbol from the
        // peekBuffer
@@ -799,6 +812,7 @@ Content-Type: application/octet-stream
                        },
                },
        },
+
        // Context: https://github.com/camlistore/camlistore/issues/642
        // If the file contents in the form happens to have a size such as:
        // size = peekBufferSize - (len("\n--") + len(boundary) + len("\r") + 1), (modulo peekBufferSize)
@@ -832,6 +846,52 @@ val
                },
        },
 
+       // Issue 46042; a nested multipart uses the outer separator followed by
+       // a dash.
+       {
+               name: "nested separator prefix is outer separator followed by a dash",
+               sep:  "foo",
+               in: strings.Replace(`--foo
+Content-Type: multipart/alternative; boundary="foo-bar"
+
+--foo-bar
+
+Body
+--foo-bar
+
+Body2
+--foo-bar--
+--foo--`, "\n", "\r\n", -1),
+               want: []headerBody{
+                       {textproto.MIMEHeader{"Content-Type": {`multipart/alternative; boundary="foo-bar"`}},
+                               strings.Replace(`--foo-bar
+
+Body
+--foo-bar
+
+Body2
+--foo-bar--`, "\n", "\r\n", -1),
+                       },
+               },
+       },
+
+       // A nested boundary cannot be the outer separator followed by double dash.
+       {
+               name: "nested separator prefix is outer separator followed by double dash",
+               sep:  "foo",
+               in: strings.Replace(`--foo
+Content-Type: multipart/alternative; boundary="foo--"
+
+--foo--
+
+Body
+
+--foo--`, "\n", "\r\n", -1),
+               want: []headerBody{
+                       {textproto.MIMEHeader{"Content-Type": {`multipart/alternative; boundary="foo--"`}}, ""},
+               },
+       },
+
        roundTripParseTest(),
 }