Skip to content

Commit 459513d

Browse files
neildgopherbot
authored andcommitted
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 <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Damien Neil <dneil@google.com> Reviewed-by: Jonathan Amsterdam <jba@google.com>
1 parent aad0180 commit 459513d

File tree

4 files changed

+33
-28
lines changed

4 files changed

+33
-28
lines changed

internal/http3/conn.go

+21-4
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ type streamHandler interface {
1919
handlePushStream(*stream) error
2020
handleEncoderStream(*stream) error
2121
handleDecoderStream(*stream) error
22-
handleRequestStream(*stream)
22+
handleRequestStream(*stream) error
2323
abort(error)
2424
}
2525

@@ -43,7 +43,7 @@ func (c *genericConn) acceptStreams(qconn *quic.Conn, h streamHandler) {
4343
if st.IsReadOnly() {
4444
go c.handleUnidirectionalStream(newStream(st), h)
4545
} else {
46-
go h.handleRequestStream(newStream(st))
46+
go c.handleRequestStream(newStream(st), h)
4747
}
4848
}
4949
}
@@ -81,7 +81,6 @@ func (c *genericConn) handleUnidirectionalStream(st *stream, h streamHandler) {
8181
// but the quic package currently doesn't allow setting error codes
8282
// for STOP_SENDING frames.
8383
// TODO: Should CloseRead take an error code?
84-
st.stream.CloseRead()
8584
err = nil
8685
}
8786
if err == io.EOF {
@@ -90,8 +89,26 @@ func (c *genericConn) handleUnidirectionalStream(st *stream, h streamHandler) {
9089
message: streamType(stype).String() + " stream closed",
9190
}
9291
}
93-
if err != nil {
92+
c.handleStreamError(st, h, err)
93+
}
94+
95+
func (c *genericConn) handleRequestStream(st *stream, h streamHandler) {
96+
c.handleStreamError(st, h, h.handleRequestStream(st))
97+
}
98+
99+
func (c *genericConn) handleStreamError(st *stream, h streamHandler, err error) {
100+
switch err := err.(type) {
101+
case *connectionError:
94102
h.abort(err)
103+
case nil:
104+
st.stream.CloseRead()
105+
st.stream.CloseWrite()
106+
case *streamError:
107+
st.stream.CloseRead()
108+
st.stream.Reset(uint64(err.code))
109+
default:
110+
st.stream.CloseRead()
111+
st.stream.Reset(uint64(errH3InternalError))
95112
}
96113
}
97114

internal/http3/server.go

+3-14
Original file line numberDiff line numberDiff line change
@@ -86,18 +86,7 @@ func newServerConn(qconn *quic.Conn) {
8686
controlStream.writeSettings()
8787
controlStream.Flush()
8888

89-
// Accept streams on the connection.
90-
for {
91-
st, err := sc.qconn.AcceptStream(context.Background())
92-
if err != nil {
93-
return // connection closed
94-
}
95-
if st.IsReadOnly() {
96-
go sc.handleUnidirectionalStream(newStream(st), sc)
97-
} else {
98-
go sc.handleRequestStream(newStream(st))
99-
}
100-
}
89+
sc.acceptStreams(sc.qconn, sc)
10190
}
10291

10392
func (sc *serverConn) handleControlStream(st *stream) error {
@@ -165,9 +154,9 @@ func (sc *serverConn) handlePushStream(*stream) error {
165154
}
166155
}
167156

168-
func (sc *serverConn) handleRequestStream(st *stream) {
157+
func (sc *serverConn) handleRequestStream(st *stream) error {
169158
// TODO
170-
return
159+
return nil
171160
}
172161

173162
// abort closes the connection with an error.

internal/http3/settings.go

+6-7
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ package http3
88

99
import (
1010
"golang.org/x/net/internal/quic/quicwire"
11-
"golang.org/x/net/quic"
1211
)
1312

1413
const (
@@ -39,9 +38,9 @@ func (st *stream) writeSettings(settings ...int64) {
3938
func (st *stream) readSettings(f func(settingType, value int64) error) error {
4039
frameType, err := st.readFrameHeader()
4140
if err != nil || frameType != frameTypeSettings {
42-
return &quic.ApplicationError{
43-
Code: uint64(errH3MissingSettings),
44-
Reason: "settings not sent on control stream",
41+
return &connectionError{
42+
code: errH3MissingSettings,
43+
message: "settings not sent on control stream",
4544
}
4645
}
4746
for st.lim > 0 {
@@ -59,9 +58,9 @@ func (st *stream) readSettings(f func(settingType, value int64) error) error {
5958
// https://www.rfc-editor.org/rfc/rfc9114.html#section-7.2.4.1-5
6059
switch settingsType {
6160
case 0x02, 0x03, 0x04, 0x05:
62-
return &quic.ApplicationError{
63-
Code: uint64(errH3SettingsError),
64-
Reason: "use of reserved setting",
61+
return &connectionError{
62+
code: errH3SettingsError,
63+
message: "use of reserved setting",
6564
}
6665
}
6766

internal/http3/transport.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -167,14 +167,14 @@ func (cc *ClientConn) handlePushStream(*stream) error {
167167
}
168168
}
169169

170-
func (cc *ClientConn) handleRequestStream(st *stream) {
170+
func (cc *ClientConn) handleRequestStream(st *stream) error {
171171
// "Clients MUST treat receipt of a server-initiated bidirectional
172172
// stream as a connection error of type H3_STREAM_CREATION_ERROR [...]"
173173
// https://www.rfc-editor.org/rfc/rfc9114.html#section-6.1-3
174-
cc.abort(&connectionError{
174+
return &connectionError{
175175
code: errH3StreamCreationError,
176176
message: "server created bidirectional stream",
177-
})
177+
}
178178
}
179179

180180
// abort closes the connection with an error.

0 commit comments

Comments
 (0)