http2: catch panics server-side, respect RST_STREAM on the Transport side

Tests in https://go-review.googlesource.com/#/c/17683/5/src/net/http/serve_test.go:
TestHandlerPanicNil_h2 and TestHandlerPanic_h2, to be re-enabled
after the copy to the main repo.

Updates golang/go#13555 (fixes the bug after the copy to the main repo)

Change-Id: I7aa3e55fb21b576ea4f94c7ed41d1ebd750ef951
Reviewed-on: https://go-review.googlesource.com/17823
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This commit is contained in:
Brad Fitzpatrick
2015-12-15 00:47:28 +00:00
parent 0d2c2e17c4
commit 74bd44bb05
3 changed files with 41 additions and 5 deletions

View File

@@ -47,6 +47,7 @@ import (
"net"
"net/http"
"net/url"
"runtime"
"strconv"
"strings"
"sync"
@@ -615,6 +616,7 @@ func (sc *serverConn) stopShutdownTimer() {
}
func (sc *serverConn) notePanic() {
// Note: this is for serverConn.serve panicking, not http.Handler code.
if testHookOnPanicMu != nil {
testHookOnPanicMu.Lock()
defer testHookOnPanicMu.Unlock()
@@ -837,6 +839,11 @@ func (sc *serverConn) startFrameWrite(wm frameWriteMsg) {
go sc.writeFrameAsync(wm)
}
// errHandlerPanicked is the error given to any callers blocked in a read from
// Request.Body when the main goroutine panics. Since most handlers read in the
// the main ServeHTTP goroutine, this will show up rarely.
var errHandlerPanicked = errors.New("http2: handler panicked")
// wroteFrame is called on the serve goroutine with the result of
// whatever happened on writeFrameAsync.
func (sc *serverConn) wroteFrame(res frameWriteResult) {
@@ -851,6 +858,10 @@ func (sc *serverConn) wroteFrame(res frameWriteResult) {
closeStream := endsStream(wm.write)
if _, ok := wm.write.(handlerPanicRST); ok {
sc.closeStream(st, errHandlerPanicked)
}
// Reply (if requested) to the blocked ServeHTTP goroutine.
if ch := wm.done; ch != nil {
select {
@@ -1530,9 +1541,25 @@ func (sc *serverConn) newWriterAndRequest() (*responseWriter, *http.Request, err
// Run on its own goroutine.
func (sc *serverConn) runHandler(rw *responseWriter, req *http.Request, handler func(http.ResponseWriter, *http.Request)) {
defer rw.handlerDone()
// TODO: catch panics like net/http.Server
didPanic := true
defer func() {
if didPanic {
e := recover()
// Same as net/http:
const size = 64 << 10
buf := make([]byte, size)
buf = buf[:runtime.Stack(buf, false)]
sc.writeFrameFromHandler(frameWriteMsg{
write: handlerPanicRST{rw.rws.stream.id},
stream: rw.rws.stream,
})
sc.logf("http2: panic serving %v: %v\n%s", sc.conn.RemoteAddr(), e, buf)
return
}
rw.handlerDone()
}()
handler(rw, req)
didPanic = false
}
func handleHeaderListTooLong(w http.ResponseWriter, r *http.Request) {
@@ -1920,9 +1947,6 @@ func (w *responseWriter) write(lenData int, dataB []byte, dataS string) (n int,
func (w *responseWriter) handlerDone() {
rws := w.rws
if rws == nil {
panic("handlerDone called twice")
}
rws.handlerDone = true
w.Flush()
w.rws = nil

View File

@@ -584,6 +584,8 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) {
case <-requestCancel(req):
cs.abortRequestBodyWrite()
return nil, errRequestCanceled
case <-cs.peerReset:
return nil, cs.resetErr
case err := <-bodyCopyErrc:
if err != nil {
return nil, err

View File

@@ -95,6 +95,16 @@ func (w *writeData) writeFrame(ctx writeContext) error {
return ctx.Framer().WriteData(w.streamID, w.endStream, w.p)
}
// handlerPanicRST is the message sent from handler goroutines when
// the handler panics.
type handlerPanicRST struct {
StreamID uint32
}
func (hp handlerPanicRST) writeFrame(ctx writeContext) error {
return ctx.Framer().WriteRSTStream(hp.StreamID, ErrCodeInternal)
}
func (se StreamError) writeFrame(ctx writeContext) error {
return ctx.Framer().WriteRSTStream(se.StreamID, se.Code)
}