code review 7300092: mime/multipart: allow unescaped newlines through in quo...

mime/multipart: allow unescaped newlines through in quoted-printable

This makes Go's quoted-printable decoder more like other
popular ones, allowing through a bare \r or \n, and also
passes through \r\n which looked like a real bug before.

Fixes issue 4771

[bradfitz, 2013-02-11 22:52:49]

Hello minux.ma@gmail.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[bradfitz, 2013-02-19 18:11:00]

Hello minux.ma@gmail.com (cc: golang-dev@googlegroups.com),

Please take another look.

[minux1, 2013-02-20 17:21:35]

https://codereview.appspot.com/7300092/diff/16001/src/pkg/mime/multipart/quot...
File src/pkg/mime/multipart/quotedprintable.go (right):

https://codereview.appspot.com/7300092/diff/16001/src/pkg/mime/multipart/quot...
src/pkg/mime/multipart/quotedprintable.go:5: // The file define a
quoted-printable decoder, as specified in RFC 2045.
we should document this:
// Deviations:
// 1. in addition to "=\r\n", "=\n" is also treated as soft line break.
// 2. it will pass "\r" and "\n" not preceded by "=" through as some
// broken encoders do this.

https://codereview.appspot.com/7300092/diff/16001/src/pkg/mime/multipart/quot...
src/pkg/mime/multipart/quotedprintable.go:85: if bytes.HasSuffix(q.line,
softSuffix) {
this will treat =\r\r\r\n as soft line break.
whatever our choice, we should document at the start of this file.

[bradfitz, 2013-02-21 00:33:15]

Hello minux.ma@gmail.com (cc: golang-dev@googlegroups.com),

Please take another look.

[bradfitz, 2013-02-21 00:39:57]

PTAL

https://codereview.appspot.com/7300092/diff/16001/src/pkg/mime/multipart/quot...
File src/pkg/mime/multipart/quotedprintable.go (right):

https://codereview.appspot.com/7300092/diff/16001/src/pkg/mime/multipart/quot...
src/pkg/mime/multipart/quotedprintable.go:5: // The file define a
quoted-printable decoder, as specified in RFC 2045.
On 2013/02/20 17:21:35, minux wrote:
> we should document this:
> // Deviations:
> // 1. in addition to "=\r\n", "=\n" is also treated as soft line break.
> // 2. it will pass "\r" and "\n" not preceded by "=" through as some
> // broken encoders do this.

Done.

https://codereview.appspot.com/7300092/diff/16001/src/pkg/mime/multipart/quot...
src/pkg/mime/multipart/quotedprintable.go:85: if bytes.HasSuffix(q.line,
softSuffix) {
On 2013/02/20 17:21:35, minux wrote:
> this will treat =\r\r\r\n as soft line break.
> whatever our choice, we should document at the start of this file.

Fixed, with new tests.

[bradfitz, 2013-02-22 07:38:27]

ping

On Wed, Feb 20, 2013 at 4:39 PM, <bradfitz@golang.org> wrote:

> PTAL
>
>
>
>
> https://codereview.appspot.**com/7300092/diff/16001/src/**
>
pkg/mime/multipart/**quotedprintable.go<https://codereview.appspot.com/7300092/diff/16001/src/pkg/mime/multipart/quotedprintable.go>
> File src/pkg/mime/multipart/**quotedprintable.go (right):
>
> https://codereview.appspot.**com/7300092/diff/16001/src/**
>
pkg/mime/multipart/**quotedprintable.go#newcode5<https://codereview.appspot.com/7300092/diff/16001/src/pkg/mime/multipart/quotedprintable.go#newcode5>
> src/pkg/mime/multipart/**quotedprintable.go:5: // The file define a
> quoted-printable decoder, as specified in RFC 2045.
> On 2013/02/20 17:21:35, minux wrote:
>
>> we should document this:
>> // Deviations:
>> // 1. in addition to "=\r\n", "=\n" is also treated as soft line
>>
> break.
>
>> // 2. it will pass "\r" and "\n" not preceded by "=" through as some
>> // broken encoders do this.
>>
>
> Done.
>
>
> https://codereview.appspot.**com/7300092/diff/16001/src/**
>
pkg/mime/multipart/**quotedprintable.go#newcode85<https://codereview.appspot.com/7300092/diff/16001/src/pkg/mime/multipart/quotedprintable.go#newcode85>
> src/pkg/mime/multipart/**quotedprintable.go:85: if bytes.HasSuffix(q.line,
> softSuffix) {
> On 2013/02/20 17:21:35, minux wrote:
>
>> this will treat =\r\r\r\n as soft line break.
>> whatever our choice, we should document at the start of this file.
>>
>
> Fixed, with new tests.
>
>
https://codereview.appspot.**com/7300092/<https://codereview.appspot.com/7300...
>

[minux1, 2013-02-22 08:29:27]

LGTM.

This is more complex than I though, but ok, we can optimize
it later if needed.

[bradfitz, 2013-02-22 18:40:27]

*** Submitted as https://code.google.com/p/go/source/detail?r=b3bb265bfecf ***

mime/multipart: allow unescaped newlines through in quoted-printable

This makes Go's quoted-printable decoder more like other
popular ones, allowing through a bare \r or \n, and also
passes through \r\n which looked like a real bug before.

Fixes issue 4771

R=minux.ma
CC=golang-dev
https://codereview.appspot.com/7300092