net/http/internal: ignore chunk-extension when reading chunked encoding bodies

Fixes #13135

Change-Id: I45666f32cd91102211bf01a306edcb10deb65187
Reviewed-on: https://go-review.googlesource.com/16680
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>

diff --git a/src/net/http/internal/chunked.go b/src/net/http/internal/chunked.go
index 6d7c69874d9d6f6acbce282a28cb80cd34d070a9..3967ad614f465aa57491e87a824fd79c85bb9608 100644 (file)
--- a/src/net/http/internal/chunked.go
+++ b/src/net/http/internal/chunked.go
@@ -44,7 +44,7 @@ type chunkedReader struct {
 func (cr *chunkedReader) beginChunk() {
        // chunk-size CRLF
        var line []byte
-       line, cr.err = readLine(cr.r)
+       line, cr.err = readChunkLine(cr.r)
        if cr.err != nil {
                return
        }
@@ -104,10 +104,11 @@ func (cr *chunkedReader) Read(b []uint8) (n int, err error) {
 
 // Read a line of bytes (up to \n) from b.
 // Give up if the line exceeds maxLineLength.
-// The returned bytes are a pointer into storage in
-// the bufio, so they are only valid until the next bufio read.
-func readLine(b *bufio.Reader) (p []byte, err error) {
-       if p, err = b.ReadSlice('\n'); err != nil {
+// The returned bytes are owned by the bufio.Reader
+// so they are only valid until the next bufio read.
+func readChunkLine(b *bufio.Reader) ([]byte, error) {
+       p, err := b.ReadSlice('\n')
+       if err != nil {
                // We always know when EOF is coming.
                // If the caller asked for a line, there should be a line.
                if err == io.EOF {
@@ -120,7 +121,12 @@ func readLine(b *bufio.Reader) (p []byte, err error) {
        if len(p) >= maxLineLength {
                return nil, ErrLineTooLong
        }
-       return trimTrailingWhitespace(p), nil
+       p = trimTrailingWhitespace(p)
+       p, err = removeChunkExtension(p)
+       if err != nil {
+               return nil, err
+       }
+       return p, nil
 }
 
 func trimTrailingWhitespace(b []byte) []byte {
@@ -134,6 +140,23 @@ func isASCIISpace(b byte) bool {
        return b == ' ' || b == '\t' || b == '\n' || b == '\r'
 }
 
+// removeChunkExtension removes any chunk-extension from p.
+// For example,
+//     "0" => "0"
+//     "0;token" => "0"
+//     "0;token=val" => "0"
+//     `0;token="quoted string"` => "0"
+func removeChunkExtension(p []byte) ([]byte, error) {
+       semi := bytes.IndexByte(p, ';')
+       if semi == -1 {
+               return p, nil
+       }
+       // TODO: care about exact syntax of chunk extensions? We're
+       // ignoring and stripping them anyway. For now just never
+       // return an error.
+       return p[:semi], nil
+}
+
 // NewChunkedWriter returns a new chunkedWriter that translates writes into HTTP
 // "chunked" format before writing them to w. Closing the returned chunkedWriter
 // sends the final 0-length chunk that marks the end of the stream.

diff --git a/src/net/http/internal/chunked_test.go b/src/net/http/internal/chunked_test.go
index ebc626ea9d0ec368d5028ffb04b6f39024dedd14..7c1c91662fdd50f3ebdb1ee47d1d7f46cd72f73f 100644 (file)
--- a/src/net/http/internal/chunked_test.go
+++ b/src/net/http/internal/chunked_test.go
@@ -154,3 +154,18 @@ func TestParseHexUint(t *testing.T) {
                t.Error("expected error on bogus input")
        }
 }
+
+func TestChunkReadingIgnoresExtensions(t *testing.T) {
+       in := "7;ext=\"some quoted string\"\r\n" + // token=quoted string
+               "hello, \r\n" +
+               "17;someext\r\n" + // token without value
+               "world! 0123456789abcdef\r\n" +
+               "0;someextension=sometoken\r\n" // token=token
+       data, err := ioutil.ReadAll(NewChunkedReader(strings.NewReader(in)))
+       if err != nil {
+               t.Fatalf("ReadAll = %q, %v", data, err)
+       }
+       if g, e := string(data), "hello, world! 0123456789abcdef"; g != e {
+               t.Errorf("read %q; want %q", g, e)
+       }
+}