code review 88330044: encoding/base64: don't lose a byte of output when encou...

encoding/base64: don't lose a byte of output when encountering trailing garbage

Fixes Issue 7733

[bradfitz, 2014-04-16 03:03:46]

Hello golang-codereviews@googlegroups.com (cc: nigeltao@golang.org,
r@golang.org, rsc@golang.org),

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

[bradfitz, 2014-04-16 03:08:47]

R=anybody who knows base64 stuff

Just cleaning up Go 1.3 dashboard bugs.



On Tue, Apr 15, 2014 at 8:03 PM, <bradfitz@golang.org> wrote:

> Reviewers: golang-codereviews,
>
> Message:
> Hello golang-codereviews@googlegroups.com (cc: nigeltao@golang.org,
> r@golang.org, rsc@golang.org),
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> encoding/base64: don't lose a byte of output when encountering trailing
> garbage
>
> Fixes Issue 7733
>
> Please review this at https://codereview.appspot.com/88330044/
>
> Affected files (+15, -2 lines):
>   M src/pkg/encoding/base64/base64.go
>   M src/pkg/encoding/base64/base64_test.go
>
>
> Index: src/pkg/encoding/base64/base64.go
> ===================================================================
> --- a/src/pkg/encoding/base64/base64.go
> +++ b/src/pkg/encoding/base64/base64.go
> @@ -250,7 +250,7 @@
>                                 }
>                                 if len(src) > 0 {
>                                         // trailing garbage
> -                                       return n, false,
> CorruptInputError(olen - len(src))
> +                                       err = CorruptInputError(olen -
> len(src))
>                                 }
>                                 dlen, end = j, true
>                                 break
> @@ -277,7 +277,7 @@
>                 n += dlen - 1
>         }
>
> -       return n, end, nil
> +       return n, end, err
>  }
>
>  // Decode decodes src using the encoding enc.  It writes at most
> Index: src/pkg/encoding/base64/base64_test.go
> ===================================================================
> --- a/src/pkg/encoding/base64/base64_test.go
> +++ b/src/pkg/encoding/base64/base64_test.go
> @@ -9,6 +9,7 @@
>         "errors"
>         "io"
>         "io/ioutil"
> +       "reflect"
>         "strings"
>         "testing"
>         "time"
> @@ -165,6 +166,7 @@
>                 {"AAA=", -1},
>                 {"AAAA", -1},
>                 {"AAAAAA=", 7},
> +               {"YWJjZA=====", 8},
>         }
>         for _, tc := range testCases {
>                 dbuf := make([]byte, StdEncoding.DecodedLen(len(tc.
> input)))
> @@ -329,3 +331,14 @@
>                 t.Error("Decoded results not equal")
>         }
>  }
> +
> +func TestDecoderIssue7733(t *testing.T) {
> +       s, err := StdEncoding.DecodeString("YWJjZA=====")
> +       want := CorruptInputError(8)
> +       if !reflect.DeepEqual(want, err) {
> +               t.Errorf("Error = %v; want CorruptInputError(8)")
> +       }
> +       if string(s) != "abcd" {
> +               t.Errorf("DecodeString = %q; want abcd", s)
> +       }
> +}
>
>
>

[minux1, 2014-04-16 04:53:47]

LGTM.

[bradfitz, 2014-04-16 18:32:42]

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

encoding/base64: don't lose a byte of output when encountering trailing garbage

Fixes Issue 7733

LGTM=minux.ma
R=golang-codereviews, minux.ma
CC=golang-codereviews, nigeltao, r, rsc
https://codereview.appspot.com/88330044

[nigeltao, 2014-04-16 20:01:56]

This CL has been submitted, but CC'ing ruiu@google.com, who was looking at this
code recently in https://codereview.appspot.com/75340044/

[bradfitz, 2014-04-16 20:03:15]

Thanks.

Further review welcome.



On Wed, Apr 16, 2014 at 1:01 PM, <nigeltao@golang.org> wrote:

> This CL has been submitted, but CC'ing ruiu@google.com, who was looking
> at this code recently in https://codereview.appspot.com/75340044/
>
> https://codereview.appspot.com/88330044/
>

[ruiu, 2014-04-16 21:02:56]

LGTM