Skip to content

Commit

Permalink
Fix incorrect line folding of RFC2047-encoded strings
Browse files Browse the repository at this point in the history
Use only whitespace characters as a separator to fold lines according to
the section 2.2.3 of RFC5322. It may increase number of cases where hard limit
fallback is used, but it should prevent damaging encoded subjects.
  • Loading branch information
dvalter authored and emersion committed Apr 15, 2020
1 parent 9c4415e commit fee642d
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 24 deletions.
24 changes: 2 additions & 22 deletions textproto/header.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"fmt"
"io"
"net/textproto"
"regexp"
"strings"
)

Expand Down Expand Up @@ -494,9 +493,6 @@ func ReadHeader(r *bufio.Reader) (Header, error) {
}
}

// Regexp that detects Quoted Printable (QP) characters
var qpReg = regexp.MustCompile("(=[0-9A-Z]{2,2})+")

func foldLine(v string, maxlen int) (line, next string, ok bool) {
ok = true

Expand All @@ -512,24 +508,8 @@ func foldLine(v string, maxlen int) (line, next string, ok bool) {
folding = "\r\n"
}
} else {
// Find the last QP character before limit
foldAtQP := qpReg.FindAllStringIndex(v[:foldBefore], -1)
// Find the closest whitespace before maxlen
foldAtEOL := strings.LastIndexAny(v[:foldBefore], " \t\n")

// Fold at the latest whitespace by default
foldAt = foldAtEOL

// if there are QP characters in the string
if len(foldAtQP) > 0 {
// Get the start index of the last QP character
foldAtQPLastIndex := foldAtQP[len(foldAtQP)-1][0]
if foldAtQPLastIndex > foldAt {
// Fold at the latest QP character if there are no whitespaces
// after it and before line length limit
foldAt = foldAtQPLastIndex
}
}
foldAt = strings.LastIndexAny(v[:foldBefore], " \t\n")

if foldAt == 0 {
// The whitespace we found was the previous folding WSP
Expand All @@ -551,7 +531,7 @@ func foldLine(v string, maxlen int) (line, next string, ok bool) {
// extra space in the string, so this should be avoided if
// possible.
folding = "\r\n "
ok = len(foldAtQP) > 0
ok = false
}
}

Expand Down
10 changes: 8 additions & 2 deletions textproto/header_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -404,18 +404,24 @@ var formatHeaderFieldTests = []struct {
{
k: "Subject",
v: "=?utf-8?q?=E2=80=9CDeveloper_reads_customer_requested_change.=E2=80=9D=0A?= =?utf-8?q?=0ACaravaggio=0A=0AOil_on...?=",
formatted: "Subject: =?utf-8?q?=E2=80=9CDeveloper_reads_customer_requested_change.\r\n =E2=80=9D=0A?= =?utf-8?q?=0ACaravaggio=0A=0AOil_on...?=\r\n",
formatted: "Subject: =?utf-8?q?=E2=80=9CDeveloper_reads_customer_requested_change.=E2=80=9D=0A?= =?utf-8?q?=0ACaravaggio=0A=0AOil_on...?=\r\n",
},
// Spaces should not appear in RFC2047-encoded string but it should be fine in case they do
{
k: "Subject",
v: "=?utf-8?q?=E2=80=9CShort subject=E2=80=9D=0A?= =?utf-8?q?=0AAuthor=0A=0AOil_on...?=",
formatted: "Subject: =?utf-8?q?=E2=80=9CShort subject=E2=80=9D=0A?= =?utf-8?q?\r\n =0AAuthor=0A=0AOil_on...?=\r\n",
formatted: "Subject: =?utf-8?q?=E2=80=9CShort subject=E2=80=9D=0A?=\r\n =?utf-8?q?=0AAuthor=0A=0AOil_on...?=\r\n",
},
{
k: "Subject",
v: "=?utf-8?q?=E2=80=9CVery long subject very long subject very long subject very long subject=E2=80=9D=0A?= =?utf-8?q?=0ALong second part of subject long second part of subject long second part of subject long subject=0A=0AOil_on...?=",
formatted: "Subject: =?utf-8?q?=E2=80=9CVery long subject very long subject very long\r\n subject very long subject=E2=80=9D=0A?= =?utf-8?q?=0ALong second part of\r\n subject long second part of subject long second part of subject long\r\n subject=0A=0AOil_on...?=\r\n",
},
{
k: "Subject",
v: "InCaseOfVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongStringWeStillShouldComplyToTheHardLimitOf998Symbols",
formatted: "Subject: InCaseOfVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongStringWeStillSho\r\n uldComplyToTheHardLimitOf998Symbols\r\n",
},
{
k: "DKIM-Signature",
v: "v=1;\r\n h=From:To:Reply-To:Subject:Message-ID:References:In-Reply-To:MIME-Version;\r\n d=example.org\r\n",
Expand Down

0 comments on commit fee642d

Please sign in to comment.