use io.ErrUnexpectedEOF when input is too short (#73)

Author: marten-seemann

decoder.go

@@ -8,15 +8,6 @@ import (
 	"golang.org/x/net/http2/hpack"
 )
 
-// A decodingError represents a QPACK decoding error as defined by the specification
-type decodingError struct {
-	err error
-}
-
-func (de decodingError) Error() string {
-	return fmt.Sprintf("decoding error: %v", de.err)
-}
-
 // An invalidIndexError is returned when decoding encounters an invalid index
 // (e.g., an index that is out of bounds for the static table).
 type invalidIndexError int
@@ -25,7 +16,7 @@ func (e invalidIndexError) Error() string {
 	return fmt.Sprintf("invalid indexed representation index %d", int(e))
 }
 
-var errNoDynamicTable = decodingError{errors.New("no dynamic table")}
+var errNoDynamicTable = errors.New("no dynamic table")
 
 // A Decoder decodes QPACK header blocks.
 // A Decoder can be reused to decode multiple header blocks on different streams
@@ -59,7 +50,7 @@ func (d *Decoder) Decode(p []byte) DecodeFunc {
 			p = rest
 			readRequiredInsertCount = true
 			if requiredInsertCount != 0 {
-				return HeaderField{}, decodingError{errors.New("expected Required Insert Count to be zero")}
+				return HeaderField{}, errors.New("expected Required Insert Count to be zero")
 			}
 		}
 
@@ -71,7 +62,7 @@ func (d *Decoder) Decode(p []byte) DecodeFunc {
 			p = rest
 			readDeltaBase = true
 			if base != 0 {
-				return HeaderField{}, decodingError{errors.New("expected Base to be zero")}
+				return HeaderField{}, errors.New("expected Base to be zero")
 			}
 		}
 
@@ -111,7 +102,7 @@ func (d *Decoder) parseIndexedHeaderField(buf []byte) (_ HeaderField, rest []byt
 	}
 	hf, ok := d.at(index)
 	if !ok {
-		return HeaderField{}, buf, decodingError{invalidIndexError(index)}
+		return HeaderField{}, buf, invalidIndexError(index)
 	}
 	return hf, rest, nil
 }
@@ -130,11 +121,11 @@ func (d *Decoder) parseLiteralHeaderField(buf []byte) (_ HeaderField, rest []byt
 	}
 	hf, ok := d.at(index)
 	if !ok {
-		return HeaderField{}, buf, decodingError{invalidIndexError(index)}
+		return HeaderField{}, buf, invalidIndexError(index)
 	}
 	buf = rest
 	if len(buf) == 0 {
-		return HeaderField{}, buf, decodingError{errors.New("truncated literal header field")}
+		return HeaderField{}, buf, io.ErrUnexpectedEOF
 	}
 	usesHuffman := buf[0]&0x80 > 0
 	val, rest, err := d.readString(rest, 7, usesHuffman)
@@ -153,7 +144,7 @@ func (d *Decoder) parseLiteralHeaderFieldWithoutNameReference(buf []byte) (_ Hea
 	}
 	buf = rest
 	if len(buf) == 0 {
-		return HeaderField{}, rest, decodingError{errors.New("truncated literal header field without name reference")}
+		return HeaderField{}, rest, io.ErrUnexpectedEOF
 	}
 	usesHuffmanForVal := buf[0]&0x80 > 0
 	val, rest, err := d.readString(buf, 7, usesHuffmanForVal)
