internal/http3: move more common stream processing to genericConn

Move the server stream-accept loop into genericConn. (Overlooked in a previous CL.) Be more consistent about having genericConn handle errors. For golang/go#70914 Change-Id: I872673482f16539e95a1a1381ada7d3e22affb82 Reviewed-on: https://go-review.googlesource.com/c/net/+/653395 LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Damien Neil <[email protected]> Reviewed-by: Jonathan Amsterdam <[email protected]>
golang · Feb 28, 2025 · 459513d · 459513d
1 parent aad0180
commit 459513d
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 28 deletions.
diff --git a/internal/http3/conn.go b/internal/http3/conn.go
@@ -19,7 +19,7 @@ type streamHandler interface {
 	handlePushStream(*stream) error
 	handleEncoderStream(*stream) error
 	handleDecoderStream(*stream) error
-	handleRequestStream(*stream)
+	handleRequestStream(*stream) error
 	abort(error)
 }
 
@@ -43,7 +43,7 @@ func (c *genericConn) acceptStreams(qconn *quic.Conn, h streamHandler) {
 		if st.IsReadOnly() {
 			go c.handleUnidirectionalStream(newStream(st), h)
 		} else {
-			go h.handleRequestStream(newStream(st))
+			go c.handleRequestStream(newStream(st), h)
 		}
 	}
 }
@@ -81,7 +81,6 @@ func (c *genericConn) handleUnidirectionalStream(st *stream, h streamHandler) {
 		// but the quic package currently doesn't allow setting error codes
 		// for STOP_SENDING frames.
 		// TODO: Should CloseRead take an error code?
-		st.stream.CloseRead()
 		err = nil
 	}
 	if err == io.EOF {
@@ -90,8 +89,26 @@ func (c *genericConn) handleUnidirectionalStream(st *stream, h streamHandler) {
 			message: streamType(stype).String() + " stream closed",
 		}
 	}
-	if err != nil {
+	c.handleStreamError(st, h, err)
+}
+
+func (c *genericConn) handleRequestStream(st *stream, h streamHandler) {
+	c.handleStreamError(st, h, h.handleRequestStream(st))
+}
+
+func (c *genericConn) handleStreamError(st *stream, h streamHandler, err error) {
+	switch err := err.(type) {
+	case *connectionError:
 		h.abort(err)
+	case nil:
+		st.stream.CloseRead()
+		st.stream.CloseWrite()
+	case *streamError:
+		st.stream.CloseRead()
+		st.stream.Reset(uint64(err.code))
+	default:
+		st.stream.CloseRead()
+		st.stream.Reset(uint64(errH3InternalError))
 	}
 }
 

diff --git a/internal/http3/server.go b/internal/http3/server.go
@@ -86,18 +86,7 @@ func newServerConn(qconn *quic.Conn) {
 	controlStream.writeSettings()
 	controlStream.Flush()
 
-	// Accept streams on the connection.
-	for {
-		st, err := sc.qconn.AcceptStream(context.Background())
-		if err != nil {
-			return // connection closed
-		}
-		if st.IsReadOnly() {
-			go sc.handleUnidirectionalStream(newStream(st), sc)
-		} else {
-			go sc.handleRequestStream(newStream(st))
-		}
-	}
+	sc.acceptStreams(sc.qconn, sc)
 }
 
 func (sc *serverConn) handleControlStream(st *stream) error {
@@ -165,9 +154,9 @@ func (sc *serverConn) handlePushStream(*stream) error {
 	}
 }
 
-func (sc *serverConn) handleRequestStream(st *stream) {
+func (sc *serverConn) handleRequestStream(st *stream) error {
 	// TODO
-	return
+	return nil
 }
 
 // abort closes the connection with an error.

diff --git a/internal/http3/settings.go b/internal/http3/settings.go
@@ -8,7 +8,6 @@ package http3
 
 import (
 	"golang.org/x/net/internal/quic/quicwire"
-	"golang.org/x/net/quic"
 )
 
 const (
@@ -39,9 +38,9 @@ func (st *stream) writeSettings(settings ...int64) {
 func (st *stream) readSettings(f func(settingType, value int64) error) error {
 	frameType, err := st.readFrameHeader()
 	if err != nil || frameType != frameTypeSettings {
-		return &quic.ApplicationError{
-			Code:   uint64(errH3MissingSettings),
-			Reason: "settings not sent on control stream",
+		return &connectionError{
+			code:    errH3MissingSettings,
+			message: "settings not sent on control stream",
 		}
 	}
 	for st.lim > 0 {
@@ -59,9 +58,9 @@ func (st *stream) readSettings(f func(settingType, value int64) error) error {
 		// https://www.rfc-editor.org/rfc/rfc9114.html#section-7.2.4.1-5
 		switch settingsType {
 		case 0x02, 0x03, 0x04, 0x05:
-			return &quic.ApplicationError{
-				Code:   uint64(errH3SettingsError),
-				Reason: "use of reserved setting",
+			return &connectionError{
+				code:    errH3SettingsError,
+				message: "use of reserved setting",
 			}
 		}
 

diff --git a/internal/http3/transport.go b/internal/http3/transport.go
@@ -167,14 +167,14 @@ func (cc *ClientConn) handlePushStream(*stream) error {
 	}
 }
 
-func (cc *ClientConn) handleRequestStream(st *stream) {
+func (cc *ClientConn) handleRequestStream(st *stream) error {
 	// "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.abort(&connectionError{
+	return &connectionError{
 		code:    errH3StreamCreationError,
 		message: "server created bidirectional stream",
-	})
+	}
 }
 
 // abort closes the connection with an error.