FAQ
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:
encoding/base64: Add a reader that discard \n and \r

The decoder gets confused by \n and \r, so filter these out
using a small Reader that discards these.

The same issue applies to the encoding/base32 package.
Fixes issue 4779

Please review this at https://codereview.appspot.com/7679043/

Affected files:
M src/pkg/encoding/base32/base32.go
M src/pkg/encoding/base32/base32_test.go
M src/pkg/encoding/base64/base64.go
M src/pkg/encoding/base64/base64_test.go


Index: src/pkg/encoding/base32/base32.go
===================================================================
--- a/src/pkg/encoding/base32/base32.go
+++ b/src/pkg/encoding/base32/base32.go
@@ -220,6 +220,28 @@
* Decoder
*/

+// Reader thats discards \n and \r from its input
+type discardReader struct {
+ io.Reader
+}
+
+func (d *discardReader) Read(p []byte) (n int, err error) {
+ n, err = d.Reader.Read(p)
+ // TODO: could check for adjacent \n and \r's
+ for i := 0; i < n; i++ {
+ if p[i] == '\n' || p[i] == '\r' {
+ if i+1 == n {
+ n--
+ } else {
+ p = append(p[:i], p[i+1:]...)
+ n--
+ i--
+ }
+ }
+ }
+ return
+}
+
type CorruptInputError int64

func (e CorruptInputError) Error() string {
@@ -379,7 +401,7 @@

// NewDecoder constructs a new base32 stream decoder.
func NewDecoder(enc *Encoding, r io.Reader) io.Reader {
- return &decoder{enc: enc, r: r}
+ return &decoder{enc: enc, r: &discardReader{r}}
}

// DecodedLen returns the maximum length in bytes of the decoded data
Index: src/pkg/encoding/base32/base32_test.go
===================================================================
--- a/src/pkg/encoding/base32/base32_test.go
+++ b/src/pkg/encoding/base32/base32_test.go
@@ -15,6 +15,10 @@
decoded, encoded string
}

+type testtriplet struct {
+ decoded, encodedNewline, encoded string
+}
+
var pairs = []testpair{
// RFC 4648 examples
{"", ""},
@@ -36,6 +40,14 @@
{"sure.", "ON2XEZJO"},
}

+var triplets = []testtriplet{
+ {"", "\n", ""},
+ {"f", "M\r\nY======", "MY======"},
+ {"fo", "MZXQ====\n\r", "MZXQ===="},
+ {"foo", "MZXW\r6=\n==", "MZXW6==="},
+ {"foob", "MZXW6Y\rQ=", "MZXW6YrQ="},
+}
+
var bigtest = testpair{
"Twas brillig, and the slithy toves",
"KR3WC4ZAMJZGS3DMNFTSYIDBNZSCA5DIMUQHG3DJORUHSIDUN53GK4Y=",
@@ -136,6 +148,16 @@
}
}

