Invalid flags now result in a GOAWAY

This lands server change 40997047.

Also add TODOs to upstream OnControlFrameCompressed and to fix use of GMock objects.

Review URL: https://chromiumcodereview.appspot.com/12213062

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@181910 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 398 insertions, 41 deletions

diff --git a/net/spdy/spdy_framer.cc b/net/spdy/spdy_framer.cc
index 0b95e63..21615a0 100644
--- a/net/spdy/spdy_framer.cc
+++ b/net/spdy/spdy_framer.cc
@@ -207,6 +207,8 @@ const char* SpdyFramer::ErrorCodeToString(int error_code) {
       return "COMPRESS_FAILURE";
     case SPDY_INVALID_DATA_FRAME_FLAGS:
       return "SPDY_INVALID_DATA_FRAME_FLAGS";
+    case SPDY_INVALID_CONTROL_FRAME_FLAGS:
+      return "SPDY_INVALID_CONTROL_FRAME_FLAGS";
   }
   return "UNKNOWN_ERROR";
 }
@@ -407,17 +409,21 @@ size_t SpdyFramer::ProcessCommonHeader(const char* data, size_t len) {
     // if we're here, then we have the common header all received.
     if (!current_frame.is_control_frame()) {
       SpdyDataFrame data_frame(current_frame_buffer_.get(), false);
-      visitor_->OnDataFrameHeader(&data_frame);
-
-      if (current_frame.length() > 0) {
-        CHANGE_STATE(SPDY_FORWARD_STREAM_FRAME);
+      if (data_frame.flags() & ~DATA_FLAG_FIN) {
+        set_error(SPDY_INVALID_DATA_FRAME_FLAGS);
       } else {
-        // Empty data frame.
-        if (current_frame.flags() & DATA_FLAG_FIN) {
-          visitor_->OnStreamFrameData(data_frame.stream_id(),
-                                      NULL, 0, DATA_FLAG_FIN);
+        visitor_->OnDataFrameHeader(&data_frame);
+
+        if (current_frame.length() > 0) {
+          CHANGE_STATE(SPDY_FORWARD_STREAM_FRAME);
+        } else {
+          // Empty data frame.
+          if (current_frame.flags() & DATA_FLAG_FIN) {
+            visitor_->OnStreamFrameData(data_frame.stream_id(),
+                                        NULL, 0, DATA_FLAG_FIN);
+          }
+          CHANGE_STATE(SPDY_AUTO_RESET);
         }
-        CHANGE_STATE(SPDY_AUTO_RESET);
       }
     } else {
       ProcessControlFrameHeader();
@@ -459,18 +465,28 @@ void SpdyFramer::ProcessControlFrameHeader() {
   switch (current_control_frame.type()) {
     case SYN_STREAM:
       if (current_control_frame.length() <
-          SpdySynStreamControlFrame::size() - SpdyControlFrame::kHeaderSize)
+          SpdySynStreamControlFrame::size() - SpdyControlFrame::kHeaderSize) {
         set_error(SPDY_INVALID_CONTROL_FRAME);
+      } else if (current_control_frame.flags() &
+                 ~(CONTROL_FLAG_FIN | CONTROL_FLAG_UNIDIRECTIONAL)) {
+        set_error(SPDY_INVALID_CONTROL_FRAME_FLAGS);
+      }
       break;
     case SYN_REPLY:
       if (current_control_frame.length() <
-          SpdySynReplyControlFrame::size() - SpdyControlFrame::kHeaderSize)
+          SpdySynReplyControlFrame::size() - SpdyControlFrame::kHeaderSize) {
         set_error(SPDY_INVALID_CONTROL_FRAME);
+      } else if (current_control_frame.flags() & ~CONTROL_FLAG_FIN) {
+        set_error(SPDY_INVALID_CONTROL_FRAME_FLAGS);
+      }
       break;
     case RST_STREAM:
       if (current_control_frame.length() !=
-          SpdyRstStreamControlFrame::size() - SpdyFrame::kHeaderSize)
+          SpdyRstStreamControlFrame::size() - SpdyFrame::kHeaderSize) {
         set_error(SPDY_INVALID_CONTROL_FRAME);
+      } else if (current_control_frame.flags() != 0) {
+        set_error(SPDY_INVALID_CONTROL_FRAME_FLAGS);
+      }
       break;
     case SETTINGS:
       // Make sure that we have an integral number of 8-byte key/value pairs,
@@ -481,6 +497,9 @@ void SpdyFramer::ProcessControlFrameHeader() {
         DLOG(WARNING) << "Invalid length for SETTINGS frame: "
                       << current_control_frame.length();
         set_error(SPDY_INVALID_CONTROL_FRAME);
+      } else if (current_control_frame.flags() &
+                 ~SETTINGS_FLAG_CLEAR_PREVIOUSLY_PERSISTED_SETTINGS) {
+        set_error(SPDY_INVALID_CONTROL_FRAME_FLAGS);
       }
       break;
     case GOAWAY:
@@ -490,30 +509,45 @@ void SpdyFramer::ProcessControlFrameHeader() {
         // SpdyGoAwayControlFrame::size() returns the SPDY 3 size.
         const size_t goaway_offset = (protocol_version() < 3) ? 4 : 0;
         if (current_control_frame.length() + goaway_offset !=
-            SpdyGoAwayControlFrame::size() - SpdyFrame::kHeaderSize)
+            SpdyGoAwayControlFrame::size() - SpdyFrame::kHeaderSize) {
           set_error(SPDY_INVALID_CONTROL_FRAME);
+        } else if (current_control_frame.flags() != 0) {
+          set_error(SPDY_INVALID_CONTROL_FRAME_FLAGS);
+        }
         break;
       }
     case HEADERS:
       if (current_control_frame.length() <
-          SpdyHeadersControlFrame::size() - SpdyControlFrame::kHeaderSize)
+          SpdyHeadersControlFrame::size() - SpdyControlFrame::kHeaderSize) {
         set_error(SPDY_INVALID_CONTROL_FRAME);
+      } else if (current_control_frame.flags() & ~CONTROL_FLAG_FIN) {
+        set_error(SPDY_INVALID_CONTROL_FRAME_FLAGS);
+      }
       break;
     case WINDOW_UPDATE:
       if (current_control_frame.length() !=
           SpdyWindowUpdateControlFrame::size() -
-          SpdyControlFrame::kHeaderSize)
+          SpdyControlFrame::kHeaderSize) {
         set_error(SPDY_INVALID_CONTROL_FRAME);
+      } else if (current_control_frame.flags() != 0) {
+        set_error(SPDY_INVALID_CONTROL_FRAME_FLAGS);
+      }
       break;
     case PING:
       if (current_control_frame.length() !=
-          SpdyPingControlFrame::size() - SpdyControlFrame::kHeaderSize)
+          SpdyPingControlFrame::size() - SpdyControlFrame::kHeaderSize) {
         set_error(SPDY_INVALID_CONTROL_FRAME);
+      } else if (current_control_frame.flags() != 0) {
+        set_error(SPDY_INVALID_CONTROL_FRAME_FLAGS);
+      }
       break;
     case CREDENTIAL:
       if (current_control_frame.length() <
-          SpdyCredentialControlFrame::size() - SpdyControlFrame::kHeaderSize)
+          SpdyCredentialControlFrame::size() - SpdyControlFrame::kHeaderSize) {
         set_error(SPDY_INVALID_CONTROL_FRAME);
+      } else if (current_control_frame.flags() != 0) {
+        set_error(SPDY_INVALID_CONTROL_FRAME_FLAGS);
+      }
       break;
     default:
       LOG(WARNING) << "Valid " << display_protocol_
