Skip to content

Commit d1e1b35

Browse files
vcabbagebradfitz
authored andcommitted
http2: enforce write deadline per stream
If the writeDeadline elapses RST_STREAM is sent with ErrCodeInternal. Fixes tests added in https://golang.org/cl/34726 Updates golang/go#18437 (fixes once it's bundled into net/http) Change-Id: I84e4f76753963c29cd3c06730d6bfbb7f1ee6051 Reviewed-on: https://go-review.googlesource.com/34727 Reviewed-by: Brad Fitzpatrick <[email protected]> Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
1 parent ffcf1be commit d1e1b35

File tree

1 file changed

+23
-7
lines changed

1 file changed

+23
-7
lines changed

http2/server.go

+23-7
Original file line numberDiff line numberDiff line change
@@ -307,10 +307,9 @@ func (s *Server) ServeConn(c net.Conn, opts *ServeConnOpts) {
307307

308308
// The net/http package sets the write deadline from the
309309
// http.Server.WriteTimeout during the TLS handshake, but then
310-
// passes the connection off to us with the deadline already
311-
// set. Disarm it here so that it is not applied to additional
312-
// streams opened on this connection.
313-
// TODO: implement WriteTimeout fully. See Issue 18437.
310+
// passes the connection off to us with the deadline already set.
311+
// Write deadlines are set per stream in serverConn.newStream.
312+
// Disarm the net.Conn write deadline here.
314313
if sc.hs.WriteTimeout != 0 {
315314
sc.conn.SetWriteDeadline(time.Time{})
316315
}
@@ -493,9 +492,10 @@ type stream struct {
493492
numTrailerValues int64
494493
weight uint8
495494
state streamState
496-
resetQueued bool // RST_STREAM queued for write; set by sc.resetStream
497-
gotTrailerHeader bool // HEADER frame for trailers was seen
498-
wroteHeaders bool // whether we wrote headers (not status 100)
495+
resetQueued bool // RST_STREAM queued for write; set by sc.resetStream
496+
gotTrailerHeader bool // HEADER frame for trailers was seen
497+
wroteHeaders bool // whether we wrote headers (not status 100)
498+
writeDeadline *time.Timer // nil if unused
499499

500500
trailer http.Header // accumulated trailers
501501
reqTrailer http.Header // handler's Request.Trailer
@@ -766,6 +766,10 @@ func (sc *serverConn) serve() {
766766
loopNum++
767767
select {
768768
case wr := <-sc.wantWriteFrameCh:
769+
if se, ok := wr.write.(StreamError); ok {
770+
sc.resetStream(se)
771+
break
772+
}
769773
sc.writeFrame(wr)
770774
case spr := <-sc.wantStartPushCh:
771775
sc.startPush(spr)
@@ -1336,6 +1340,9 @@ func (sc *serverConn) closeStream(st *stream, err error) {
13361340
panic(fmt.Sprintf("invariant; can't close stream in state %v", st.state))
13371341
}
13381342
st.state = stateClosed
1343+
if st.writeDeadline != nil {
1344+
st.writeDeadline.Stop()
1345+
}
13391346
if st.isPushed() {
13401347
sc.curPushedStreams--
13411348
} else {
@@ -1574,6 +1581,12 @@ func (st *stream) copyTrailersToHandlerRequest() {
15741581
}
15751582
}
15761583

1584+
// onWriteTimeout is run on its own goroutine (from time.AfterFunc)
1585+
// when the stream's WriteTimeout has fired.
1586+
func (st *stream) onWriteTimeout() {
1587+
st.sc.writeFrameFromHandler(FrameWriteRequest{write: streamError(st.id, ErrCodeInternal)})
1588+
}
1589+
15771590
func (sc *serverConn) processHeaders(f *MetaHeadersFrame) error {
15781591
sc.serveG.check()
15791592
id := f.StreamID
@@ -1753,6 +1766,9 @@ func (sc *serverConn) newStream(id, pusherID uint32, state streamState) *stream
17531766
st.flow.add(sc.initialStreamSendWindowSize)
17541767
st.inflow.conn = &sc.inflow // link to conn-level counter
17551768
st.inflow.add(sc.srv.initialStreamRecvWindowSize())
1769+
if sc.hs.WriteTimeout != 0 {
1770+
st.writeDeadline = time.AfterFunc(sc.hs.WriteTimeout, st.onWriteTimeout)
1771+
}
17561772

17571773
sc.streams[id] = st
17581774
sc.writeSched.OpenStream(st.id, OpenStreamOptions{PusherID: pusherID})

0 commit comments

Comments
 (0)