http2: reject stream self-dependencies
Fix spec bug: Section 5.3.1 says that self-dependencies should be
treated as a stream error of type PROTOCOL_ERROR.
Updates golang/go#16046
Change-Id: I8b5dc8808943dc96aac0c543c7032fa989ab9e0b
Reviewed-on: https://go-review.googlesource.com/31858
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
diff --git a/http2/server.go b/http2/server.go
index 345b08c..50bc112 100644
--- a/http2/server.go
+++ b/http2/server.go
@@ -1473,9 +1473,6 @@
sc.streams[id] = st
sc.writeSched.OpenStream(st.id, OpenStreamOptions{})
- if f.HasPriority() {
- sc.writeSched.AdjustStream(st.id, f.Priority)
- }
sc.curOpenStreams++
if sc.curOpenStreams == 1 {
sc.setConnState(http.StateActive)
@@ -1498,6 +1495,12 @@
// runtime.
return streamError(st.id, ErrCodeRefusedStream)
}
+ if f.HasPriority() {
+ if err := checkPriority(f.StreamID, f.Priority); err != nil {
+ return err
+ }
+ sc.writeSched.AdjustStream(st.id, f.Priority)
+ }
rw, req, err := sc.newWriterAndRequest(st, f)
if err != nil {
@@ -1565,10 +1568,24 @@
return nil
}
+func checkPriority(streamID uint32, p PriorityParam) error {
+ if streamID == p.StreamDep {
+ // Section 5.3.1: "A stream cannot depend on itself. An endpoint MUST treat
+ // this as a stream error (Section 5.4.2) of type PROTOCOL_ERROR."
+ // Section 5.3.3 says that a stream can depend on one of its dependencies,
+ // so it's only self-dependencies that are forbidden.
+ return streamError(streamID, ErrCodeProtocol)
+ }
+ return nil
+}
+
func (sc *serverConn) processPriority(f *PriorityFrame) error {
if sc.inGoAway {
return nil
}
+ if err := checkPriority(f.StreamID, f.PriorityParam); err != nil {
+ return err
+ }
sc.writeSched.AdjustStream(f.StreamID, f.PriorityParam)
return nil
}
diff --git a/http2/server_test.go b/http2/server_test.go
index e7309b9..013abe1 100644
--- a/http2/server_test.go
+++ b/http2/server_test.go
@@ -256,6 +256,12 @@
}
}
+func (st *serverTester) writePriority(id uint32, p PriorityParam) {
+ if err := st.fr.WritePriority(id, p); err != nil {
+ st.t.Fatalf("Error writing PRIORITY: %v", err)
+ }
+}
+
func (st *serverTester) encodeHeaderField(k, v string) {
err := st.hpackEnc.WriteField(hpack.HeaderField{Name: k, Value: v})
if err != nil {
@@ -1481,6 +1487,36 @@
})
}
+// No PRIORITY on stream 0.
+func TestServer_Rejects_Priority0(t *testing.T) {
+ testServerRejectsConn(t, func(st *serverTester) {
+ st.fr.AllowIllegalWrites = true
+ st.writePriority(0, PriorityParam{StreamDep: 1})
+ })
+}
+
+// No HEADERS frame with a self-dependence.
+func TestServer_Rejects_HeadersSelfDependence(t *testing.T) {
+ testServerRejectsStream(t, ErrCodeProtocol, func(st *serverTester) {
+ st.fr.AllowIllegalWrites = true
+ st.writeHeaders(HeadersFrameParam{
+ StreamID: 1,
+ BlockFragment: st.encodeHeader(),
+ EndStream: true,
+ EndHeaders: true,
+ Priority: PriorityParam{StreamDep: 1},
+ })
+ })
+}
+
+// No PRIORTY frame with a self-dependence.
+func TestServer_Rejects_PrioritySelfDependence(t *testing.T) {
+ testServerRejectsStream(t, ErrCodeProtocol, func(st *serverTester) {
+ st.fr.AllowIllegalWrites = true
+ st.writePriority(1, PriorityParam{StreamDep: 1})
+ })
+}
+
func TestServer_Rejects_PushPromise(t *testing.T) {
testServerRejectsConn(t, func(st *serverTester) {
pp := PushPromiseParam{
