Browse Source

http2: allow StreamDep of 0 in HEADERS and PRIORITY frames

But still check that the high bit isn't set.

Fixes golang/go#15444

Change-Id: Ic9339244d3d0c4e4d6573a691b3ba37c9eac3981
Reviewed-on: https://go-review.googlesource.com/22476
Reviewed-by: Andrew Gerrand <adg@golang.org>
Brad Fitzpatrick 9 years ago
parent
commit
cb0ed7acc4
2 changed files with 62 additions and 3 deletions
  1. 13 3
      http2/frame.go
  2. 49 0
      http2/frame_test.go

+ 13 - 3
http2/frame.go

@@ -590,7 +590,14 @@ func parseDataFrame(fh FrameHeader, payload []byte) (Frame, error) {
 	return f, nil
 	return f, nil
 }
 }
 
 
-var errStreamID = errors.New("invalid streamid")
+var (
+	errStreamID    = errors.New("invalid stream ID")
+	errDepStreamID = errors.New("invalid dependent stream ID")
+)
+
+func validStreamIDOrZero(streamID uint32) bool {
+	return streamID&(1<<31) == 0
+}
 
 
 func validStreamID(streamID uint32) bool {
 func validStreamID(streamID uint32) bool {
 	return streamID != 0 && streamID&(1<<31) == 0
 	return streamID != 0 && streamID&(1<<31) == 0
@@ -977,8 +984,8 @@ func (f *Framer) WriteHeaders(p HeadersFrameParam) error {
 	}
 	}
 	if !p.Priority.IsZero() {
 	if !p.Priority.IsZero() {
 		v := p.Priority.StreamDep
 		v := p.Priority.StreamDep
-		if !validStreamID(v) && !f.AllowIllegalWrites {
-			return errors.New("invalid dependent stream id")
+		if !validStreamIDOrZero(v) && !f.AllowIllegalWrites {
+			return errDepStreamID
 		}
 		}
 		if p.Priority.Exclusive {
 		if p.Priority.Exclusive {
 			v |= 1 << 31
 			v |= 1 << 31
@@ -1046,6 +1053,9 @@ func (f *Framer) WritePriority(streamID uint32, p PriorityParam) error {
 	if !validStreamID(streamID) && !f.AllowIllegalWrites {
 	if !validStreamID(streamID) && !f.AllowIllegalWrites {
 		return errStreamID
 		return errStreamID
 	}
 	}
+	if !validStreamIDOrZero(p.StreamDep) {
+		return errDepStreamID
+	}
 	f.startWrite(FramePriority, 0, streamID)
 	f.startWrite(FramePriority, 0, streamID)
 	v := p.StreamDep
 	v := p.StreamDep
 	if p.Exclusive {
 	if p.Exclusive {

+ 49 - 0
http2/frame_test.go

@@ -202,6 +202,37 @@ func TestWriteHeaders(t *testing.T) {
 				headerFragBuf: []byte("abc"),
 				headerFragBuf: []byte("abc"),
 			},
 			},
 		},
 		},
+		{
+			"with priority stream dep zero", // golang.org/issue/15444
+			HeadersFrameParam{
+				StreamID:      42,
+				BlockFragment: []byte("abc"),
+				EndStream:     true,
+				EndHeaders:    true,
+				PadLength:     2,
+				Priority: PriorityParam{
+					StreamDep: 0,
+					Exclusive: true,
+					Weight:    127,
+				},
+			},
+			"\x00\x00\v\x01-\x00\x00\x00*\x02\x80\x00\x00\x00\u007fabc\x00\x00",
+			&HeadersFrame{
+				FrameHeader: FrameHeader{
+					valid:    true,
+					StreamID: 42,
+					Type:     FrameHeaders,
+					Flags:    FlagHeadersEndStream | FlagHeadersEndHeaders | FlagHeadersPadded | FlagHeadersPriority,
+					Length:   uint32(1 + 5 + len("abc") + 2), // pad length + priority + contents + padding
+				},
+				Priority: PriorityParam{
+					StreamDep: 0,
+					Exclusive: true,
+					Weight:    127,
+				},
+				headerFragBuf: []byte("abc"),
+			},
+		},
 	}
 	}
 	for _, tt := range tests {
 	for _, tt := range tests {
 		fr, buf := testFramer()
 		fr, buf := testFramer()
@@ -223,6 +254,24 @@ func TestWriteHeaders(t *testing.T) {
 	}
 	}
 }
 }
 
 
+func TestWriteInvalidStreamDep(t *testing.T) {
+	fr, _ := testFramer()
+	err := fr.WriteHeaders(HeadersFrameParam{
+		StreamID: 42,
+		Priority: PriorityParam{
+			StreamDep: 1 << 31,
+		},
+	})
+	if err != errDepStreamID {
+		t.Errorf("header error = %v; want %q", err, errDepStreamID)
+	}
+
+	err = fr.WritePriority(2, PriorityParam{StreamDep: 1 << 31})
+	if err != errDepStreamID {
+		t.Errorf("priority error = %v; want %q", err, errDepStreamID)
+	}
+}
+
 func TestWriteContinuation(t *testing.T) {
 func TestWriteContinuation(t *testing.T) {
 	const streamID = 42
 	const streamID = 42
 	tests := []struct {
 	tests := []struct {