FAQ
Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev@googlegroups.com,

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


Description:
net/http: remove more garbage from chunk reading

Noticed this while closing tabs. Yesterday I thought I could
ignore this garbage and hope that a fix for issue 2205 handled
it, but I just realized that's the opposite case,
string->[]byte, whereas this is []byte->string. I'm having a
hard time convincing myself that an Issue 2205-style fix with
static analysis and faking a string header would be safe in
all cases without violating the memory model (callee assumes
frozen memory; are there non-racy ways it could keep being
modified?)

Please review this at http://codereview.appspot.com/6850067/

Affected files:
M src/pkg/net/http/chunked.go
M src/pkg/net/http/chunked_test.go
M src/pkg/net/http/httputil/chunked.go
M src/pkg/net/http/httputil/chunked_test.go


Index: src/pkg/net/http/chunked.go
===================================================================
--- a/src/pkg/net/http/chunked.go
+++ b/src/pkg/net/http/chunked.go
@@ -15,7 +15,6 @@
"errors"
"fmt"
"io"
- "strconv"
)

const maxLineLength = 4096 // assumed <= bufio.defaultBufSize
@@ -45,12 +44,12 @@

func (cr *chunkedReader) beginChunk() {
// chunk-size CRLF
- var line string
+ var line []byte
line, cr.err = readLine(cr.r)
if cr.err != nil {
return
}
- cr.n, cr.err = strconv.ParseUint(line, 16, 64)
+ cr.n, cr.err = parseHexUint(line)
if cr.err != nil {
return
}
@@ -89,7 +88,7 @@
// 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 readLineBytes(b *bufio.Reader) (p []byte, err error) {
+func readLine(b *bufio.Reader) (p []byte, err error) {
if p, err = b.ReadSlice('\n'); err != nil {
// We always know when EOF is coming.
// If the caller asked for a line, there should be a line.
@@ -110,15 +109,6 @@
return p, nil
}

-// readLineBytes, but convert the bytes into a string.
-func readLine(b *bufio.Reader) (s string, err error) {
- p, e := readLineBytes(b)
- if e != nil {
- return "", e
- }
- return string(p), 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.
@@ -167,3 +157,21 @@
_, err := io.WriteString(cw.Wire, "0\r\n")
return err
}
+
+func parseHexUint(v []byte) (n uint64, err error) {
+ for _, b := range v {
+ n <<= 4
+ switch {
+ case '0' <= b && b <= '9':
+ b = b - '0'
+ case 'a' <= b && b <= 'f':
+ b = b - 'a' + 10
+ case 'A' <= b && b <= 'F':
+ b = b - 'A' + 10
+ default:
+ return 0, errors.New("invalid byte in chunk length")
+ }
+ n |= uint64(b)
+ }
+ return
+}
Index: src/pkg/net/http/chunked_test.go
===================================================================
--- a/src/pkg/net/http/chunked_test.go
+++ b/src/pkg/net/http/chunked_test.go
@@ -9,6 +9,7 @@

import (
"bytes"
+ "fmt"
"io/ioutil"
"testing"
)
@@ -37,3 +38,16 @@
t.Errorf("chunk reader read %q; want %q", g, e)
}
}
+
+func TestParseHexUint(t *testing.T) {
+ for i := uint64(0); i <= 1234; i++ {
+ line := []byte(fmt.Sprintf("%x", i))
+ got, err := parseHexUint(line)
+ if err != nil {
+ t.Fatalf("on %d: %v", i, err)
+ }
+ if got != i {
+ t.Errorf("for input %q = %d; want %d", line, got, i)
+ }
+ }
+}
Index: src/pkg/net/http/httputil/chunked.go
===================================================================
--- a/src/pkg/net/http/httputil/chunked.go
+++ b/src/pkg/net/http/httputil/chunked.go
@@ -17,7 +17,6 @@
"errors"
"fmt"
"io"
- "strconv"
)

const maxLineLength = 4096 // assumed <= bufio.defaultBufSize
@@ -47,12 +46,12 @@

func (cr *chunkedReader) beginChunk() {
// chunk-size CRLF
- var line string
+ var line []byte
line, cr.err = readLine(cr.r)
if cr.err != nil {
return
}
- cr.n, cr.err = strconv.ParseUint(line, 16, 64)
+ cr.n, cr.err = parseHexUint(line)
if cr.err != nil {
return
}
@@ -91,7 +90,7 @@
// 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 readLineBytes(b *bufio.Reader) (p []byte, err error) {
+func readLine(b *bufio.Reader) (p []byte, err error) {
if p, err = b.ReadSlice('\n'); err != nil {
// We always know when EOF is coming.
// If the caller asked for a line, there should be a line.
@@ -112,15 +111,6 @@
return p, nil
}

-// readLineBytes, but convert the bytes into a string.
-func readLine(b *bufio.Reader) (s string, err error) {
- p, e := readLineBytes(b)
- if e != nil {
- return "", e
- }
- return string(p), 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.
@@ -169,3 +159,21 @@
_, err := io.WriteString(cw.Wire, "0\r\n")
return err
}
+
+func parseHexUint(v []byte) (n uint64, err error) {
+ for _, b := range v {
+ n <<= 4
+ switch {
+ case '0' <= b && b <= '9':
+ b = b - '0'
+ case 'a' <= b && b <= 'f':
+ b = b - 'a' + 10
+ case 'A' <= b && b <= 'F':
+ b = b - 'A' + 10
+ default:
+ return 0, errors.New("invalid byte in chunk length")
+ }
+ n |= uint64(b)
+ }
+ return
+}
Index: src/pkg/net/http/httputil/chunked_test.go
===================================================================
--- a/src/pkg/net/http/httputil/chunked_test.go
+++ b/src/pkg/net/http/httputil/chunked_test.go
@@ -11,6 +11,7 @@

import (
"bytes"
+ "fmt"
"io/ioutil"
"testing"
)
@@ -39,3 +40,16 @@
t.Errorf("chunk reader read %q; want %q", g, e)
}
}
+
+func TestParseHexUint(t *testing.T) {
+ for i := uint64(0); i <= 1234; i++ {
+ line := []byte(fmt.Sprintf("%x", i))
+ got, err := parseHexUint(line)
+ if err != nil {
+ t.Fatalf("on %d: %v", i, err)
+ }
+ if got != i {
+ t.Errorf("for input %q = %d; want %d", line, got, i)
+ }
+ }
+}

Search Discussions

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedNov 17, '12 at 5:43p
activeNov 20, '12 at 9:56a
posts15
users6
websitegolang.org

People

Translate

site design / logo © 2022 Grokbase