Recover a multipart part's boundary past a run of malformed header lines

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,36 +580,18 @@ 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.
 		if key == "" {
 			continue
 		}
 
-		i++ // skip colon
-		v := kv[i:]
-
-		value := trimAroundNewlines(v)
 		fs = append(fs, newHeaderField(key, value, kv))
 
 		if err != nil {

textproto/multipart.go

@@ -97,42 +97,110 @@ 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
+		}
+		raw = append(raw, line...)
+		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 key == "" {
+			continue
+		}
+
+		if badLines > 0 {
+			raw = append(raw, kv...)
+		}
+		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

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,91 @@ 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)
+	}
+}
+
 // 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) {