code review 6127045: [release-branch.go1] mime: make ParseMediaType return z...

[release-branch.go1] mime: make ParseMediaType return zero results on error

««« CL 6119051 / 1384d7510575
mime: make ParseMediaType return zero results on error

Fixes issue 3562

R=golang-dev, adg, rsc
CC=golang-dev
http://codereview.appspot.com/6119051
»»»

[rsc, 2012-04-25 19:42:39]

Hello golang-dev@googlegroups.com,

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

[bradfitz, 2012-04-25 19:43:58]

LGTM

On Wed, Apr 25, 2012 at 12:42 PM, <rsc@golang.org> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> [release-branch.go1] mime: make ParseMediaType return zero results on
> error
>
> ««« CL 6119051 / 1384d7510575
> mime: make ParseMediaType return zero results on error
>
> Fixes issue 3562
>
> R=golang-dev, adg, rsc
> CC=golang-dev
>
http://codereview.appspot.com/**6119051<http://codereview.appspot.com/6119051>
> »»»
>
>
> Please review this at
http://codereview.appspot.com/**6127045/<http://codereview.appspot.com/6127045/>
>
> 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
> @@ -99,7 +99,7 @@
>
>        err = checkMediaTypeDisposition(**mediatype)
>        if err != nil {
> -               return
> +               return "", nil, err
>        }
>
>        params = make(map[string]string)
> Index: src/pkg/mime/mediatype_test.go
> ==============================**==============================**=======
> --- a/src/pkg/mime/mediatype_test.**go
> +++ b/src/pkg/mime/mediatype_test.**go
> @@ -244,13 +244,33 @@
>        }
>  }
>
> +type badMediaTypeTest struct {
> +       in  string
> +       err string
> +}
> +
> +var badMediaTypeTests = []badMediaTypeTest{
> +       {"bogus ;=========", "mime: invalid media parameter"},
> +       {"bogus/<script>alert</script>**", "mime: expected token after
> slash"},
> +       {"bogus/bogus<script>alert</**script>", "mime: unexpected content
> after media subtype"},
> +}
> +
>  func TestParseMediaTypeBogus(t *testing.T) {
> -       mt, params, err := ParseMediaType("bogus ;=========")
> -       if err == nil {
> -               t.Fatalf("expected an error parsing invalid media type;
> got type %q, params %#v", mt, params)
> -       }
> -       if err.Error() != "mime: invalid media parameter" {
> -               t.Errorf("expected invalid media parameter; got error %q",
> err)
> +       for _, tt := range badMediaTypeTests {
> +               mt, params, err := ParseMediaType(tt.in)
> +               if err == nil {
> +                       t.Errorf("ParseMediaType(%q) = nil error; want
> parse error", tt.in)
> +                       continue
> +               }
> +               if err.Error() != tt.err {
> +                       t.Errorf("ParseMediaType(%q) = err %q; want %q",
> tt.in, err.Error(), tt.err)
> +               }
> +               if params != nil {
> +                       t.Errorf("ParseMediaType(%q): got non-nil params
> on error", tt.in)
> +               }
> +               if mt != "" {
> +                       t.Errorf("ParseMediaType(%q): got non-empty media
> type string on error", tt.in)
> +               }
>        }
>  }
>
>
>
>

[rsc, 2012-04-25 19:44:36]

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

[release-branch.go1] mime: make ParseMediaType return zero results on error

««« CL 6119051 / 1384d7510575
mime: make ParseMediaType return zero results on error

Fixes issue 3562

R=golang-dev, adg, rsc
CC=golang-dev
http://codereview.appspot.com/6119051
»»»

R=golang-dev, bradfitz
CC=golang-dev
http://codereview.appspot.com/6127045