diff --git a/net/spdy/spdy_framer.h b/net/spdy/spdy_framer.h
index bca151e..a97eddb 100644
--- a/net/spdy/spdy_framer.h
+++ b/net/spdy/spdy_framer.h
@@ -210,6 +210,8 @@ class NET_EXPORT_PRIVATE SpdyFramerVisitorInterface {
 
   // Called after a control frame has been compressed to allow the visitor
   // to record compression statistics.
+  //
+  // TODO(akalin): Upstream this function.
   virtual void OnControlFrameCompressed(
       const SpdyControlFrame& uncompressed_frame,
       const SpdyControlFrame& compressed_frame) = 0;
@@ -264,14 +266,15 @@ class NET_EXPORT_PRIVATE SpdyFramer {
   // SPDY error codes.
   enum SpdyError {
     SPDY_NO_ERROR,
-    SPDY_INVALID_CONTROL_FRAME,      // Control frame is mal-formatted.
-    SPDY_CONTROL_PAYLOAD_TOO_LARGE,  // Control frame payload was too large.
-    SPDY_ZLIB_INIT_FAILURE,          // The Zlib library could not initialize.
-    SPDY_UNSUPPORTED_VERSION,        // Control frame has unsupported version.
-    SPDY_DECOMPRESS_FAILURE,         // There was an error decompressing.
-    SPDY_COMPRESS_FAILURE,           // There was an error compressing.
-    SPDY_CREDENTIAL_FRAME_CORRUPT,   // CREDENTIAL frame could not be parsed.
-    SPDY_INVALID_DATA_FRAME_FLAGS,   // Data frame has invalid flags.
+    SPDY_INVALID_CONTROL_FRAME,        // Control frame is mal-formatted.
+    SPDY_CONTROL_PAYLOAD_TOO_LARGE,    // Control frame payload was too large.
+    SPDY_ZLIB_INIT_FAILURE,            // The Zlib library could not initialize.
+    SPDY_UNSUPPORTED_VERSION,          // Control frame has unsupported version.
+    SPDY_DECOMPRESS_FAILURE,           // There was an error decompressing.
+    SPDY_COMPRESS_FAILURE,             // There was an error compressing.
+    SPDY_CREDENTIAL_FRAME_CORRUPT,     // CREDENTIAL frame could not be parsed.
+    SPDY_INVALID_DATA_FRAME_FLAGS,     // Data frame has invalid flags.
+    SPDY_INVALID_CONTROL_FRAME_FLAGS,  // Control frame has invalid flags.
 
     LAST_ERROR,  // Must be the last entry in the enum.
   };
diff --git a/net/spdy/spdy_framer_test.cc b/net/spdy/spdy_framer_test.cc
index 394586e..6462078 100644
--- a/net/spdy/spdy_framer_test.cc
+++ b/net/spdy/spdy_framer_test.cc
@@ -26,6 +26,9 @@ namespace test {
 
 static const size_t kMaxDecompressedSize = 1024;
 
+// TODO(akalin): Make sure expectations on mocks are set before mock
+// functions are called, as interleaving expectations and calls is
+// undefined.
 class MockVisitor : public SpdyFramerVisitorInterface {
  public:
   MOCK_METHOD1(OnError, void(SpdyFramer* framer));
@@ -3079,6 +3082,9 @@ TEST_P(SpdyFramerTest, ErrorCodeToStringTest) {
   EXPECT_STREQ("SPDY_INVALID_DATA_FRAME_FLAGS",
                SpdyFramer::ErrorCodeToString(
                    SpdyFramer::SPDY_INVALID_DATA_FRAME_FLAGS));
+  EXPECT_STREQ("SPDY_INVALID_CONTROL_FRAME_FLAGS",
+               SpdyFramer::ErrorCodeToString(
+                   SpdyFramer::SPDY_INVALID_CONTROL_FRAME_FLAGS));
   EXPECT_STREQ("UNKNOWN_ERROR",
                SpdyFramer::ErrorCodeToString(SpdyFramer::LAST_ERROR));
 }
@@ -3176,24 +3182,22 @@ TEST_P(SpdyFramerTest, CatchProbableHttpResponse) {
     SpdyFramer framer(spdy_version_);
     framer.set_visitor(&visitor);
 
-    // This won't cause an error at the framer level.  It will cause
-    // flag validation errors at the Visitor::OnDataFrameHeader level.
-    EXPECT_CALL(visitor, OnDataFrameHeader(_));
+    EXPECT_CALL(visitor, OnError(_));
     framer.ProcessInput("HTTP/1.1", 8);
     EXPECT_TRUE(framer.probable_http_response());
-    EXPECT_EQ(SpdyFramer::SPDY_FORWARD_STREAM_FRAME, framer.state());
+    EXPECT_EQ(SpdyFramer::SPDY_ERROR, framer.state());
+    EXPECT_EQ(SpdyFramer::SPDY_INVALID_DATA_FRAME_FLAGS, framer.error_code());
   }
   {
     testing::StrictMock<test::MockVisitor> visitor;
     SpdyFramer framer(spdy_version_);
     framer.set_visitor(&visitor);
 
-    // This won't cause an error at the framer level.  It will cause
-    // flag validation errors at the Visitor::OnDataFrameHeader level.
-    EXPECT_CALL(visitor, OnDataFrameHeader(_));
+    EXPECT_CALL(visitor, OnError(_));
     framer.ProcessInput("HTTP/1.0", 8);
     EXPECT_TRUE(framer.probable_http_response());
-    EXPECT_EQ(SpdyFramer::SPDY_FORWARD_STREAM_FRAME, framer.state());
+    EXPECT_EQ(SpdyFramer::SPDY_ERROR, framer.state());
+    EXPECT_EQ(SpdyFramer::SPDY_INVALID_DATA_FRAME_FLAGS, framer.error_code());
   }
 }
 
@@ -3209,18 +3213,333 @@ TEST_P(SpdyFramerTest, DataFrameFlags) {
         framer.CreateDataFrame(1, "hello", 5, DATA_FLAG_NONE));
     frame->set_flags(flags);
 
-    // Flags are just passed along since they need to be validated at
-    // a higher protocol layer.
-    EXPECT_CALL(visitor, OnDataFrameHeader(_));
-    EXPECT_CALL(visitor, OnStreamFrameData(_, _, 5, SpdyDataFlags()));
-    if (flags & DATA_FLAG_FIN) {
-      EXPECT_CALL(visitor, OnStreamFrameData(_, _, 0, DATA_FLAG_FIN));
+    if (flags & ~DATA_FLAG_FIN) {
+      EXPECT_CALL(visitor, OnError(_));
+    } else {
+      EXPECT_CALL(visitor, OnDataFrameHeader(_));
+      EXPECT_CALL(visitor, OnStreamFrameData(_, _, 5, SpdyDataFlags()));
+      if (flags & DATA_FLAG_FIN) {
+        EXPECT_CALL(visitor, OnStreamFrameData(_, _, 0, DATA_FLAG_FIN));
+      }
+    }
+
+    size_t frame_size = frame->length() + SpdyFrame::kHeaderSize;
+    framer.ProcessInput(frame->data(), frame_size);
+    if (flags & ~DATA_FLAG_FIN) {
+      EXPECT_EQ(SpdyFramer::SPDY_ERROR, framer.state());
+      EXPECT_EQ(SpdyFramer::SPDY_INVALID_DATA_FRAME_FLAGS,
+                framer.error_code());
+    } else {
+      EXPECT_EQ(SpdyFramer::SPDY_RESET, framer.state());
+      EXPECT_EQ(SpdyFramer::SPDY_NO_ERROR, framer.error_code());
+    }
+  }
+}
+
+TEST_P(SpdyFramerTest, SynStreamFrameFlags) {
+  for (int flags = 0; flags < 256; ++flags) {
+    SCOPED_TRACE(testing::Message() << "Flags " << flags);
+
+    testing::StrictMock<net::test::MockVisitor> visitor;
+    SpdyFramer framer(spdy_version_);
+    framer.set_visitor(&visitor);
+
+    EXPECT_CALL(visitor, OnControlFrameCompressed(_, _));
+
+    SpdyHeaderBlock headers;
+    headers["foo"] = "bar";
+    scoped_ptr<SpdyFrame> frame(
+        framer.CreateSynStream(8, 3, 1, 0, CONTROL_FLAG_NONE, true, &headers));
+    frame->set_flags(flags);
+
+    if (flags & ~(CONTROL_FLAG_FIN | CONTROL_FLAG_UNIDIRECTIONAL)) {
+      EXPECT_CALL(visitor, OnError(_));
+    } else {
+      EXPECT_CALL(visitor, OnSynStream(8, 3, 1, 0, flags & CONTROL_FLAG_FIN,
+                                       flags & CONTROL_FLAG_UNIDIRECTIONAL));
+      EXPECT_CALL(visitor, OnControlFrameHeaderData(8, _, _))
+          .WillRepeatedly(testing::Return(true));
+      if (flags & DATA_FLAG_FIN) {
+        EXPECT_CALL(visitor, OnStreamFrameData(_, _, 0, DATA_FLAG_FIN));
+      }
+    }
+
+    size_t frame_size = frame->length() + SpdyFrame::kHeaderSize;
+    framer.ProcessInput(frame->data(), frame_size);
+    if (flags & ~(CONTROL_FLAG_FIN | CONTROL_FLAG_UNIDIRECTIONAL)) {
+      EXPECT_EQ(SpdyFramer::SPDY_ERROR, framer.state());
+      EXPECT_EQ(SpdyFramer::SPDY_INVALID_CONTROL_FRAME_FLAGS,
+                framer.error_code());
+    } else {
+      EXPECT_EQ(SpdyFramer::SPDY_RESET, framer.state());
+      EXPECT_EQ(SpdyFramer::SPDY_NO_ERROR, framer.error_code());
+    }
+  }
+}
+
+TEST_P(SpdyFramerTest, SynReplyFrameFlags) {
+  for (int flags = 0; flags < 256; ++flags) {
+    SCOPED_TRACE(testing::Message() << "Flags " << flags);
+
+    testing::StrictMock<net::test::MockVisitor> visitor;
+    SpdyFramer framer(spdy_version_);
+    framer.set_visitor(&visitor);
+
+    EXPECT_CALL(visitor, OnControlFrameCompressed(_, _));
+
+    SpdyHeaderBlock headers;
+    headers["foo"] = "bar";
+    scoped_ptr<SpdyFrame> frame(
+        framer.CreateSynReply(37, CONTROL_FLAG_NONE, true, &headers));
+    frame->set_flags(flags);
+
+    if (flags & ~CONTROL_FLAG_FIN) {
+      EXPECT_CALL(visitor, OnError(_));
+    } else {
+      EXPECT_CALL(visitor, OnSynReply(37, flags & CONTROL_FLAG_FIN));
+      EXPECT_CALL(visitor, OnControlFrameHeaderData(37, _, _))
+          .WillRepeatedly(testing::Return(true));
+      if (flags & DATA_FLAG_FIN) {
+        EXPECT_CALL(visitor, OnStreamFrameData(_, _, 0, DATA_FLAG_FIN));
+      }
+    }
+
+    size_t frame_size = frame->length() + SpdyFrame::kHeaderSize;
+    framer.ProcessInput(frame->data(), frame_size);
+    if (flags & ~CONTROL_FLAG_FIN) {
+      EXPECT_EQ(SpdyFramer::SPDY_ERROR, framer.state());
+      EXPECT_EQ(SpdyFramer::SPDY_INVALID_CONTROL_FRAME_FLAGS,
+                framer.error_code());
+    } else {
+      EXPECT_EQ(SpdyFramer::SPDY_RESET, framer.state());
+      EXPECT_EQ(SpdyFramer::SPDY_NO_ERROR, framer.error_code());
+    }
+  }
+}
+
+TEST_P(SpdyFramerTest, RstStreamFrameFlags) {
+  for (int flags = 0; flags < 256; ++flags) {
+    SCOPED_TRACE(testing::Message() << "Flags " << flags);
+
+    testing::StrictMock<net::test::MockVisitor> visitor;
+    SpdyFramer framer(spdy_version_);
+    framer.set_visitor(&visitor);
+
+    scoped_ptr<SpdyFrame> frame(framer.CreateRstStream(13, RST_STREAM_CANCEL));
+    frame->set_flags(flags);
+
+    if (flags != 0) {
+      EXPECT_CALL(visitor, OnError(_));
+    } else {
+      EXPECT_CALL(visitor, OnRstStream(13, RST_STREAM_CANCEL));
+    }
+
+    size_t frame_size = frame->length() + SpdyFrame::kHeaderSize;
+    framer.ProcessInput(frame->data(), frame_size);
+    if (flags != 0) {
+      EXPECT_EQ(SpdyFramer::SPDY_ERROR, framer.state());
+      EXPECT_EQ(SpdyFramer::SPDY_INVALID_CONTROL_FRAME_FLAGS,
+                framer.error_code());
+    } else {
+      EXPECT_EQ(SpdyFramer::SPDY_RESET, framer.state());
+      EXPECT_EQ(SpdyFramer::SPDY_NO_ERROR, framer.error_code());
+    }
+  }
+}
+
+TEST_P(SpdyFramerTest, SettingsFrameFlags) {
+  for (int flags = 0; flags < 256; ++flags) {
+    SCOPED_TRACE(testing::Message() << "Flags " << flags);
+
+    testing::StrictMock<net::test::MockVisitor> visitor;
+    SpdyFramer framer(spdy_version_);
+    framer.set_visitor(&visitor);
+
+    SettingsMap settings;
+    settings[SETTINGS_UPLOAD_BANDWIDTH] =
+        std::make_pair(SETTINGS_FLAG_NONE, 54321);
+    scoped_ptr<SpdyFrame> frame(framer.CreateSettings(settings));
+    frame->set_flags(flags);
+
+    if (flags & ~SETTINGS_FLAG_CLEAR_PREVIOUSLY_PERSISTED_SETTINGS) {
+      EXPECT_CALL(visitor, OnError(_));
+    } else {
+      EXPECT_CALL(visitor, OnSetting(SETTINGS_UPLOAD_BANDWIDTH,
+                                     SETTINGS_FLAG_NONE, 54321));
+    }
+
+    size_t frame_size = frame->length() + SpdyFrame::kHeaderSize;
+    framer.ProcessInput(frame->data(), frame_size);
+    if (flags & ~SETTINGS_FLAG_CLEAR_PREVIOUSLY_PERSISTED_SETTINGS) {
+      EXPECT_EQ(SpdyFramer::SPDY_ERROR, framer.state());
+      EXPECT_EQ(SpdyFramer::SPDY_INVALID_CONTROL_FRAME_FLAGS,
+                framer.error_code());
+    } else {
+      EXPECT_EQ(SpdyFramer::SPDY_RESET, framer.state());
+      EXPECT_EQ(SpdyFramer::SPDY_NO_ERROR, framer.error_code());
+    }
+  }
+}
+
+TEST_P(SpdyFramerTest, GoawayFrameFlags) {
+  for (int flags = 0; flags < 256; ++flags) {
+    SCOPED_TRACE(testing::Message() << "Flags " << flags);
+
+    testing::StrictMock<net::test::MockVisitor> visitor;
+    SpdyFramer framer(spdy_version_);
+    framer.set_visitor(&visitor);
+
+    scoped_ptr<SpdyFrame> frame(framer.CreateGoAway(97, GOAWAY_OK));
+    frame->set_flags(flags);
+
+    if (flags != 0) {
+      EXPECT_CALL(visitor, OnError(_));
+    } else {
+      EXPECT_CALL(visitor, OnGoAway(97, GOAWAY_OK));
     }
 
     size_t frame_size = frame->length() + SpdyFrame::kHeaderSize;
     framer.ProcessInput(frame->data(), frame_size);
-    EXPECT_EQ(SpdyFramer::SPDY_RESET, framer.state());
-    EXPECT_EQ(SpdyFramer::SPDY_NO_ERROR, framer.error_code());
+    if (flags != 0) {
+      EXPECT_EQ(SpdyFramer::SPDY_ERROR, framer.state());
+      EXPECT_EQ(SpdyFramer::SPDY_INVALID_CONTROL_FRAME_FLAGS,
+                framer.error_code());
+    } else {
+      EXPECT_EQ(SpdyFramer::SPDY_RESET, framer.state());
+      EXPECT_EQ(SpdyFramer::SPDY_NO_ERROR, framer.error_code());
+    }
+  }
+}
+
+TEST_P(SpdyFramerTest, HeadersFrameFlags) {
+  for (int flags = 0; flags < 256; ++flags) {
+    SCOPED_TRACE(testing::Message() << "Flags " << flags);
+
+    testing::StrictMock<net::test::MockVisitor> visitor;
+    SpdyFramer framer(spdy_version_);
+    framer.set_visitor(&visitor);
+
+    EXPECT_CALL(visitor, OnControlFrameCompressed(_, _));
+
+    SpdyHeaderBlock headers;
+    headers["foo"] = "bar";
+    scoped_ptr<SpdyFrame> frame(
+        framer.CreateHeaders(57, CONTROL_FLAG_NONE, true, &headers));
+    frame->set_flags(flags);
+
+    if (flags & ~CONTROL_FLAG_FIN) {
+      EXPECT_CALL(visitor, OnError(_));
+    } else {
+      EXPECT_CALL(visitor, OnHeaders(57, flags & CONTROL_FLAG_FIN));
+      EXPECT_CALL(visitor, OnControlFrameHeaderData(57, _, _))
+          .WillRepeatedly(testing::Return(true));
+      if (flags & DATA_FLAG_FIN) {
+        EXPECT_CALL(visitor, OnStreamFrameData(_, _, 0, DATA_FLAG_FIN));
+      }
+    }
+
+    size_t frame_size = frame->length() + SpdyFrame::kHeaderSize;
+    framer.ProcessInput(frame->data(), frame_size);
+    if (flags & ~CONTROL_FLAG_FIN) {
+      EXPECT_EQ(SpdyFramer::SPDY_ERROR, framer.state());
+      EXPECT_EQ(SpdyFramer::SPDY_INVALID_CONTROL_FRAME_FLAGS,
+                framer.error_code());
+    } else {
+      EXPECT_EQ(SpdyFramer::SPDY_RESET, framer.state());
+      EXPECT_EQ(SpdyFramer::SPDY_NO_ERROR, framer.error_code());
+    }
+  }
+}
+
+TEST_P(SpdyFramerTest, PingFrameFlags) {
+  for (int flags = 0; flags < 256; ++flags) {
+    SCOPED_TRACE(testing::Message() << "Flags " << flags);
+
+    testing::StrictMock<net::test::MockVisitor> visitor;
+    SpdyFramer framer(spdy_version_);
+    framer.set_visitor(&visitor);
+
+    scoped_ptr<SpdyFrame> frame(framer.CreatePingFrame(42));
+    frame->set_flags(flags);
+
+    if (flags != 0) {
+      EXPECT_CALL(visitor, OnError(_));
+    } else {
+      EXPECT_CALL(visitor, OnPing(42));
+    }
+
+    size_t frame_size = frame->length() + SpdyFrame::kHeaderSize;
+    framer.ProcessInput(frame->data(), frame_size);
+    if (flags != 0) {
+      EXPECT_EQ(SpdyFramer::SPDY_ERROR, framer.state());
+      EXPECT_EQ(SpdyFramer::SPDY_INVALID_CONTROL_FRAME_FLAGS,
+                framer.error_code());
+    } else {
+      EXPECT_EQ(SpdyFramer::SPDY_RESET, framer.state());
+      EXPECT_EQ(SpdyFramer::SPDY_NO_ERROR, framer.error_code());
+    }
+  }
+}
+
+TEST_P(SpdyFramerTest, WindowUpdateFrameFlags) {
+  for (int flags = 0; flags < 256; ++flags) {
+    SCOPED_TRACE(testing::Message() << "Flags " << flags);
+
+    testing::StrictMock<net::test::MockVisitor> visitor;
+    SpdyFramer framer(spdy_version_);
+    framer.set_visitor(&visitor);
+
+    scoped_ptr<SpdyFrame> frame(framer.CreateWindowUpdate(4, 1024));
+    frame->set_flags(flags);
+
+    if (flags != 0) {
+      EXPECT_CALL(visitor, OnError(_));
+    } else {
+      EXPECT_CALL(visitor, OnWindowUpdate(4, 1024));
+    }
+
+    size_t frame_size = frame->length() + SpdyFrame::kHeaderSize;
+    framer.ProcessInput(frame->data(), frame_size);
+    if (flags != 0) {
+      EXPECT_EQ(SpdyFramer::SPDY_ERROR, framer.state());
+      EXPECT_EQ(SpdyFramer::SPDY_INVALID_CONTROL_FRAME_FLAGS,
+                framer.error_code());
+    } else {
+      EXPECT_EQ(SpdyFramer::SPDY_RESET, framer.state());
+      EXPECT_EQ(SpdyFramer::SPDY_NO_ERROR, framer.error_code());
+    }
+  }
+}
+
+TEST_P(SpdyFramerTest, CredentialFrameFlags) {
+  for (int flags = 0; flags < 256; ++flags) {
+    SCOPED_TRACE(testing::Message() << "Flags " << flags);
+
+    testing::StrictMock<net::test::MockVisitor> visitor;
+    SpdyFramer framer(spdy_version_);
+    framer.set_visitor(&visitor);
+
+    SpdyCredential credential;
+    scoped_ptr<SpdyFrame> frame(framer.CreateCredentialFrame(credential));
+    frame->set_flags(flags);
+
+    if (flags != 0) {
+      EXPECT_CALL(visitor, OnError(_));
+    } else {
+      EXPECT_CALL(visitor, OnCredentialFrameData(_, _))
+          .WillRepeatedly(testing::Return(true));
+    }
+
+    size_t frame_size = frame->length() + SpdyFrame::kHeaderSize;
+    framer.ProcessInput(frame->data(), frame_size);
+    if (flags != 0) {
+      EXPECT_EQ(SpdyFramer::SPDY_ERROR, framer.state());
+      EXPECT_EQ(SpdyFramer::SPDY_INVALID_CONTROL_FRAME_FLAGS,
+                framer.error_code());
+    } else {
+      EXPECT_EQ(SpdyFramer::SPDY_RESET, framer.state());
+      EXPECT_EQ(SpdyFramer::SPDY_NO_ERROR, framer.error_code());
+    }
   }
 }
 
diff --git a/net/spdy/spdy_session.h b/net/spdy/spdy_session.h
index c0b5d15..2ce3e14 100644
--- a/net/spdy/spdy_session.h
+++ b/net/spdy/spdy_session.h
@@ -63,6 +63,7 @@ enum SpdyProtocolErrorDetails {
   SPDY_ERROR_COMPRESS_FAILURE,
   SPDY_ERROR_CREDENTIAL_FRAME_CORRUPT,
   SPDY_ERROR_INVALID_DATA_FRAME_FLAGS,
+  SPDY_ERROR_INVALID_CONTROL_FRAME_FLAGS,
 
   // SpdyRstStreamStatus
   STATUS_CODE_INVALID,