diff --git a/tools/http2_interop/s6.5.go b/tools/http2_interop/s6.5.go index 32468abe831..4295c46f73a 100644 --- a/tools/http2_interop/s6.5.go +++ b/tools/http2_interop/s6.5.go @@ -1,6 +1,7 @@ package http2interop import ( + "fmt" "time" ) @@ -30,3 +31,60 @@ func testSmallMaxFrameSize(ctx *HTTP2InteropCtx) error { return nil } + +// Section 6.5.3 says all settings frames must be acked. +func testAllSettingsFramesAcked(ctx *HTTP2InteropCtx) error { + conn, err := connect(ctx) + if err != nil { + return err + } + defer conn.Close() + conn.SetDeadline(time.Now().Add(defaultTimeout)) + + sf := &SettingsFrame{} + if err := http2Connect(conn, sf); err != nil { + return err + } + + // The spec says "The values in the SETTINGS frame MUST be processed in the order they + // appear. [...] Once all values have been processed, the recipient MUST immediately + // emit a SETTINGS frame with the ACK flag set." From my understanding, processing all + // of no values warrants an ack per frame. + for i := 0; i < 10; i++ { + if err := streamFrame(conn, sf); err != nil { + return err + } + } + + var settingsFramesReceived = 0 + // The server by default sends a settings frame as part of the handshake, and another + // after the receipt of the initial settings frame as part of our conneection preface. + // This means we expected 1 + 1 + 10 = 12 settings frames in return, with all but the + // first having the ack bit. + for settingsFramesReceived < 12 { + f, err := parseFrame(conn) + if err != nil { + return err + } + + // Other frames come down the wire too, including window update. Just ignore those. + if f, ok := f.(*SettingsFrame); ok { + settingsFramesReceived += 1 + if settingsFramesReceived == 1 { + if f.Header.Flags&SETTINGS_FLAG_ACK > 0 { + return fmt.Errorf("settings frame should not have used ack: %v") + } + continue + } + + if f.Header.Flags&SETTINGS_FLAG_ACK == 0 { + return fmt.Errorf("settings frame should have used ack: %v", f) + } + if len(f.Params) != 0 { + return fmt.Errorf("settings ack cannot have params: %v", f) + } + } + } + + return nil +} diff --git a/tools/http2_interop/s6.5_test.go b/tools/http2_interop/s6.5_test.go index 9dadd4e699c..063fd5664c8 100644 --- a/tools/http2_interop/s6.5_test.go +++ b/tools/http2_interop/s6.5_test.go @@ -13,3 +13,14 @@ func TestSoonSmallMaxFrameSize(t *testing.T) { err := testSmallMaxFrameSize(ctx) matchError(t, err, "Got goaway frame") } + +func TestSoonAllSettingsFramesAcked(t *testing.T) { + defer Report(t) + if *testCase != "framing" { + t.SkipNow() + } + ctx := InteropCtx(t) + if err := testAllSettingsFramesAcked(ctx); err != nil { + t.Fatal(err) + } +} diff --git a/tools/http2_interop/settings.go b/tools/http2_interop/settings.go index 97914d960fa..544cec01ee7 100644 --- a/tools/http2_interop/settings.go +++ b/tools/http2_interop/settings.go @@ -26,6 +26,10 @@ const ( SettingsMaxHeaderListSize SettingsIdentifier = 6 ) +const ( + SETTINGS_FLAG_ACK byte = 0x01 +) + func (si SettingsIdentifier) String() string { switch si { case SettingsHeaderTableSize: