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 <[email protected]>
Run-TryBot: Brad Fitzpatrick <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>

Author: tombergan

http2/server.go

@@ -1473,9 +1473,6 @@ func (sc *serverConn) processHeaders(f *MetaHeadersFrame) error {
 
 	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 @@ func (sc *serverConn) processHeaders(f *MetaHeadersFrame) error {
 		// 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 @@ func (st *stream) processTrailerHeaders(f *MetaHeadersFrame) error {
 	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
 }

http2/server_test.go

@@ -256,6 +256,12 @@ func (st *serverTester) writeHeaders(p HeadersFrameParam) {
 	}
 }
 
+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 @@ func TestServer_Rejects_Continuation0(t *testing.T) {
 	})
 }
 
+// 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{