commit 891ebc4b82d6e74f468c533b06f983c7be918a96
author Michael Fraenkel <michael.fraenkel@gmail.com> Thu Dec 13 06:41:52 2018 -0500
committer Brad Fitzpatrick <bradfitz@golang.org> Thu Dec 13 20:27:11 2018 +0000
tree 433e7498e5bf6de8b12dd5a4633fcbf6bd513d44
parent 610586996380ceef02dd726cc09df7e00a3f8e56

http2/hpack: track the beginning of a header block

dynamic table size updates must occur at the beginning of the first
header block. The original fix, golang/go#25023, guaranteed it was at
the beginning of the very first block. The Close method implicitly
marked the end of the current header. We now document the Close behavior
and can track when we are at the beginning of the first block.

Updates golang/go#29187

Change-Id: I83ec39546527cb17d7de8a88ec417a46443d2baa
Reviewed-on: https://go-review.googlesource.com/c/153978
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

http2/hpack/hpack.go

diff --git a/http2/hpack/hpack.go b/http2/hpack/hpack.go
index 166788c..85f18a2 100644
--- a/http2/hpack/hpack.go
+++ b/http2/hpack/hpack.go
@@ -92,6 +92,8 @@
 	// saveBuf is previous data passed to Write which we weren't able
 	// to fully parse before. Unlike buf, we own this data.
 	saveBuf bytes.Buffer
+
+	firstField bool // processing the first field of the header block
 }
 
 // NewDecoder returns a new decoder with the provided maximum dynamic
@@ -101,6 +103,7 @@
 	d := &Decoder{
 		emit:        emitFunc,
 		emitEnabled: true,
+		firstField:  true,
 	}
 	d.dynTab.table.init()
 	d.dynTab.allowedMaxSize = maxDynamicTableSize
@@ -226,11 +229,15 @@
 	return hf, nil
 }
 
+// Close declares that the decoding is complete and resets the Decoder
+// to be reused again for a new header block. If there is any remaining
+// data in the decoder's buffer, Close returns an error.
 func (d *Decoder) Close() error {
 	if d.saveBuf.Len() > 0 {
 		d.saveBuf.Reset()
 		return DecodingError{errors.New("truncated headers")}
 	}
+	d.firstField = true
 	return nil
 }
 
@@ -266,6 +273,7 @@
 			d.saveBuf.Write(d.buf)
 			return len(p), nil
 		}
+		d.firstField = false
 		if err != nil {
 			break
 		}
@@ -391,7 +399,7 @@
 func (d *Decoder) parseDynamicTableSizeUpdate() error {
 	// RFC 7541, sec 4.2: This dynamic table size update MUST occur at the
 	// beginning of the first header block following the change to the dynamic table size.
-	if d.dynTab.size > 0 {
+	if !d.firstField && d.dynTab.size > 0 {
 		return DecodingError{errors.New("dynamic table size update MUST occur at the beginning of a header block")}
 	}
 

http2/hpack/hpack_test.go

diff --git a/http2/hpack/hpack_test.go b/http2/hpack/hpack_test.go
index 3f22274..a361a2a 100644
--- a/http2/hpack/hpack_test.go
+++ b/http2/hpack/hpack_test.go
@@ -748,14 +748,22 @@
 	enc.SetMaxDynamicTableSize(255)
 	enc.WriteField(HeaderField{Name: "foo", Value: "bar"})
 
-	d := NewDecoder(4096, nil)
-	_, err := d.DecodeFull(buf.Bytes())
+	d := NewDecoder(4096, func(_ HeaderField) {})
+	_, err := d.Write(buf.Bytes())
+	if err != nil {
+		t.Fatalf("unexpected error: got = %v", err)
+	}
+
+	d.Close()
+
+	// Start a new header
+	_, err = d.Write(buf.Bytes())
 	if err != nil {
 		t.Fatalf("unexpected error: got = %v", err)
 	}
 
 	// must fail since the dynamic table update must be at the beginning
-	_, err = d.DecodeFull(buf.Bytes())
+	_, err = d.Write(buf.Bytes())
 	if err == nil {
 		t.Fatalf("dynamic table size update not at the beginning of a header block")
 	}