internal/http3: define connection and stream error types

HTTP/3 distinguishes between connection errors which result in an entire connection closing, and stream errors which only terminate a single request stream. Define internal types to represent these two types of error. For golang/go#70914 Change-Id: I907f395adc82a683b5c2eda65f936b1ab4904ffb Reviewed-on: https://go-review.googlesource.com/c/net/+/644117 Reviewed-by: Jonathan Amsterdam <jba@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Damien Neil <dneil@google.com>
golang · Jan 24, 2025 · 5bda71a · 5bda71a
1 parent 3c1185a
commit 5bda71a
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 27 deletions.
diff --git a/internal/http3/errors.go b/internal/http3/errors.go
@@ -82,3 +82,23 @@ func (e http3Error) Error() string {
 	}
 	return fmt.Sprintf("H3_ERROR_%v", int(e))
 }
+
+// A streamError is an error which terminates a stream, but not the connection.
+// https://www.rfc-editor.org/rfc/rfc9114.html#section-8-1
+type streamError struct {
+	code    http3Error
+	message string
+}
+
+func (e *streamError) Error() string { return e.message }
+func (e *streamError) Unwrap() error { return e.code }
+
+// A connectionError is an error which results in the entire connection closing.
+// https://www.rfc-editor.org/rfc/rfc9114.html#section-8-2
+type connectionError struct {
+	code    http3Error
+	message string
+}
+
+func (e *connectionError) Error() string { return e.message }
+func (e *connectionError) Unwrap() error { return e.code }
diff --git a/internal/http3/stream.go b/internal/http3/stream.go
@@ -87,7 +87,10 @@ func (st *stream) readFrameHeader() (ftype frameType, err error) {
 // It returns an error if the entire contents of a frame have not been read.
 func (st *stream) endFrame() error {
 	if st.lim != 0 {
-		return errH3FrameError
+		return &connectionError{
+			code:    errH3FrameError,
+			message: "invalid HTTP/3 frame",
+		}
 	}
 	st.lim = -1
 	return nil
@@ -160,9 +163,9 @@ func (st *stream) discardUnknownFrame(ftype frameType) error {
 		frameTypePushPromise,
 		frameTypeGoaway,
 		frameTypeMaxPushID:
-		return &quic.ApplicationError{
-			Code:   uint64(errH3FrameUnexpected),
-			Reason: "unexpected " + ftype.String() + " frame",
+		return &connectionError{
+			code:    errH3FrameUnexpected,
+			message: "unexpected " + ftype.String() + " frame",
 		}
 	}
 	return st.discardFrame()
@@ -174,7 +177,7 @@ func (st *stream) discardFrame() error {
 	for range st.lim {
 		_, err := st.stream.ReadByte()
 		if err != nil {
-			return errH3FrameError
+			return &streamError{errH3FrameError, err.Error()}
 		}
 	}
 	st.lim = -1
@@ -250,7 +253,10 @@ func (st *stream) recordBytesRead(n int) error {
 	st.lim -= int64(n)
 	if st.lim < 0 {
 		st.stream = nil // panic if we try to read again
-		return errH3FrameError
+		return &connectionError{
+			code:    errH3FrameError,
+			message: "invalid HTTP/3 frame",
+		}
 	}
 	return nil
 }
diff --git a/internal/http3/stream_test.go b/internal/http3/stream_test.go
@@ -8,6 +8,7 @@ package http3
 
 import (
 	"bytes"
+	"errors"
 	"io"
 	"testing"
 
@@ -142,7 +143,7 @@ func TestStreamReadFrameUnderflow(t *testing.T) {
 		t.Fatalf("st.Read() = %v", err)
 	}
 	// We have not consumed the full frame: Error.
-	if err := st2.endFrame(); err != errH3FrameError {
+	if err := st2.endFrame(); !errors.Is(err, errH3FrameError) {
 		t.Fatalf("st.endFrame before end: %v, want errH3FrameError", err)
 	}
 }
@@ -178,7 +179,7 @@ func TestStreamReadFrameOverflow(t *testing.T) {
 	if _, err := st2.readFrameHeader(); err != nil {
 		t.Fatalf("st.readFrameHeader() = %v", err)
 	}
-	if _, err := io.ReadFull(st2, make([]byte, size+1)); err != errH3FrameError {
+	if _, err := io.ReadFull(st2, make([]byte, size+1)); !errors.Is(err, errH3FrameError) {
 		t.Fatalf("st.Read past end of frame: %v, want errH3FrameError", err)
 	}
 }

diff --git a/internal/http3/transport.go b/internal/http3/transport.go
@@ -148,9 +148,9 @@ func (cc *ClientConn) acceptStreams() {
 			// "Clients MUST treat receipt of a server-initiated bidirectional
 			// stream as a connection error of type H3_STREAM_CREATION_ERROR [...]"
 			// https://www.rfc-editor.org/rfc/rfc9114.html#section-6.1-3
-			cc.qconn.Abort(&quic.ApplicationError{
-				Code:   uint64(errH3StreamCreationError),
-				Reason: "server created bidirectional stream",
+			cc.abort(&connectionError{
+				code:    errH3StreamCreationError,
+				message: "server created bidirectional stream",
 			})
 			return
 		}
@@ -162,9 +162,9 @@ func (cc *ClientConn) handleStream(st *stream) {
 	// Unidirectional stream header: One varint with the stream type.
 	stype, err := st.readVarint()
 	if err != nil {
-		cc.qconn.Abort(&quic.ApplicationError{
-			Code:   uint64(errH3StreamCreationError),
-			Reason: "error reading unidirectional stream header",
+		cc.abort(&connectionError{
+			code:    errH3StreamCreationError,
+			message: "error reading unidirectional stream header",
 		})
 		return
 	}
@@ -190,13 +190,13 @@ func (cc *ClientConn) handleStream(st *stream) {
 		err = nil
 	}
 	if err == io.EOF {
-		err = &quic.ApplicationError{
-			Code:   uint64(errH3ClosedCriticalStream),
-			Reason: streamType(stype).String() + " stream closed",
+		err = &connectionError{
+			code:    errH3ClosedCriticalStream,
+			message: streamType(stype).String() + " stream closed",
 		}
 	}
 	if err != nil {
-		cc.qconn.Abort(err)
+		cc.abort(err)
 	}
 }
 
@@ -205,9 +205,9 @@ func (cc *ClientConn) checkStreamCreation(stype streamType, name string) error {
 	defer cc.mu.Unlock()
 	bit := uint8(1) << stype
 	if cc.streamsCreated&bit != 0 {
-		return &quic.ApplicationError{
-			Code:   uint64(errH3StreamCreationError),
-			Reason: "multiple " + name + " streams created",
+		return &connectionError{
+			code:    errH3StreamCreationError,
+			message: "multiple " + name + " streams created",
 		}
 	}
 	cc.streamsCreated |= bit
@@ -251,9 +251,9 @@ func (cc *ClientConn) handleControlStream(st *stream) error {
 			// greater than currently allowed on the connection,
 			// this MUST be treated as a connection error of type H3_ID_ERROR."
 			// https://www.rfc-editor.org/rfc/rfc9114.html#section-7.2.3-7
-			return &quic.ApplicationError{
-				Code:   uint64(errH3IDError),
-				Reason: "CANCEL_PUSH received when no MAX_PUSH_ID has been sent",
+			return &connectionError{
+				code:    errH3IDError,
+				message: "CANCEL_PUSH received when no MAX_PUSH_ID has been sent",
 			}
 		case frameTypeGoaway:
 			// TODO: Wait for requests to complete before closing connection.
@@ -293,8 +293,20 @@ func (cc *ClientConn) handlePushStream(*stream) error {
 	// "A client MUST treat receipt of a push stream as a connection error
 	// of type H3_ID_ERROR when no MAX_PUSH_ID frame has been sent [...]"
 	// https://www.rfc-editor.org/rfc/rfc9114.html#section-4.6-3
-	return &quic.ApplicationError{
-		Code:   uint64(errH3IDError),
-		Reason: "push stream created when no MAX_PUSH_ID has been sent",
+	return &connectionError{
+		code:    errH3IDError,
+		message: "push stream created when no MAX_PUSH_ID has been sent",
+	}
+}
+
+// abort closes the connection with an error.
+func (cc *ClientConn) abort(err error) {
+	if e, ok := err.(*connectionError); ok {
+		cc.qconn.Abort(&quic.ApplicationError{
+			Code:   uint64(e.code),
+			Reason: e.message,
+		})
+	} else {
+		cc.qconn.Abort(err)
 	}
 }