Commit 821b5492 authored by mpl's avatar mpl Committed by Brad Fitzpatrick

mime/multipart: fix peekBufferSeparatorIndex edge case

The case fixed by this change happens when, in func (pr partReader)
Read, the Peek happens to read so that peek looks like:

  "somedata\r\n--Boundary\r"

peekBufferSeparatorIndex was returning (-1, false) because it didn't
find the trailing '\n'.

This was wrong because:

1) It didn't match the documentation: as "\r\n--Boundary" was found, it
should return the index of that pattern, not -1.

2) It lead to an nCopy cut such as:
  "somedata\r| |\n--Boundary\r" instead of "somedata| |\r\n--Boundary\r"
which made the subsequent Read miss the boundary, and eventually end
with a "return 0, io.ErrUnexpectedEOF" case, as reported in:

https://github.com/camlistore/camlistore/issues/642

Change-Id: I1ba78a741bc0c7719e160add9cca932d10f8a615
Reviewed-on: https://go-review.googlesource.com/15269Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
parent b4377319
......@@ -366,7 +366,7 @@ func (mr *Reader) peekBufferSeparatorIndex(peek []byte) (idx int, isEnd bool) {
peek = skipLWSPChar(peek)
// Don't have a complete line after the peek.
if bytes.IndexByte(peek, '\n') == -1 {
return -1, false
return idx, false
}
if len(peek) > 0 && peek[0] == '\n' {
return idx, true
......
......@@ -664,6 +664,38 @@ 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)
// then peekBufferSeparatorIndex was wrongly returning (-1, false), which was leading to an nCopy
// cut such as:
// "somedata\r| |\n--Boundary\r" (instead of "somedata| |\r\n--Boundary\r"), which was making the
// subsequent Read miss the boundary.
{
name: "safeCount off by one",
sep: "08b84578eabc563dcba967a945cdf0d9f613864a8f4a716f0e81caa71a74",
in: strings.Replace(`--08b84578eabc563dcba967a945cdf0d9f613864a8f4a716f0e81caa71a74
Content-Disposition: form-data; name="myfile"; filename="my-file.txt"
Content-Type: application/octet-stream
`, "\n", "\r\n", -1) +
strings.Repeat("A", peekBufferSize-(len("\n--")+len("08b84578eabc563dcba967a945cdf0d9f613864a8f4a716f0e81caa71a74")+len("\r")+1)) +
strings.Replace(`
--08b84578eabc563dcba967a945cdf0d9f613864a8f4a716f0e81caa71a74
Content-Disposition: form-data; name="key"
val
--08b84578eabc563dcba967a945cdf0d9f613864a8f4a716f0e81caa71a74--
`, "\n", "\r\n", -1),
want: []headerBody{
{textproto.MIMEHeader{"Content-Type": {`application/octet-stream`}, "Content-Disposition": {`form-data; name="myfile"; filename="my-file.txt"`}},
strings.Repeat("A", peekBufferSize-(len("\n--")+len("08b84578eabc563dcba967a945cdf0d9f613864a8f4a716f0e81caa71a74")+len("\r")+1)),
},
{textproto.MIMEHeader{"Content-Disposition": {`form-data; name="key"`}},
"val",
},
},
},
roundTripParseTest(),
}
......@@ -704,6 +736,53 @@ Cases:
}
}
func partsFromReader(r *Reader) ([]headerBody, error) {
got := []headerBody{}
for {
p, err := r.NextPart()
if err == io.EOF {
return got, nil
}
if err != nil {
return nil, fmt.Errorf("NextPart: %v", err)
}
pbody, err := ioutil.ReadAll(p)
if err != nil {
return nil, fmt.Errorf("error reading part: %v", err)
}
got = append(got, headerBody{p.Header, string(pbody)})
}
}
func TestParseAllSizes(t *testing.T) {
const maxSize = 5 << 10
var buf bytes.Buffer
body := strings.Repeat("a", maxSize)
bodyb := []byte(body)
for size := 0; size < maxSize; size++ {
buf.Reset()
w := NewWriter(&buf)
part, _ := w.CreateFormField("f")
part.Write(bodyb[:size])
part, _ = w.CreateFormField("key")
part.Write([]byte("val"))
w.Close()
r := NewReader(&buf, w.Boundary())
got, err := partsFromReader(r)
if err != nil {
t.Errorf("For size %d: %v", size, err)
continue
}
if len(got) != 2 {
t.Errorf("For size %d, num parts = %d; want 2", size, len(got))
continue
}
if got[0].body != body[:size] {
t.Errorf("For size %d, got unexpected len %d: %q", size, len(got[0].body), got[0].body)
}
}
}
func roundTripParseTest() parseTest {
t := parseTest{
name: "round trip",
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment