code review 12689045: mime: now properly escape backslash in attribute values

mime: now properly escape backslash in attribute values

[peted, 2013-08-09 19:39:54]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[bradfitz, 2013-08-09 20:09:34]

LGTM

But your commit message could be simplified:

> mime: now properly escape backslash in attribute values

Both "now" and "properly" are redundant: now is implicit with the time of
the CL & its commit.  And we never do anything improperly on purpose, so
saying properly is also redundant.

mime: escape backslash on attribute values.

I trust that's proper.  If not, we'll hear about it later.




On Fri, Aug 9, 2013 at 12:39 PM, <pieter@binky.org.uk> wrote:

> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> mime: now properly escape backslash in attribute values
>
> Please review this at
https://codereview.appspot.**com/12689045/<https://codereview.appspot.com/126...
>
> Affected files:
>   M src/pkg/mime/mediatype.go
>   M src/pkg/mime/mediatype_test.go
>
>
> Index: src/pkg/mime/mediatype.go
> ==============================**==============================**=======
> --- a/src/pkg/mime/mediatype.go
> +++ b/src/pkg/mime/mediatype.go
> @@ -47,7 +47,7 @@
>                 b.WriteByte('"')
>                 offset := 0
>                 for index, character := range value {
> -                       if character == '"' || character == '\r' {
> +                       if character == '"' || character == '\\' {
>                                 b.WriteString(value[offset:**index])
>                                 offset = index
>                                 b.WriteByte('\\')
> Index: src/pkg/mime/mediatype_test.go
> ==============================**==============================**=======
> --- a/src/pkg/mime/mediatype_test.**go
> +++ b/src/pkg/mime/mediatype_test.**go
> @@ -282,8 +282,17 @@
>
>  var formatTests = []formatTest{
>         {"noslash", nil, ""},
> +       {"foo bar/baz", nil, ""},
> +       {"foo/bar baz", nil, ""},
>         {"foo/BAR", nil, "foo/bar"},
>         {"foo/BAR", map[string]string{"X": "Y"}, "foo/bar; x=Y"},
> +       {"foo/BAR", map[string]string{"space": "With space"}, `foo/bar;
> space="With space"`},
> +       {"foo/BAR", map[string]string{"quote": `With "quote`}, `foo/bar;
> quote="With \"quote"`},
> +       {"foo/BAR", map[string]string{"bslash": `With \backslash`},
> `foo/bar; bslash="With \\backslash"`},
> +       {"foo/BAR", map[string]string{"both": `With \backslash and
> "quote`}, `foo/bar; both="With \\backslash and \"quote"`},
> +       {"foo/BAR", map[string]string{"": "empty attribute"}, ""},
> +       {"foo/BAR", map[string]string{"bad attribute": "baz"}, ""},
> +       {"foo/BAR", map[string]string{"nonascii": "not an ascii character:
> ä"}, ""},
>  }
>
>  func TestFormatMediaType(t *testing.T) {
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>

[bradfitz, 2013-08-09 20:10:52]

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

mime: escape backslash in attribute values

R=golang-dev, bradfitz
CC=golang-dev
https://codereview.appspot.com/12689045

Committer: Brad Fitzpatrick <bradfitz@golang.org>

[peted, 2013-08-09 21:29:14]

On 2013/08/09 20:09:34, bradfitz wrote:
> LGTM
> 
> But your commit message could be simplified:
> 
> > mime: now properly escape backslash in attribute values
> 
> Both "now" and "properly" are redundant: now is implicit with the time of
> the CL & its commit.  And we never do anything improperly on purpose, so
> saying properly is also redundant.
> 
> mime: escape backslash on attribute values.
> 
> I trust that's proper.  If not, we'll hear about it later.

Well done sir :P

[dvyukov, 2013-08-10 10:11:40]

On Sat, Aug 10, 2013 at 12:10 AM, <bradfitz@golang.org> wrote:

> *** Submitted as
>
https://code.google.com/p/go/**source/detail?r=4fb48ae1b069<https://code.goog...
>
> mime: escape backslash in attribute values
>
> R=golang-dev, bradfitz
> CC=golang-dev
>
https://codereview.appspot.**com/12689045<https://codereview.appspot.com/1268...
>
> Committer: Brad Fitzpatrick <bradfitz@golang.org>
>
>
>
https://codereview.appspot.**com/12689045/<https://codereview.appspot.com/126...
>
>

test?

[peted, 2013-08-10 10:53:53]

On 2013/08/10 10:11:40, dvyukov wrote:
> On Sat, Aug 10, 2013 at 12:10 AM, <mailto:bradfitz@golang.org> wrote:
> 
> > *** Submitted as
> >
>
https://code.google.com/p/go/**source/detail?r=4fb48ae1b069%3Chttps://code.go...
> >
> > mime: escape backslash in attribute values
> >
> > R=golang-dev, bradfitz
> > CC=golang-dev
> >
>
https://codereview.appspot.**com/12689045%3Chttps://codereview.appspot.com/12...>
> >
> > Committer: Brad Fitzpatrick <mailto:bradfitz@golang.org>
> >
> >
> >
>
https://codereview.appspot.**com/12689045/%3Chttps://codereview.appspot.com/1...>
> >
> >
> 
> test?

I included some.

[dvyukov, 2013-08-10 10:57:59]

On Sat, Aug 10, 2013 at 2:53 PM, <pieter@binky.org.uk> wrote:

> On 2013/08/10 10:11:40, dvyukov wrote:
>
>  On Sat, Aug 10, 2013 at 12:10 AM, <mailto:bradfitz@golang.org> wrote:
>>
>
>  > *** Submitted as
>> >
>>
>
> https://code.google.com/p/go/****source/detail?r=4fb48ae1b069%**
>
3Chttps://code.google.com/p/**go/source/detail?r=**4fb48ae1b069%3E***<https://code.google.com/p/go/**source/detail?r=4fb48ae1b069%3Chttps://code.google.com/p/go/source/detail?r=4fb48ae1b069%3E***>
>
>  >
>> > mime: escape backslash in attribute values
>> >
>> > R=golang-dev, bradfitz
>> > CC=golang-dev
>> >
>>
>
> https://codereview.appspot.****com/12689045%3Chttps://coderev**
> iew.appspot.com/12689045 <http://codereview.appspot.com/12689045>>
>
>> >
>> > Committer: Brad Fitzpatrick <mailto:bradfitz@golang.org>
>> >
>> >
>> >
>>
>
> https://codereview.appspot.****com/12689045/%3Chttps://codere**
> view.appspot.com/12689045/ <http://codereview.appspot.com/12689045/>>
>
>> >
>> >
>>
>
>  test?
>>
>
> I included some.
>


Sorry, missed that.

[peted, 2013-08-10 11:03:53]

On 2013/08/10 10:57:59, dvyukov wrote:
> On Sat, Aug 10, 2013 at 2:53 PM, <mailto:pieter@binky.org.uk> wrote:
> > On 2013/08/10 10:11:40, dvyukov wrote:
> >  test?
> >>
> >
> > I included some.
> >
> 
> 
> Sorry, missed that.

No worries.