Skip to content

Commit

Permalink
fix(GODT-2573): Remove Prelude header parsing support
Browse files Browse the repository at this point in the history
The handling of Preludes in the headers stem from the MBox format.
According to the IMAP spec, the literal submitted to APPEND should be a
valid RFC822 formatted message.

The handling of the prelude was also causing issues where an MBox
formatted entry would slip through and then cause issues later.
  • Loading branch information
LBeernaertProton committed Apr 12, 2023
1 parent 7eae5cb commit 81e7559
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 33 deletions.
1 change: 1 addition & 0 deletions rfc822/header.go
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,7 @@ func EraseHeaderValue(literal []byte, key string) ([]byte, error) {
var (
ErrNonASCIIHeaderKey = fmt.Errorf("header key contains invalid characters")
ErrKeyNotFound = fmt.Errorf("invalid header key")
ErrParseHeader = fmt.Errorf("failed to parse header")
)

func mergeMultiline(line []byte) string {
Expand Down
50 changes: 44 additions & 6 deletions rfc822/header_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,47 @@ func (hp *headerParser) next() (parsedHeaderEntry, error) {
if hp.header[hp.offset] == ':' {
prevOffset := hp.offset
hp.offset++
if hp.offset < headerLen && (hp.header[hp.offset] == ' ' || hp.header[hp.offset] == '\r' || hp.header[hp.offset] == '\n') {
if hp.offset < headerLen {
result.keyEnd = prevOffset

// Validate the header key.
for i := result.keyStart; i < result.keyEnd; i++ {
if v := hp.header[i]; v < 33 || v > 126 {
return parsedHeaderEntry{}, ErrNonASCIIHeaderKey
validateHeaderField := func(h parsedHeaderEntry) error {
for i := h.keyStart; i < h.keyEnd; i++ {
if v := hp.header[i]; v < 33 || v > 126 {
return ErrNonASCIIHeaderKey
}
}

return nil
}

switch {
case hp.header[hp.offset] == ' ':
if err := validateHeaderField(result); err != nil {
return parsedHeaderEntry{}, err
}

case hp.header[hp.offset] == '\r':
// ensure next char is '\n'
hp.offset++
if hp.offset < headerLen && hp.header[hp.offset] != '\n' {
return parsedHeaderEntry{}, fmt.Errorf("expected \\n after \\r: %w", ErrParseHeader)
}
fallthrough
case hp.header[hp.offset] == '\n':
hp.offset++

if err := validateHeaderField(result); err != nil {
return parsedHeaderEntry{}, err
}

// If the next char it's not a space, it's an empty header field.
if hp.offset < headerLen && !isWSP(hp.header[hp.offset]) {
result.valueStart = result.keyEnd
result.valueEnd = result.keyEnd
return result, nil
}
default:
return parsedHeaderEntry{}, fmt.Errorf("unexpected char '%v', for header field value: %w", string(hp.header[hp.offset]), ErrParseHeader)
}

break
Expand All @@ -66,7 +99,12 @@ func (hp *headerParser) next() (parsedHeaderEntry, error) {
}

// collect value.
searchOffset := result.keyEnd + 2
searchOffset := hp.offset

for searchOffset < headerLen && isWSP(hp.header[searchOffset]) {
searchOffset++
}

result.valueStart = searchOffset

for searchOffset < headerLen {
Expand Down
56 changes: 33 additions & 23 deletions rfc822/header_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,14 +289,6 @@ func TestSplitHeaderBodyOnlyHeaderNoNewline(t *testing.T) {
assert.Equal(t, []byte(""), body)
}

func TestParseHeaderWithPrelude(t *testing.T) {
const literal = "From [email protected] Tue Aug 6 13:34:34 2002\r\nTo: [email protected]"
header, err := NewHeader([]byte(literal))
require.NoError(t, err)

assert.Equal(t, header.Get("to"), "[email protected]")
}

func TestSetHeaderValue(t *testing.T) {
const literal = "To: [email protected]"

Expand All @@ -311,21 +303,6 @@ func TestSetHeaderValue(t *testing.T) {
assert.Equal(t, literalBytes, []byte(literal))
}

func TestSetHeaderValueWithPrelude(t *testing.T) {
const literal = "From [email protected] Tue Aug 6 13:34:34 2002\r\nTo: [email protected]"

// Create a clone so we can test this with mutable memory.
literalBytes := xslices.Clone([]byte(literal))

newHeader, err := SetHeaderValue(literalBytes, "foo", "bar")
require.NoError(t, err)

assert.Equal(t, newHeader, []byte("From [email protected] Tue Aug 6 13:34:34 2002\r\nFoo: bar\r\nTo: [email protected]"))

// Ensure the original data wasn't modified.
assert.Equal(t, literalBytes, []byte(literal))
}

func TestHeader_Erase(t *testing.T) {
literal := []byte("Subject: this is\r\n\ta multiline field\r\nFrom: duplicate entry\r\nReferences:\r\n\t <[email protected]>\r\n\r\n")
literalWithoutSubject := []byte("From: duplicate entry\r\nReferences:\r\n\t <[email protected]>\r\n\r\n")
Expand Down Expand Up @@ -390,3 +367,36 @@ Content-type: multipart/mixed; boundary="simple boundary"
require.Equal(t, "1.0", header.Get("MIME-Version"))
require.Equal(t, `multipart/mixed; boundary="simple boundary"`, header.Get("Content-type"))
}

func TestHeader_MBoxFormatCausesError(t *testing.T) {
const literal = `X-Mozilla-Keys:
>From 1637354717149124322@xxx Tue Jun 25 22:52:20 +0000 2019
X-GM-THIRD: 12345
`

_, err := NewHeader([]byte(literal))
require.Error(t, err)
}

func TestHeader_EmptyField(t *testing.T) {
const literal = "X-Mozilla-Keys:\r\nX-GM-THIRD: 12345\r\n"

header, err := NewHeader([]byte(literal))
require.NoError(t, err)
require.Empty(t, header.Get("X-Mozilla-Key"))
require.Equal(t, "12345", header.Get("X-GM-THIRD"))
}

func TestHeader_MissingLFAfterCRIsError(t *testing.T) {
const literal = "X-Mozilla-Keys:\rX-GM-THIRD: 12345\r\n"

_, err := NewHeader([]byte(literal))
require.Error(t, err)
}

func TestHeader_InvalidCharAfterColonIsError(t *testing.T) {
const literal = "X-Mozilla-Keys:#X-GM-THIRD: 12345\r\n"

_, err := NewHeader([]byte(literal))
require.Error(t, err)
}
3 changes: 1 addition & 2 deletions tests/fetch_body_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -704,8 +704,7 @@ func TestFetchBodyPeekInvalidSectionNumber(t *testing.T) {
func TestFetchBody_Dovecot_InvalidMessageHeader(t *testing.T) {
// This tests fails when requesting when fetch BODY.PEEK[HEADER.FIELDS (In-Reply-To In-Reply-To Cc)].
// Instead of only returning In-Reply-To and Cc, it was also returning the References header.
const message = `From [email protected] Mon Apr 7 01:27:38 2003
Received: with ECARTIS (v1.0.0; list dovecot); Mon, 07 Apr 2003 01:27:38 +0300 (EEST)
const message = `Received: with ECARTIS (v1.0.0; list dovecot); Mon, 07 Apr 2003 01:27:38 +0300 (EEST)
Return-Path: <[email protected]>
X-Original-To: [email protected]
Delivered-To: [email protected]
Expand Down
3 changes: 1 addition & 2 deletions tests/fetch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -463,8 +463,7 @@ func TestFetchFromDataUids(t *testing.T) {
}

func TestFetchInReplyTo(t *testing.T) {
const message = `From [email protected] Tue Jul 23 19:39:23 2002
Received: with ECARTIS (v1.0.0; list dovecot); Tue, 23 Jul 2002 19:39:23 +0300 (EEST)
const message = `Received: with ECARTIS (v1.0.0; list dovecot); Tue, 23 Jul 2002 19:39:23 +0300 (EEST)
Return-Path: <[email protected]>
Delivered-To: [email protected]
Date: Tue, 23 Jul 2002 19:39:23 +0300
Expand Down

0 comments on commit 81e7559

Please sign in to comment.