From 5bda71aec0d0242e8b2cc529863c6484c0f1f24b Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Fri, 24 Jan 2025 10:15:50 -0800 Subject: [PATCH] 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 LUCI-TryBot-Result: Go LUCI Auto-Submit: Damien Neil --- internal/http3/errors.go | 20 ++++++++++++++ internal/http3/stream.go | 18 ++++++++----- internal/http3/stream_test.go | 5 ++-- internal/http3/transport.go | 50 ++++++++++++++++++++++------------- 4 files changed, 66 insertions(+), 27 deletions(-) diff --git a/internal/http3/errors.go b/internal/http3/errors.go index 93c28075b..db46acfcc 100644 --- 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 index fc717671e..0f975407b 100644 --- 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 index 37bb0e298..12b281c55 100644 --- 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 index fc078b826..7c465117f 100644 --- 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) } }