Skip to content

Commit

Permalink
http2/hpack: track the beginning of a header block
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
fraenkel authored and bradfitz committed Dec 13, 2018
1 parent 6105869 commit 891ebc4
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 4 deletions.
10 changes: 9 additions & 1 deletion http2/hpack/hpack.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ type Decoder struct {
// 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
Expand All @@ -101,6 +103,7 @@ func NewDecoder(maxDynamicTableSize uint32, emitFunc func(f HeaderField)) *Decod
d := &Decoder{
emit: emitFunc,
emitEnabled: true,
firstField: true,
}
d.dynTab.table.init()
d.dynTab.allowedMaxSize = maxDynamicTableSize
Expand Down Expand Up @@ -226,11 +229,15 @@ func (d *Decoder) DecodeFull(p []byte) ([]HeaderField, error) {
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
}

Expand Down Expand Up @@ -266,6 +273,7 @@ func (d *Decoder) Write(p []byte) (n int, err error) {
d.saveBuf.Write(d.buf)
return len(p), nil
}
d.firstField = false
if err != nil {
break
}
Expand Down Expand Up @@ -391,7 +399,7 @@ func (d *Decoder) callEmit(hf HeaderField) error {
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")}
}

Expand Down
14 changes: 11 additions & 3 deletions http2/hpack/hpack_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -748,14 +748,22 @@ func TestDynamicSizeUpdate(t *testing.T) {
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")
}
Expand Down

0 comments on commit 891ebc4

Please sign in to comment.