internal/http3: return error on mid-frame EOF

When a stream ends in the middle of a frame,
return a non-EOF error from Read or ReadByte.

When a stream ends at the end of a frame,
don't return io.EOF from the Read call that reads
the last byte of the frame.
(This complicates stream.Read slightly,
but means that code that reads frames consistently
never sees an io.EOF, but gets an error if it tries
to read past the end of a frame.)

For golang/go#70914

Change-Id: If1b852716fe5e3aa3503f6970e2e1fba2ebb5f48
Reviewed-on: https://go-review.googlesource.com/c/net/+/644116
Auto-Submit: Damien Neil <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>

Author: neild

internal/http3/stream.go

@@ -114,7 +114,7 @@ func (st *stream) ReadByte() (b byte, err error) {
 	}
 	b, err = st.stream.ReadByte()
 	if err != nil {
-		if err == io.EOF {
+		if err == io.EOF && st.lim < 0 {
 			return 0, io.EOF
 		}
 		return 0, errH3FrameError
@@ -125,14 +125,23 @@ func (st *stream) ReadByte() (b byte, err error) {
 // Read reads from the stream.
 func (st *stream) Read(b []byte) (int, error) {
 	n, err := st.stream.Read(b)
-	if err != nil {
-		if err == io.EOF {
-			return 0, io.EOF
+	if e2 := st.recordBytesRead(n); e2 != nil {
+		return 0, e2
+	}
+	if err == io.EOF {
+		if st.lim == 0 {
+			// EOF at end of frame, ignore.
+			return n, nil
+		} else if st.lim > 0 {
+			// EOF inside frame, error.
+			return 0, errH3FrameError
+		} else {
+			// EOF outside of frame, surface to caller.
+			return n, io.EOF
 		}
-		return 0, errH3FrameError
 	}
-	if err := st.recordBytesRead(n); err != nil {
-		return 0, err
+	if err != nil {
+		return 0, errH3FrameError
 	}
 	return n, nil
 }

internal/http3/stream_test.go

@@ -183,7 +183,7 @@ func TestStreamReadFrameOverflow(t *testing.T) {
 	}
 }
 
-func TestStreamReadFramePartial(t *testing.T) {
+func TestStreamReadFrameHeaderPartial(t *testing.T) {
 	var frame []byte
 	frame = quicwire.AppendVarint(frame, 1000) // type
 	frame = quicwire.AppendVarint(frame, 2000) // size
@@ -202,6 +202,54 @@ func TestStreamReadFramePartial(t *testing.T) {
 	}
 }
 
+func TestStreamReadFrameDataPartial(t *testing.T) {
+	st1, st2 := newStreamPair(t)
+	st1.writeVarint(1)          // type
+	st1.writeVarint(100)        // size
+	st1.Write(make([]byte, 50)) // data
+	st1.stream.CloseWrite()
+	if _, err := st2.readFrameHeader(); err != nil {
+		t.Fatalf("st.readFrameHeader() = %v", err)
+	}
+	if n, err := io.ReadAll(st2); err == nil {
+		t.Fatalf("io.ReadAll with partial frame = %v, nil; want error", n)
+	}
+}
+
+func TestStreamReadByteFrameDataPartial(t *testing.T) {
+	st1, st2 := newStreamPair(t)
+	st1.writeVarint(1)   // type
+	st1.writeVarint(100) // size
+	st1.stream.CloseWrite()
+	if _, err := st2.readFrameHeader(); err != nil {
+		t.Fatalf("st.readFrameHeader() = %v", err)
+	}
+	if b, err := st2.ReadByte(); err == nil {
+		t.Fatalf("io.ReadAll with partial frame = %v, nil; want error", b)
+	}
+}
+
+func TestStreamReadFrameDataAtEOF(t *testing.T) {
+	const typ = 10
+	data := []byte("hello")
+	st1, st2 := newStreamPair(t)
+	st1.writeVarint(typ)              // type
+	st1.writeVarint(int64(len(data))) // size
+	if err := st1.Flush(); err != nil {
+		t.Fatal(err)
+	}
+	if got, err := st2.readFrameHeader(); err != nil || got != typ {
+		t.Fatalf("st.readFrameHeader() = %v, %v; want %v, nil", got, err, typ)
+	}
+
+	st1.Write(data)         // data
+	st1.stream.CloseWrite() // end stream
+	got := make([]byte, len(data)+1)
+	if n, err := st2.Read(got); err != nil || n != len(data) || !bytes.Equal(got[:n], data) {
+		t.Fatalf("st.Read() = %v, %v (data=%x); want %v, nil (data=%x)", n, err, got[:n], len(data), data)
+	}
+}
+
 func TestStreamReadFrameData(t *testing.T) {
 	const typ = 10
 	data := []byte("hello")