@@ -169,7 +160,7 @@ func (d *Decoder) readString(buf []byte, n uint8, usesHuffman bool) (string, []b
 		return "", nil, err
 	}
 	if uint64(len(buf)) < l {
-		return "", nil, decodingError{errors.New("truncated string")}
+		return "", nil, io.ErrUnexpectedEOF
 	}
 	var val string
 	if usesHuffman {

decoder_test.go

@@ -24,12 +24,12 @@ func TestDecoderInvalidInputs(t *testing.T) {
 		{
 			name:     "non-zero required insert count", // we don't support dynamic table updates
 			input:    append(appendVarInt(nil, 8, 1), appendVarInt(nil, 7, 0)...),
-			expected: "decoding error: expected Required Insert Count to be zero",
+			expected: "expected Required Insert Count to be zero",
 		},
 		{
 			name:     "non-zero delta base", // we don't support dynamic table updates
 			input:    append(appendVarInt(nil, 8, 0), appendVarInt(nil, 7, 1)...),
-			expected: "decoding error: expected Base to be zero",
+			expected: "expected Base to be zero",
 		},
 		{
 			name:     "unknown type byte",
@@ -174,7 +174,7 @@ func TestDecoderInvalidIndexedHeaderFields(t *testing.T) {
 				data[0] ^= 0x80 | 0x40
 				return insertPrefix(data)
 			}(),
-			expected: "decoding error: invalid indexed representation index 10000",
+			expected: "invalid indexed representation index 10000",
 		},
 		{
 			name: "rejects an indexed header field that references the dynamic table",
@@ -209,6 +209,58 @@ func TestDecoderLiteralHeaderFieldWithoutNameReference(t *testing.T) {
 	require.Equal(t, literalFieldWithoutNameReference.Expected, decodeAll(t, decodeFn))
 }
 
+func TestDecoderEOF(t *testing.T) {
+	t.Run("literal field without name reference", func(t *testing.T) {
+		testDecoderEOF(t,
+			literalFieldWithoutNameReference.Data,
+			len(literalFieldWithoutNameReference.Expected),
+		)
+	})
+
+	t.Run("literal field with name reference", func(t *testing.T) {
+		testDecoderEOF(t,
+			literalFieldWithNameReference.Data,
+			len(literalFieldWithNameReference.Expected),
+		)
+	})
+
+	t.Run("literal field with Huffman encoding", func(t *testing.T) {
+		testDecoderEOF(t,
+			literalFieldWithHuffmanEncoding.Data,
+			len(literalFieldWithHuffmanEncoding.Expected),
+		)
+	})
+
+	t.Run("indexed field", func(t *testing.T) {
+		testDecoderEOF(t,
+			indexedField.Data,
+			len(indexedField.Expected),
+		)
+	})
+}
+
+func testDecoderEOF(t *testing.T, data []byte, numExpected int) {
+	for i := range data {
+		dec := NewDecoder()
+		decodeFn := dec.Decode(data[:i])
+		var hfs []HeaderField
+		for {
+			hf, err := decodeFn()
+			// the data might have been cut right after a header field,
+			// which is a valid header
+			if err == io.EOF {
+				require.Less(t, len(hfs), numExpected)
+				break
+			}
+			if err != nil {
+				require.ErrorIs(t, err, io.ErrUnexpectedEOF)
+				break
+			}
+			hfs = append(hfs, hf)
+		}
+	}
+}
+
 func BenchmarkDecoder(b *testing.B) {
 	b.Run("literal field without name reference", func(b *testing.B) {
 		benchmarkDecoder(b,

varint.go

@@ -2,13 +2,13 @@ package qpack
 
 // copied from the Go standard library HPACK implementation
 
-import "errors"
-
-var (
-	errVarintOverflow = errors.New("varint integer overflow")
-	errNeedMore       = errors.New("need more data")
+import (
+	"errors"
+	"io"
 )
 
+var errVarintOverflow = errors.New("varint integer overflow")
+
 // appendVarInt appends i, as encoded in variable integer form using n
 // bit prefix, to dst and returns the extended buffer.
 //
@@ -34,13 +34,13 @@ func appendVarInt(dst []byte, n byte, i uint64) []byte {
 // n must always be between 1 and 8.
 //
 // The returned remain buffer is either a smaller suffix of p, or err != nil.
-// The error is errNeedMore if p doesn't contain a complete integer.
+// The error is io.ErrUnexpectedEOF if p doesn't contain a complete integer.
 func readVarInt(n byte, p []byte) (i uint64, remain []byte, err error) {
 	if n < 1 || n > 8 {
 		panic("bad n")
 	}
 	if len(p) == 0 {
-		return 0, p, errNeedMore
+		return 0, p, io.ErrUnexpectedEOF
 	}
 	i = uint64(p[0])
 	if n < 8 {
@@ -65,5 +65,5 @@ func readVarInt(n byte, p []byte) (i uint64, remain []byte, err error) {
 			return 0, origP, errVarintOverflow
 		}
 	}
-	return 0, origP, errNeedMore
+	return 0, origP, io.ErrUnexpectedEOF
 }