GODT-1263: Fix crash on invalid or empty header

pkg/message/build_rfc822.go

@@ -347,6 +347,12 @@ func writeMultipartEncryptedRFC822(header message.Header, body []byte) ([]byte,
 		return nil, err
 	}
 
+	// If parsed header is empty then either it is malformed or it is missing.
+	// Anyway message could not be considered multipart/mixed anymore since there will be no boundary.
+	if bodyHeader.Len() == 0 {
+		header.Del("Content-Type")
+	}
+
 	entFields := bodyHeader.Fields()
 
 	for entFields.Next() {

pkg/message/build_test.go

@@ -120,6 +120,126 @@ func TestBuildPlainEncryptedMessage(t *testing.T) {
 		expectBody(contains(`Where do fruits go on vacation? Pear-is!`))
 }
 
+func TestBuildPlainEncryptedMessageMissingHeader(t *testing.T) {
+	m := gomock.NewController(t)
+	defer m.Finish()
+
+	b := NewBuilder(1, 1, 1)
+	defer b.Done()
+
+	body := readerToString(getFileReader("plaintext-missing-header.eml"))
+
+	kr := tests.MakeKeyRing(t)
+	msg := newTestMessage(t, kr, "messageID", "addressID", "multipart/mixed", body, time.Now())
+
+	res, err := b.NewJob(context.Background(), newTestFetcher(m, kr, msg), msg.ID).GetResult()
+	require.NoError(t, err)
+
+	section(t, res).
+		expectContentType(is(`text/plain`)).
+		expectBody(is("How do we know that the ocean is friendly? It waves!\r\n"))
+}
+
+func TestBuildPlainEncryptedMessageInvalidHeader(t *testing.T) {
+	m := gomock.NewController(t)
+	defer m.Finish()
+
+	b := NewBuilder(1, 1, 1)
+	defer b.Done()
+
+	body := readerToString(getFileReader("plaintext-invalid-header.eml"))
+
+	kr := tests.MakeKeyRing(t)
+	msg := newTestMessage(t, kr, "messageID", "addressID", "multipart/mixed", body, time.Now())
+
+	res, err := b.NewJob(context.Background(), newTestFetcher(m, kr, msg), msg.ID).GetResult()
+	require.NoError(t, err)
+
+	section(t, res).
+		expectContentType(is(`text/plain`)).
+		expectBody(is("MalformedKey Value\r\n\r\nHow do we know that the ocean is friendly? It waves!\r\n"))
+}
+
+func TestBuildPlainSignedEncryptedMessageMissingHeader(t *testing.T) {
+	m := gomock.NewController(t)
+	defer m.Finish()
+
+	b := NewBuilder(1, 1, 1)
+	defer b.Done()
+
+	body := readerToString(getFileReader("plaintext-missing-header.eml"))
+
+	kr := tests.MakeKeyRing(t)
+	sig := tests.MakeKeyRing(t)
+
+	enc, err := kr.Encrypt(crypto.NewPlainMessageFromString(body), sig)
+	require.NoError(t, err)
+
+	arm, err := enc.GetArmored()
+	require.NoError(t, err)
+
+	msg := newRawTestMessage("messageID", "addressID", "multipart/mixed", arm, time.Date(2020, time.January, 1, 0, 0, 0, 0, time.UTC))
+
+	res, err := b.NewJob(context.Background(), newTestFetcher(m, kr, msg), msg.ID).GetResult()
+	require.NoError(t, err)
+
+	section(t, res).
+		expectContentType(is(`multipart/signed`)).
+		expectContentTypeParam(`micalg`, is(`SHA-256`)). // NOTE: Maybe this is bad... should probably be pgp-sha256
+		expectContentTypeParam(`protocol`, is(`application/pgp-signature`)).
+		expectDate(is(`Wed, 01 Jan 2020 00:00:00 +0000`))
+
+	section(t, res, 1).
+		expectContentType(is(`text/plain`)).
+		expectBody(is("How do we know that the ocean is friendly? It waves!\r\n"))
+
+	section(t, res, 2).
+		expectContentType(is(`application/pgp-signature`)).
+		expectContentTypeParam(`name`, is(`OpenPGP_signature.asc`)).
+		expectContentDisposition(is(`attachment`)).
+		expectContentDispositionParam(`filename`, is(`OpenPGP_signature`))
+}
+
+func TestBuildPlainSignedEncryptedMessageInvalidHeader(t *testing.T) {
+	m := gomock.NewController(t)
+	defer m.Finish()
+
+	b := NewBuilder(1, 1, 1)
+	defer b.Done()
+
+	body := readerToString(getFileReader("plaintext-invalid-header.eml"))
+
+	kr := tests.MakeKeyRing(t)
+	sig := tests.MakeKeyRing(t)
+
+	enc, err := kr.Encrypt(crypto.NewPlainMessageFromString(body), sig)
+	require.NoError(t, err)
+
+	arm, err := enc.GetArmored()
+	require.NoError(t, err)
+
+	msg := newRawTestMessage("messageID", "addressID", "multipart/mixed", arm, time.Date(2020, time.January, 1, 0, 0, 0, 0, time.UTC))
+
+	res, err := b.NewJob(context.Background(), newTestFetcher(m, kr, msg), msg.ID).GetResult()
+	require.NoError(t, err)
+
+	section(t, res).
+		expectContentType(is(`multipart/signed`)).
+		expectContentTypeParam(`micalg`, is(`SHA-256`)). // NOTE: Maybe this is bad... should probably be pgp-sha256
+		expectContentTypeParam(`protocol`, is(`application/pgp-signature`)).
+		expectDate(is(`Wed, 01 Jan 2020 00:00:00 +0000`))
+
+	section(t, res, 1).
+		expectContentType(is(`text/plain`)).
+		expectBody(is("MalformedKey Value\r\n\r\nHow do we know that the ocean is friendly? It waves!\r\n"))
+
+	section(t, res, 2).
+		expectContentType(is(`application/pgp-signature`)).
+		expectContentTypeParam(`name`, is(`OpenPGP_signature.asc`)).
+		expectContentDisposition(is(`attachment`)).
+		expectContentDispositionParam(`filename`, is(`OpenPGP_signature`))
+}
+
 func TestBuildPlainEncryptedLatin2Message(t *testing.T) {
 	m := gomock.NewController(t)
 	defer m.Finish()

pkg/message/header.go

@@ -22,6 +22,7 @@ import (
 	"bytes"
 	"io"
 	"io/ioutil"
+	"unicode"
 
 	"github.com/emersion/go-message/textproto"
 	"github.com/pkg/errors"
@@ -37,8 +38,7 @@ func HeaderLines(header []byte) [][]byte {
 	forEachLine(bufio.NewReader(bytes.NewReader(header)), func(line []byte) {
 		l := bytes.SplitN(line, []byte(`: `), 2)
 		isLineContinuation := quote%2 != 0 || // no quotes opened
-			len(l) != 2 || // it doesn't have colon
+			!bytes.Equal(bytes.TrimLeftFunc(l[0], unicode.IsSpace), l[0]) // has whitespace indent at beginning
-			(len(l) == 2 && !bytes.Equal(bytes.TrimSpace(l[0]), l[0])) // has white space in front of header field
 		switch {
 		case len(bytes.TrimSpace(line)) == 0:
 			lines = append(lines, line)
@@ -89,6 +89,12 @@ func readHeaderBody(b []byte) (*textproto.Header, []byte, error) {
 
 	var header textproto.Header
 
+	// We assume that everything before first occurrence of empty line is header.
+	// If header is invalid for any reason or empty - put everything as body and let header be empty.
+	if !isHeaderValid(lines) {
+		return &header, b, nil
+	}
+
 	// We add lines in reverse so that calling textproto.WriteHeader later writes with the correct order.
 	for i := len(lines) - 1; i >= 0; i-- {
 		if len(bytes.TrimSpace(lines[i])) > 0 {
@@ -99,6 +105,20 @@ func readHeaderBody(b []byte) (*textproto.Header, []byte, error) {
 	return &header, body, nil
 }
 
+func isHeaderValid(headerLines [][]byte) bool {
+	if len(headerLines) == 0 {
+		return false
+	}
+
+	for _, line := range headerLines {
+		if (bytes.IndexByte(line, ':') == -1) && (len(bytes.TrimSpace(line)) > 0) {
+			return false
+		}
+	}
+
+	return true
+}
+
 func splitHeaderBody(b []byte) ([]byte, []byte, error) {
 	br := bufio.NewReader(bytes.NewReader(b))
 

pkg/message/header_test.go

@@ -79,3 +79,34 @@ Content-ID: <>
 		[]byte("Content-ID: <>\n"),
 	}, HeaderLines([]byte(header)))
 }
+
+func TestReadHeaderBody(t *testing.T) {
+	const data = "key: value\r\n\r\nbody\n"
+
+	header, body, err := readHeaderBody([]byte(data))
+
+	assert.NoError(t, err)
+	assert.Equal(t, 1, header.Len())
+	assert.Equal(t, "value", header.Get("key"))
+	assert.Equal(t, []byte("body\n"), body)
+}
+
+func TestReadHeaderBodyWithoutHeader(t *testing.T) {
+	const data = "body\n"
+
+	header, body, err := readHeaderBody([]byte(data))
+
+	assert.NoError(t, err)
+	assert.Equal(t, 0, header.Len())
+	assert.Equal(t, []byte(data), body)
+}
+
+func TestReadHeaderBodyInvalidHeader(t *testing.T) {
+	const data = "value\r\n\r\nbody\n"
+
+	header, body, err := readHeaderBody([]byte(data))
+
+	assert.NoError(t, err)
+	assert.Equal(t, 0, header.Len())
+	assert.Equal(t, []byte(data), body)
+}

pkg/message/testdata/plaintext-invalid-header.eml

@@ -0,0 +1,3 @@
+MalformedKey Value
+
+How do we know that the ocean is friendly? It waves!

pkg/message/testdata/plaintext-missing-header.eml

@@ -0,0 +1 @@
+How do we know that the ocean is friendly? It waves!