diff --git a/message/writer.go b/message/writer.go index 021150f..a75ac38 100644 --- a/message/writer.go +++ b/message/writer.go @@ -13,7 +13,9 @@ type Writer struct { Has8bit bool // Whether a byte with the high/8bit has been read. So whether this needs SMTP 8BITMIME instead of 7BIT. Size int64 // Number of bytes written, may be different from bytes read due to LF to CRLF conversion. - tail [3]byte // For detecting header/body-separating crlf. + // For detecting header/body-separating crlf and fixing up bare lf. These are the + // incoming bytes, not the fixed up bytes. So CRs may be missing from tail. + tail [3]byte // todo: should be parsing headers here, as we go } @@ -26,9 +28,16 @@ func NewWriter(w io.Writer) *Writer { // io.Writer. It converts bare new lines (LF) to carriage returns with new lines // (CRLF). func (w *Writer) Write(buf []byte) (int, error) { - origtail := w.tail + if !w.Has8bit { + for _, b := range buf { + if b >= 0x80 { + w.Has8bit = true + break + } + } + } - if !w.HaveBody && len(buf) > 0 { + if !w.HaveBody { get := func(i int) byte { if i < 0 { return w.tail[3+i] @@ -42,29 +51,27 @@ func (w *Writer) Write(buf []byte) (int, error) { break } } + } + // Update w.tail after having written. Regardless of error, writers can't expect + // subsequent writes to work again properly anyway. + defer func() { n := len(buf) if n > 3 { n = 3 } copy(w.tail[:], w.tail[n:]) copy(w.tail[3-n:], buf[len(buf)-n:]) - } - if !w.Has8bit { - for _, b := range buf { - if b&0x80 != 0 { - w.Has8bit = true - break - } - } - } + }() wrote := 0 o := 0 Top: for o < len(buf) { + // Look for bare newline. If present, write up to that position while adding the + // missing carriage return. Then start the loop again. for i := o; i < len(buf); i++ { - if buf[i] == '\n' && (i > 0 && buf[i-1] != '\r' || i == 0 && origtail[2] != '\r') { + if buf[i] == '\n' && (i > 0 && buf[i-1] != '\r' || i == 0 && w.tail[2] != '\r') { // Write buffer leading up to missing \r. if i > o { n, err := w.writer.Write(buf[o:i]) diff --git a/message/writer_test.go b/message/writer_test.go index ff18b49..4f10f5e 100644 --- a/message/writer_test.go +++ b/message/writer_test.go @@ -53,3 +53,29 @@ func TestMsgWriter(t *testing.T) { t.Fatalf("got %q, expected %q", got, exp) } } + +func TestMsgWriterIssue117(t *testing.T) { + var b strings.Builder + mw := NewWriter(&b) + + // Write header and header/body separator, but with CR missing. + _, err := mw.Write([]byte("a: b\n\n")) + tcheck(t, err, "write") + + // Write start of a line. The newline follows in a second write. Just because of buffering. + _, err = mw.Write([]byte("body\r")) + tcheck(t, err, "write") + + // Finish the line. The bug is that w.tail was only updated while writing headers, + // not while writing message data for the body. That makes the code think this \n + // wasn't preceded by a \r, causing it to add a \r. And we end up with \r\r\n in + // the file. + _, err = mw.Write([]byte("\n")) + tcheck(t, err, "write") + + got := b.String() + exp := "a: b\r\n\r\nbody\r\n" + if got != exp { + t.Fatalf("got %q, expected %q", got, exp) + } +}