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>

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.

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)
+	}
+}