diff --git a/textproto/header.go b/textproto/header.go index cd582bb..4de5e27 100644 --- a/textproto/header.go +++ b/textproto/header.go @@ -518,6 +518,49 @@ func trimAroundNewlines(v []byte) string { return b.String() } +// readHeaderInitialLine rejects a header block starting with a continuation +// line (leading whitespace), which has no preceding field to fold onto. A +// nil line means the header doesn't start this way; otherwise line holds the +// offending line's raw bytes, or nil if reading it failed outright. +func readHeaderInitialLine(r *bufio.Reader) (line []byte, err error) { + buf, peekErr := r.Peek(1) + if peekErr != nil || !isSpace(buf[0]) { + return nil, nil + } + + line, err = readLineSlice(r, nil) + if err != nil { + return nil, err + } + return line, fmt.Errorf("message: malformed MIME header initial line: %v", string(line)) +} + +// parseHeaderFieldLine parses a raw header line (from readContinuedLineSlice) +// into a canonical key/value pair. key is "" for an empty field name, which +// callers should skip rather than treat as an error. +func parseHeaderFieldLine(kv []byte) (key, value string, err error) { + // Key ends at first colon; should not have trailing spaces but they + // appear in the wild, violating specs, so we remove them if present. + i := bytes.IndexByte(kv, ':') + if i < 0 { + return "", "", fmt.Errorf("message: malformed MIME header line: %v", string(kv)) + } + + keyBytes := trim(kv[:i]) + + // Verify that there are no invalid characters in the header key. + // See RFC 5322 Section 2.2 + for _, c := range keyBytes { + if !validHeaderKeyByte(c) { + return "", "", fmt.Errorf("message: malformed MIME header key: %v", string(keyBytes)) + } + } + + key = textproto.CanonicalMIMEHeaderKey(string(keyBytes)) + value = trimAroundNewlines(kv[i+1:]) + return key, value, nil +} + // ReadHeader reads a MIME header from r. The header is a sequence of possibly // continued "Key: Value" lines ending in a blank line. // @@ -527,14 +570,8 @@ func trimAroundNewlines(v []byte) string { func ReadHeader(r *bufio.Reader) (Header, error) { fs := make([]*headerField, 0, 32) - // The first line cannot start with a leading space. - if buf, err := r.Peek(1); err == nil && isSpace(buf[0]) { - line, err := readLineSlice(r, nil) - if err != nil { - return newHeader(fs), err - } - - return newHeader(fs), fmt.Errorf("message: malformed MIME header initial line: %v", string(line)) + if _, err := readHeaderInitialLine(r); err != nil { + return newHeader(fs), err } for { @@ -543,25 +580,11 @@ func ReadHeader(r *bufio.Reader) (Header, error) { return newHeader(fs), err } - // Key ends at first colon; should not have trailing spaces but they - // appear in the wild, violating specs, so we remove them if present. - i := bytes.IndexByte(kv, ':') - if i < 0 { - return newHeader(fs), fmt.Errorf("message: malformed MIME header line: %v", string(kv)) + key, value, perr := parseHeaderFieldLine(kv) + if perr != nil { + return newHeader(fs), perr } - keyBytes := trim(kv[:i]) - - // Verify that there are no invalid characters in the header key. - // See RFC 5322 Section 2.2 - for _, c := range keyBytes { - if !validHeaderKeyByte(c) { - return newHeader(fs), fmt.Errorf("message: malformed MIME header key: %v", string(keyBytes)) - } - } - - key := textproto.CanonicalMIMEHeaderKey(string(keyBytes)) - // As per RFC 7230 field-name is a token, tokens consist of one or more // chars. We could return a an error here, but better to be liberal in // what we accept, so if we get an empty key, skip it. @@ -569,10 +592,6 @@ func ReadHeader(r *bufio.Reader) (Header, error) { continue } - i++ // skip colon - v := kv[i:] - - value := trimAroundNewlines(v) fs = append(fs, newHeaderField(key, value, kv)) if err != nil { diff --git a/textproto/multipart.go b/textproto/multipart.go index 5bfe375..5df50aa 100644 --- a/textproto/multipart.go +++ b/textproto/multipart.go @@ -97,42 +97,112 @@ func IsMalformedPartHeader(err error) bool { func newPart(mr *MultipartReader) (*Part, error) { bp := &Part{mr: mr} - if err := bp.populateHeaders(); err != nil { - // The header block contained at least one unparseable line. ReadHeader - // consumed that line and stopped, so the reader is now positioned at - // the next line. Try once more: if the remaining lines form a valid - // header block (e.g. a single bad line preceded otherwise-valid headers) - // we can still deliver the part to the caller. - if recoveryErr := bp.populateHeaders(); recoveryErr == nil { - // Recovered: part != nil signals the headers are usable. - bp.r = partReader{bp} - return bp, &MalformedPartHeaderError{Err: err} - } - // Recovery failed. Discard through the part boundary so the reader is - // left in a valid state for the next NextPart call. + + // If there are consecutive boundaries, just return an empty header. + peek, _ := bp.mr.bufReader.Peek(len(bp.mr.dashBoundary)) + if bytes.HasPrefix(peek, bp.mr.dashBoundary) { + bp.Header = Header{} + bp.r = partReader{bp} + return bp, nil + } + + header, leftover, err := readPartHeader(bp.mr.bufReader) + bp.Header = header + switch { + case err == nil: + bp.r = partReader{bp} + return bp, nil + case leftover != nil: + // A recovered multipart Content-Type's boundary is still usable: + // folding leftover into the body lets NextPart's own preamble-skip + // (partsRead == 0) find the real parts past the garbage. + bp.r = io.MultiReader(bytes.NewReader(leftover), partReader{bp}) + return bp, err + case IsMalformedPartHeader(err): + // A single bad line, recovered cleanly once the rest of the header + // parsed normally. + bp.r = partReader{bp} + return bp, err + default: + // A genuine reader-level error (never a malformed-line condition: + // every such case is a MalformedPartHeaderError caught above). + // Discard through the part boundary so the reader is left in a + // valid state for the next NextPart call. discard := &Part{mr: mr} discard.r = partReader{discard} io.Copy(io.Discard, discard) - // part == nil signals the part was unrecoverable. - return nil, &MalformedPartHeaderError{Err: err} + return nil, err } - bp.r = partReader{bp} - return bp, nil } -func (bp *Part) populateHeaders() error { - // If there are consecutive boundaries, just return an empty header. - peek, _ := bp.mr.bufReader.Peek(len(bp.mr.dashBoundary)) - if bytes.HasPrefix(peek, bp.mr.dashBoundary) { - bp.Header = Header{} - return nil +// readPartHeader reads a part's header, tolerating up to one malformed line. +// A second malformed line rolls the header back to the fields parsed before +// the first failure; leftover then holds everything consumed from that point +// onward, for the caller to use instead of losing it. +func readPartHeader(r *bufio.Reader) (h Header, leftover []byte, err error) { + fs := make([]*headerField, 0, 32) + var raw []byte + + var badLines int + var firstErr error + var checkpointFields int + + // giveUp reports whether a second malformed line has now been seen. The + // error is formatted lazily since only the first occurrence is kept. + giveUp := func(cf int, format string, args ...any) bool { + badLines++ + if badLines == 1 { + firstErr = fmt.Errorf(format, args...) + checkpointFields = cf + return false + } + return true } - header, err := ReadHeader(bp.mr.bufReader) - if err == nil { - bp.Header = header + if line, err := readHeaderInitialLine(r); err != nil { + if line == nil { + return newHeader(fs), nil, err + } + // readLineSlice strips the line ending; restore it so this doesn't + // run into a following line (e.g. a nested boundary needing its own). + raw = append(raw, line...) + raw = append(raw, '\r', '\n') + badLines, firstErr = 1, err + } + + for { + kv, rerr := readContinuedLineSlice(r) + if len(kv) == 0 { + // rerr is only non-nil here on a genuine read failure, never a + // normal blank line or clean EOF, so it takes priority. + if rerr == nil && badLines > 0 { + return newHeader(fs), nil, &MalformedPartHeaderError{Err: firstErr} + } + return newHeader(fs), nil, rerr + } + + key, value, perr := parseHeaderFieldLine(kv) + if perr != nil { + // raw only needs bytes from the first bad line onward: nothing + // before checkpointFields is ever part of leftover. + raw = append(raw, kv...) + if giveUp(len(fs), "%w", perr) { + return newHeader(fs[:checkpointFields]), raw, &MalformedPartHeaderError{Err: firstErr} + } + continue + } + if badLines > 0 { + raw = append(raw, kv...) + } + if key == "" { + continue + } + fs = append(fs, newHeaderField(key, value, kv)) + + if rerr != nil { + return newHeader(fs), nil, rerr + } } - return err } // Read reads the body of a part, after its headers and before the diff --git a/textproto/multipart_test.go b/textproto/multipart_test.go index 1d4dd46..b347ea2 100644 --- a/textproto/multipart_test.go +++ b/textproto/multipart_test.go @@ -7,6 +7,7 @@ package textproto import ( "bytes" "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -993,13 +994,21 @@ func TestMalformedPartHeaderSkip(t *testing.T) { r := NewMultipartReader(strings.NewReader(body), "sep") - // Part 1: unrecoverable – two bad lines – returns nil part + error. + // Part 1: two bad lines, nothing structural recovered -- still returned + // (empty Header), with the raw content as its body rather than discarded. p, err := r.NextPart() if !IsMalformedPartHeader(err) { t.Fatalf("part 1 NextPart: expected IsMalformedPartHeader error, got %v", err) } - if p != nil { - t.Fatal("part 1 NextPart: expected nil part when recovery fails") + if p == nil { + t.Fatal("part 1 NextPart: expected non-nil part with the unparsed content as its body") + } + if got := p.Header.Get("Content-Type"); got != "" { + t.Errorf("part 1 Content-Type: got %q, want empty", got) + } + b, _ := ioutil.ReadAll(p) + if got, want := string(b), "first bad line\r\nsecond bad line\r\n\r\nskipped body"; got != want { + t.Errorf("part 1 body: got %q, want %q", got, want) } // Part 2: valid part is reachable after the skip. @@ -1010,7 +1019,7 @@ func TestMalformedPartHeaderSkip(t *testing.T) { if got, want := p.Header.Get("Content-Type"), "text/plain"; got != want { t.Errorf("part 2 Content-Type: got %q, want %q", got, want) } - b, _ := ioutil.ReadAll(p) + b, _ = ioutil.ReadAll(p) if got, want := string(b), "good body"; got != want { t.Errorf("part 2 body: got %q, want %q", got, want) } @@ -1021,6 +1030,132 @@ func TestMalformedPartHeaderSkip(t *testing.T) { } } +// TestMalformedPartHeaderPreservesSubBoundary tests that when a part's own +// Content-Type (declaring a nested multipart boundary) is followed by two +// bad lines with no recovery, that boundary isn't thrown away along with the +// garbage: the nested part reachable through it must still parse normally. +func TestMalformedPartHeaderPreservesSubBoundary(t *testing.T) { + body := "--outer\r\n" + + "Content-Type: multipart/mixed; boundary=inner\r\n" + + "bad line one\r\n" + + "bad line two\r\n" + + "--inner\r\n" + + "Content-Type: text/plain\r\n" + + "\r\n" + + "real content\r\n" + + "--inner--\r\n" + + "--outer--\r\n" + + r := NewMultipartReader(strings.NewReader(body), "outer") + + p, err := r.NextPart() + if !IsMalformedPartHeader(err) { + t.Fatalf("NextPart: expected IsMalformedPartHeader error, got %v", err) + } + if p == nil { + t.Fatal("NextPart: expected non-nil part with the recovered Content-Type") + } + if got, want := p.Header.Get("Content-Type"), "multipart/mixed; boundary=inner"; got != want { + t.Errorf("Content-Type: got %q, want %q", got, want) + } + + inner := NewMultipartReader(p, "inner") + ip, err := inner.NextPart() + if err != nil { + t.Fatalf("inner NextPart: %v", err) + } + if got, want := ip.Header.Get("Content-Type"), "text/plain"; got != want { + t.Errorf("inner Content-Type: got %q, want %q", got, want) + } + b, _ := ioutil.ReadAll(ip) + if got, want := string(b), "real content"; got != want { + t.Errorf("inner body: got %q, want %q", got, want) + } + + if _, err := inner.NextPart(); err != io.EOF { + t.Fatalf("expected io.EOF after inner final boundary, got %v", err) + } +} + +// errAfterN returns want, followed by errAfter once want is exhausted. +type errAfterN struct { + want *strings.Reader + errAfter error +} + +func (e *errAfterN) Read(p []byte) (int, error) { + if e.want.Len() == 0 { + return 0, e.errAfter + } + return e.want.Read(p) +} + +// TestMalformedPartHeaderIOErrorAfterRecovery tests that a genuine read error +// after one tolerated bad line is reported, not mistaken for clean recovery. +func TestMalformedPartHeaderIOErrorAfterRecovery(t *testing.T) { + boomErr := errors.New("boom: simulated transport failure") + // The last line ends cleanly (trailing CRLF, no partial content left + // over), so the read failure surfaces on the *next* attempt to find a + // line -- with no bytes read at all, not attached to any line's content. + body := "--sep\r\n" + + "bad line one\r\n" + + "Content-Type: text/plain\r\n" + + r := NewMultipartReader(&errAfterN{want: strings.NewReader(body), errAfter: boomErr}, "sep") + + p, err := r.NextPart() + if p != nil { + t.Fatalf("NextPart: expected nil part on a genuine I/O error, got %v", p) + } + if !errors.Is(err, boomErr) { + t.Fatalf("NextPart: expected the underlying I/O error to surface, got %v", err) + } + if IsMalformedPartHeader(err) { + t.Fatalf("NextPart: expected a plain I/O error, not IsMalformedPartHeader: %v", err) + } +} + +// TestMalformedPartHeaderInitialLinePreservesLineEnding tests that a bad +// first line (starting with a space) doesn't run straight into the next +// line once recovered. +func TestMalformedPartHeaderInitialLinePreservesLineEnding(t *testing.T) { + body := "--sep\r\n" + + " initial bad line\r\n" + + "second bad line\r\n" + + "--sep--\r\n" + + r := NewMultipartReader(strings.NewReader(body), "sep") + p, err := r.NextPart() + if !IsMalformedPartHeader(err) || p == nil { + t.Fatalf("NextPart: expected a recovered part with IsMalformedPartHeader, got p=%v err=%v", p, err) + } + b, _ := ioutil.ReadAll(p) + if got, want := string(b), " initial bad line\r\nsecond bad line\r\n"; got != want { + t.Errorf("body: got %q, want %q", got, want) + } +} + +// TestMalformedPartHeaderPreservesEmptyKeyLine tests that a colon-only line +// (empty field name) occurring after the first bad line isn't silently +// dropped from the recovered body if a second bad line forces a rollback. +func TestMalformedPartHeaderPreservesEmptyKeyLine(t *testing.T) { + body := "--sep\r\n" + + "bad line one\r\n" + + ": emptykey\r\n" + + "second bad line\r\n" + + "--sep--\r\n" + + r := NewMultipartReader(strings.NewReader(body), "sep") + p, err := r.NextPart() + if !IsMalformedPartHeader(err) || p == nil { + t.Fatalf("NextPart: expected a recovered part with IsMalformedPartHeader, got p=%v err=%v", p, err) + } + b, _ := ioutil.ReadAll(p) + if got, want := string(b), "bad line one\r\n: emptykey\r\nsecond bad line\r\n"; got != want { + t.Errorf("body: got %q, want %q", got, want) + } +} + // TestMatchAfterPrefix exercises all return paths of matchAfterPrefix, including // the new single-hyphen lookahead added to distinguish --boundary-- from --boundary-X. func TestMatchAfterPrefix(t *testing.T) {