+// Issue is about base64, but is needed for base32 too.
+func TestDecoderIssue4779(t *testing.T) {
+ for _, p := range triplets {
+ decoder := NewDecoder(StdEncoding,
bytes.NewBufferString(p.encodedNewline))
+ dbuf, err := ioutil.ReadAll(decoder)
+ testEqual(t, "Decoding of %q = error %v, want %v", p.encodedNewline,
err, error(nil))
+ testEqual(t, "Decoding of %q = %q, want %q", p.encodedNewline,
string(dbuf), p.decoded)
+ }
+}
+
func TestDecodeCorrupt(t *testing.T) {
type corrupt struct {
e string
Index: src/pkg/encoding/base64/base64.go
===================================================================
--- a/src/pkg/encoding/base64/base64.go
+++ b/src/pkg/encoding/base64/base64.go
@@ -200,6 +200,28 @@
* Decoder
*/

+// Reader thats discards \n and \r from its input
+type discardReader struct {
+ io.Reader
+}
+
+func (d *discardReader) Read(p []byte) (n int, err error) {
+ n, err = d.Reader.Read(p)
+ // TODO: could check for adjacent \n and \r's
+ for i := 0; i < n; i++ {
+ if p[i] == '\n' || p[i] == '\r' {
+ if i+1 == n {
+ n--
+ } else {
+ p = append(p[:i], p[i+1:]...)
+ n--
+ i--
+ }
+ }
+ }
+ return
+}
+
type CorruptInputError int64

func (e CorruptInputError) Error() string {
@@ -345,7 +367,7 @@

// NewDecoder constructs a new base64 stream decoder.
func NewDecoder(enc *Encoding, r io.Reader) io.Reader {
- return &decoder{enc: enc, r: r}
+ return &decoder{enc: enc, r: &discardReader{r}}
}

// DecodedLen returns the maximum length in bytes of the decoded data
Index: src/pkg/encoding/base64/base64_test.go
===================================================================
--- a/src/pkg/encoding/base64/base64_test.go
+++ b/src/pkg/encoding/base64/base64_test.go
@@ -17,6 +17,10 @@
decoded, encoded string
}

+type testtriplet struct {
+ decoded, encodedNewline, encoded string
+}
+
var pairs = []testpair{
// RFC 3548 examples
{"\x14\xfb\x9c\x03\xd9\x7e", "FPucA9l+"},
@@ -43,6 +47,19 @@
{"sure.", "c3VyZS4="},
}

+var triplets = []testtriplet{
+ {"\x14\xfb\x9c\x03\xd9\x7e", "\nFPucA9l+", "FPucA9l+"},
+ {"\x14\xfb\x9c\x03\xd9", "F\nPucA9k=", "FPucA9k="},
+ {"\x14\xfb\x9c\x03", "FPucAw=\n=", "FPucAw=="},
+ {"\x14\xfb\x9c\x03", "FPucAw=\n=\n", "FPucAw=="},
+
+ {"", "\n", ""},
+ {"f", "Z\r\ng==", "Zg=="},
+ {"fo", "Zm8=\n\r", "Zm8=r"},
+ {"foo", "Zm9\rv", "Zm9v"},
+ {"foob", "Zm9vYg=\r=", "Zm9vYg=="},
+}
+
var bigtest = testpair{
"Twas brillig, and the slithy toves",
"VHdhcyBicmlsbGlnLCBhbmQgdGhlIHNsaXRoeSB0b3Zlcw==",
@@ -141,6 +158,15 @@
}
}

+func TestDecoderIssue4779(t *testing.T) {
+ for _, p := range triplets {
+ decoder := NewDecoder(StdEncoding,
bytes.NewBufferString(p.encodedNewline))
+ dbuf, err := ioutil.ReadAll(decoder)
+ testEqual(t, "Decoding of %q = error %v, want %v", p.encodedNewline,
err, error(nil))
+ testEqual(t, "Decoding of %q = %q, want %q", p.encodedNewline,
string(dbuf), p.decoded)
+ }
+}
+
func TestDecodeCorrupt(t *testing.T) {
type corrupt struct {
e string


--

---
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.
For more options, visit https://groups.google.com/groups/opt_out.

Search Discussions

  • Brad Fitzpatrick at Mar 10, 2013 at 3:42 pm
    The first line of your commit description should summarize what it fixes,
    not how it fixes it. A reader is an implementation detail, and this almost
    makes it sound like you're adding a new public API (add a reader!, until I
    see the actual patch. Maybe just "ignore newlines when decoding".

    Also, regarding the TODO--- why not just fix it now, as long as you're
    here? If it's something that needs fixing. I looked at
    https://tools.ietf.org/html/rfc4648#section-3.1 which is referenced from
    the issue, but I'm not sure that's what the TODO is referencing. What is
    special about \r\n together?

    On Sat, Mar 9, 2013 at 11:45 PM, 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:
    encoding/base64: Add a reader that discard \n and \r

    The decoder gets confused by \n and \r, so filter these out
    using a small Reader that discards these.

    The same issue applies to the encoding/base32 package.
    Fixes issue 4779

    Please review this at https://codereview.appspot.**com/7679043/<https://codereview.appspot.com/7679043/>

    Affected files:
    M src/pkg/encoding/base32/**base32.go
    M src/pkg/encoding/base32/**base32_test.go
    M src/pkg/encoding/base64/**base64.go
    M src/pkg/encoding/base64/**base64_test.go


    Index: src/pkg/encoding/base32/**base32.go
    ==============================**==============================**=======
    --- a/src/pkg/encoding/base32/**base32.go
    +++ b/src/pkg/encoding/base32/**base32.go
    @@ -220,6 +220,28 @@
    * Decoder
    */

    +// Reader thats discards \n and \r from its input
    +type discardReader struct {
    + io.Reader
    +}
    +
    +func (d *discardReader) Read(p []byte) (n int, err error) {
    + n, err = d.Reader.Read(p)
    + // TODO: could check for adjacent \n and \r's
    + for i := 0; i < n; i++ {
    + if p[i] == '\n' || p[i] == '\r' {
    + if i+1 == n {
    + n--
    + } else {
    + p = append(p[:i], p[i+1:]...)
    + n--
    + i--
    + }
    + }
    + }
    + return
    +}
    +
    type CorruptInputError int64

    func (e CorruptInputError) Error() string {
    @@ -379,7 +401,7 @@

    // NewDecoder constructs a new base32 stream decoder.
    func NewDecoder(enc *Encoding, r io.Reader) io.Reader {
    - return &decoder{enc: enc, r: r}
    + return &decoder{enc: enc, r: &discardReader{r}}
    }

    // DecodedLen returns the maximum length in bytes of the decoded data
    Index: src/pkg/encoding/base32/**base32_test.go
    ==============================**==============================**=======
    --- a/src/pkg/encoding/base32/**base32_test.go
    +++ b/src/pkg/encoding/base32/**base32_test.go
    @@ -15,6 +15,10 @@
    decoded, encoded string
    }

    +type testtriplet struct {
    + decoded, encodedNewline, encoded string
    +}
    +
    var pairs = []testpair{
    // RFC 4648 examples
    {"", ""},
    @@ -36,6 +40,14 @@
    {"sure.", "ON2XEZJO"},
    }

    +var triplets = []testtriplet{
    + {"", "\n", ""},
    + {"f", "M\r\nY======", "MY======"},
    + {"fo", "MZXQ====\n\r", "MZXQ===="},
    + {"foo", "MZXW\r6=\n==", "MZXW6==="},
    + {"foob", "MZXW6Y\rQ=", "MZXW6YrQ="},
    +}
    +
    var bigtest = testpair{
    "Twas brillig, and the slithy toves",
    "**KR3WC4ZAMJZGS3DMNFTSYIDBNZSCA5**DIMUQHG3DJORUHSIDUN53GK4Y=",
    @@ -136,6 +148,16 @@
    }
    }

    +// Issue is about base64, but is needed for base32 too.
    +func TestDecoderIssue4779(t *testing.T) {
    + for _, p := range triplets {
    + decoder := NewDecoder(StdEncoding, bytes.NewBufferString(p.
    **encodedNewline))
    + dbuf, err := ioutil.ReadAll(decoder)
    + testEqual(t, "Decoding of %q = error %v, want %v",
    p.encodedNewline, err, error(nil))
    + testEqual(t, "Decoding of %q = %q, want %q",
    p.encodedNewline, string(dbuf), p.decoded)
    + }
    +}
    +
    func TestDecodeCorrupt(t *testing.T) {
    type corrupt struct {
    e string
    Index: src/pkg/encoding/base64/**base64.go
    ==============================**==============================**=======
    --- a/src/pkg/encoding/base64/**base64.go
    +++ b/src/pkg/encoding/base64/**base64.go
    @@ -200,6 +200,28 @@
    * Decoder
    */

    +// Reader thats discards \n and \r from its input
    +type discardReader struct {
    + io.Reader
    +}
    +
    +func (d *discardReader) Read(p []byte) (n int, err error) {
    + n, err = d.Reader.Read(p)
    + // TODO: could check for adjacent \n and \r's
    + for i := 0; i < n; i++ {
    + if p[i] == '\n' || p[i] == '\r' {
    + if i+1 == n {
    + n--
    + } else {
    + p = append(p[:i], p[i+1:]...)
    + n--
    + i--
    + }
    + }
    + }
    + return
    +}
    +
    type CorruptInputError int64

    func (e CorruptInputError) Error() string {
    @@ -345,7 +367,7 @@

    // NewDecoder constructs a new base64 stream decoder.
    func NewDecoder(enc *Encoding, r io.Reader) io.Reader {
    - return &decoder{enc: enc, r: r}
    + return &decoder{enc: enc, r: &discardReader{r}}
    }

    // DecodedLen returns the maximum length in bytes of the decoded data
    Index: src/pkg/encoding/base64/**base64_test.go
    ==============================**==============================**=======
    --- a/src/pkg/encoding/base64/**base64_test.go
    +++ b/src/pkg/encoding/base64/**base64_test.go
    @@ -17,6 +17,10 @@
    decoded, encoded string
    }

    +type testtriplet struct {
    + decoded, encodedNewline, encoded string
    +}
    +
    var pairs = []testpair{
    // RFC 3548 examples
    {"\x14\xfb\x9c\x03\xd9\x7e", "FPucA9l+"},
    @@ -43,6 +47,19 @@
    {"sure.", "c3VyZS4="},
    }

    +var triplets = []testtriplet{
    + {"\x14\xfb\x9c\x03\xd9\x7e", "\nFPucA9l+", "FPucA9l+"},
    + {"\x14\xfb\x9c\x03\xd9", "F\nPucA9k=", "FPucA9k="},
    + {"\x14\xfb\x9c\x03", "FPucAw=\n=", "FPucAw=="},
    + {"\x14\xfb\x9c\x03", "FPucAw=\n=\n", "FPucAw=="},
    +
    + {"", "\n", ""},
    + {"f", "Z\r\ng==", "Zg=="},
    + {"fo", "Zm8=\n\r", "Zm8=r"},
    + {"foo", "Zm9\rv", "Zm9v"},
    + {"foob", "Zm9vYg=\r=", "Zm9vYg=="},
    +}
    +
    var bigtest = testpair{
    "Twas brillig, and the slithy toves",
    "**VHdhcyBicmlsbGlnLCBhbmQgdGhlIH**NsaXRoeSB0b3Zlcw==",
    @@ -141,6 +158,15 @@
    }
    }

    +func TestDecoderIssue4779(t *testing.T) {
    + for _, p := range triplets {
    + decoder := NewDecoder(StdEncoding, bytes.NewBufferString(p.
    **encodedNewline))
    + dbuf, err := ioutil.ReadAll(decoder)
    + testEqual(t, "Decoding of %q = error %v, want %v",
    p.encodedNewline, err, error(nil))
    + testEqual(t, "Decoding of %q = %q, want %q",
    p.encodedNewline, string(dbuf), p.decoded)
    + }
    +}
    +
    func TestDecodeCorrupt(t *testing.T) {
    type corrupt struct {
    e string


    --

    ---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@googlegroups.com>
    .
    For more options, visit https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/opt_out>
    .

    --

    ---
    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.
    For more options, visit https://groups.google.com/groups/opt_out.

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedMar 10, '13 at 7:45a
activeMar 10, '13 at 3:42p
posts2
users2
websitegolang.org

People

Translate

site design / logo © 2021 Grokbase