Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(959)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: net/spdy/spdy_framer_test.cc

Issue 1997863002: Revert of Avoids the "re-encode HPACK as SPDY3" step. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 4 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « net/spdy/spdy_framer.cc ('k') | net/spdy/spdy_network_transaction_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/spdy/spdy_framer_test.cc
diff --git a/net/spdy/spdy_framer_test.cc b/net/spdy/spdy_framer_test.cc
index b4429a2da893a1ca2dab72d7b59130e26cc445ab..1ce33951a21fdf27fc0de1af39e80b9d76b2617d 100644
--- a/net/spdy/spdy_framer_test.cc
+++ b/net/spdy/spdy_framer_test.cc
@@ -60,91 +60,33 @@
template <class SpdyFrameType>
static SpdySerializedFrame DecompressFrame(SpdyFramer* framer,
const SpdyFrameType& frame) {
- NewDecompressionVisitor visitor;
+ DecompressionVisitor visitor(framer->protocol_version());
framer->set_visitor(&visitor);
CHECK_EQ(frame.size(), framer->ProcessInput(frame.data(), frame.size()));
CHECK_EQ(SpdyFramer::SPDY_READY_FOR_FRAME, framer->state());
- framer->set_visitor(nullptr);
- SpdyFramer serializer(framer->protocol_version());
- serializer.set_enable_compression(false);
- return serializer.SerializeFrame(visitor.GetFrame());
- }
-
- class NewDecompressionVisitor : public SpdyFramerVisitorInterface {
+ framer->set_visitor(NULL);
+
+ char* buffer = visitor.ReleaseBuffer();
+ CHECK(buffer != NULL);
+ SpdySerializedFrame decompressed_frame(buffer, visitor.size(), true);
+ SetFrameLength(&decompressed_frame,
+ visitor.size() - framer->GetControlFrameHeaderSize(),
+ framer->protocol_version());
+ return decompressed_frame;
+ }
+
+ class DecompressionVisitor : public SpdyFramerVisitorInterface {
public:
- NewDecompressionVisitor() : finished_(false) {}
-
- const SpdyFrameIR& GetFrame() const {
- CHECK(finished_);
- return *frame_;
- }
-
- SpdyHeadersHandlerInterface* OnHeaderFrameStart(
- SpdyStreamId stream_id) override {
- if (headers_handler_ == nullptr) {
- headers_handler_.reset(new TestHeadersHandler);
- }
- return headers_handler_.get();
- }
-
- void OnHeaderFrameEnd(SpdyStreamId stream_id, bool end_headers) override {
+ explicit DecompressionVisitor(SpdyMajorVersion version)
+ : version_(version), size_(0), finished_(false) {}
+
+ void ResetBuffer() {
+ CHECK(buffer_.get() == NULL);
+ CHECK_EQ(0u, size_);
CHECK(!finished_);
- frame_->set_header_block(headers_handler_->decoded_block());
- finished_ = true;
- if (end_headers) {
- headers_handler_.reset();
- }
- }
-
- void OnSynStream(SpdyStreamId stream_id,
- SpdyStreamId associated_stream_id,
- SpdyPriority priority,
- bool fin,
- bool unidirectional) override {
- SpdySynStreamIR* syn_stream = new SpdySynStreamIR(stream_id);
- syn_stream->set_associated_to_stream_id(associated_stream_id);
- syn_stream->set_priority(priority);
- syn_stream->set_fin(fin);
- syn_stream->set_unidirectional(unidirectional);
- frame_.reset(syn_stream);
- }
-
- void OnSynReply(SpdyStreamId stream_id, bool fin) override {
- SpdyHeadersIR* headers = new SpdyHeadersIR(stream_id);
- headers->set_fin(fin);
- frame_.reset(headers);
- }
-
- void OnHeaders(SpdyStreamId stream_id,
- bool has_priority,
- SpdyPriority priority,
- SpdyStreamId parent_stream_id,
- bool exclusive,
- bool fin,
- bool end) override {
- SpdyHeadersIR* headers = new SpdyHeadersIR(stream_id);
- headers->set_has_priority(has_priority);
- headers->set_priority(priority);
- headers->set_parent_stream_id(parent_stream_id);
- headers->set_exclusive(exclusive);
- headers->set_fin(fin);
- frame_.reset(headers);
- }
-
- void OnPushPromise(SpdyStreamId stream_id,
- SpdyStreamId promised_stream_id,
- bool end) override {
- SpdyPushPromiseIR* push_promise =
- new SpdyPushPromiseIR(stream_id, promised_stream_id);
- frame_.reset(push_promise);
- }
-
- // TODO(birenroy): Add support for CONTINUATION.
- void OnContinuation(SpdyStreamId stream_id, bool end) override {
- LOG(FATAL);
- }
-
- // All other methods just LOG(FATAL).
+ buffer_.reset(new char[kMaxDecompressedSize]);
+ }
+
void OnError(SpdyFramer* framer) override { LOG(FATAL); }
void OnDataFrameHeader(SpdyStreamId stream_id,
size_t length,
@@ -163,90 +105,14 @@
LOG(FATAL);
}
- bool OnControlFrameHeaderData(SpdyStreamId stream_id,
- const char* header_data,
- size_t len) override {
- LOG(FATAL);
- return true;
- }
-
- void OnRstStream(SpdyStreamId stream_id,
- SpdyRstStreamStatus status) override {
- LOG(FATAL);
- }
- void OnSetting(SpdySettingsIds id, uint8_t flags, uint32_t value) override {
- LOG(FATAL);
- }
- void OnPing(SpdyPingId unique_id, bool is_ack) override { LOG(FATAL); }
- void OnSettingsEnd() override { LOG(FATAL); }
- void OnGoAway(SpdyStreamId last_accepted_stream_id,
- SpdyGoAwayStatus status) override {
- LOG(FATAL);
- }
-
- void OnWindowUpdate(SpdyStreamId stream_id,
- int delta_window_size) override {
- LOG(FATAL);
- }
-
- void OnPriority(SpdyStreamId stream_id,
- SpdyStreamId parent_stream_id,
- uint8_t weight,
- bool exclusive) override {
- // Do nothing.
- }
-
- bool OnUnknownFrame(SpdyStreamId stream_id, int frame_type) override {
- LOG(FATAL);
- return false;
- }
-
- private:
- std::unique_ptr<TestHeadersHandler> headers_handler_;
- std::unique_ptr<SpdyFrameWithHeaderBlockIR> frame_;
- bool finished_;
-
- DISALLOW_COPY_AND_ASSIGN(NewDecompressionVisitor);
- };
-
- class DecompressionVisitor : public SpdyFramerVisitorInterface {
- public:
- explicit DecompressionVisitor(SpdyMajorVersion version)
- : version_(version), size_(0), finished_(false) {}
-
- void ResetBuffer() {
- CHECK(buffer_.get() == NULL);
- CHECK_EQ(0u, size_);
- CHECK(!finished_);
- buffer_.reset(new char[kMaxDecompressedSize]);
- }
-
- void OnError(SpdyFramer* framer) override { LOG(FATAL); }
- void OnDataFrameHeader(SpdyStreamId stream_id,
- size_t length,
- bool fin) override {
- LOG(FATAL) << "Unexpected data frame header";
- }
- void OnStreamFrameData(SpdyStreamId stream_id,
- const char* data,
- size_t len) override {
- LOG(FATAL);
- }
-
- void OnStreamEnd(SpdyStreamId stream_id) override { LOG(FATAL); }
-
- void OnStreamPadding(SpdyStreamId stream_id, size_t len) override {
- LOG(FATAL);
- }
-
SpdyHeadersHandlerInterface* OnHeaderFrameStart(
SpdyStreamId stream_id) override {
- LOG(FATAL) << "Not implemented.";
+ LOG(FATAL);
return nullptr;
}
void OnHeaderFrameEnd(SpdyStreamId stream_id, bool end_headers) override {
- LOG(FATAL) << "Not implemented.";
+ LOG(FATAL);
}
bool OnControlFrameHeaderData(SpdyStreamId stream_id,
@@ -374,7 +240,6 @@
private:
SpdyMajorVersion version_;
- TestHeadersHandler* headers_handler_;
std::unique_ptr<char[]> buffer_;
size_t size_;
bool finished_;
@@ -384,20 +249,6 @@
private:
DISALLOW_COPY_AND_ASSIGN(SpdyFramerTestUtil);
-};
-
-class SpdyFramerPeer {
- public:
- static size_t ControlFrameBufferSize() {
- return SpdyFramer::kControlFrameBufferSize;
- }
- static size_t GetNumberRequiredContinuationFrames(SpdyFramer* framer,
- size_t size) {
- return framer->GetNumberRequiredContinuationFrames(size);
- }
- static void SetError(SpdyFramer* framer, SpdyFramer::SpdyError error) {
- framer->set_error(error);
- }
};
class TestSpdyVisitor : public SpdyFramerVisitorInterface,
@@ -484,19 +335,13 @@
SpdyHeadersHandlerInterface* OnHeaderFrameStart(
SpdyStreamId stream_id) override {
- if (headers_handler_ == nullptr) {
- headers_handler_.reset(new TestHeadersHandler);
- }
- return headers_handler_.get();
+ LOG(FATAL) << "OnHeaderFrameStart(" << stream_id << ")";
+ return nullptr;
}
void OnHeaderFrameEnd(SpdyStreamId stream_id, bool end_headers) override {
- CHECK(headers_handler_ != nullptr);
- headers_ = headers_handler_->decoded_block();
- header_bytes_received_ = headers_handler_->header_bytes_parsed();
- if (end_headers) {
- headers_handler_.reset();
- }
+ LOG(FATAL) << "OnHeaderFrameEnd(" << stream_id << ", " << end_headers
+ << ")";
}
bool OnControlFrameHeaderData(SpdyStreamId stream_id,
@@ -786,15 +631,24 @@
std::unique_ptr<char[]> header_buffer_;
size_t header_buffer_length_;
size_t header_buffer_size_;
- size_t header_bytes_received_;
SpdyStreamId header_stream_id_;
SpdyFrameType header_control_type_;
bool header_buffer_valid_;
- std::unique_ptr<TestHeadersHandler> headers_handler_;
SpdyHeaderBlock headers_;
bool header_has_priority_;
SpdyStreamId header_parent_stream_id_;
bool header_exclusive_;
+};
+
+class SpdyFramerPeer {
+ public:
+ static size_t ControlFrameBufferSize() {
+ return SpdyFramer::kControlFrameBufferSize;
+ }
+ static size_t GetNumberRequiredContinuationFrames(SpdyFramer* framer,
+ size_t size) {
+ return framer->GetNumberRequiredContinuationFrames(size);
+ }
};
// Retrieves serialized headers from a HEADERS or SYN_STREAM frame.
@@ -894,7 +748,7 @@
visitor.SimulateInFramer(reinterpret_cast<unsigned char*>(frame.data()),
frame.size());
- EXPECT_EQ(0, visitor.zero_length_control_frame_header_data_count_);
+ EXPECT_EQ(1, visitor.zero_length_control_frame_header_data_count_);
EXPECT_NE(headers.header_block(), visitor.headers_);
EXPECT_EQ(1u, visitor.headers_.size());
EXPECT_EQ("key=value; foo; bar=; k2=v2 ", visitor.headers_["cookie"]);
@@ -918,7 +772,7 @@
visitor.SimulateInFramer(reinterpret_cast<unsigned char*>(frame.data()),
frame.size());
- EXPECT_EQ(0, visitor.zero_length_control_frame_header_data_count_);
+ EXPECT_EQ(1, visitor.zero_length_control_frame_header_data_count_);
EXPECT_EQ(headers.header_block(), visitor.headers_);
}
@@ -1081,7 +935,6 @@
sizeof(kH2FrameData), false);
EXPECT_CALL(visitor, OnHeaders(1, false, 0, 0, false, false, false));
- EXPECT_CALL(visitor, OnHeaderFrameStart(1)).Times(1);
EXPECT_CALL(visitor, OnError(testing::Eq(&framer)));
EXPECT_EQ(frame.size(), framer.ProcessInput(frame.data(), frame.size()));
EXPECT_TRUE(framer.HasError());
@@ -1113,7 +966,7 @@
SpdySerializedFrame frame(kH2FrameData, sizeof(kH2FrameData), false);
EXPECT_CALL(visitor, OnHeaders(1, false, 0, 0, false, false, false));
- EXPECT_CALL(visitor, OnHeaderFrameStart(1)).Times(1);
+
EXPECT_EQ(frame.size(), framer.ProcessInput(frame.data(), frame.size()));
EXPECT_FALSE(framer.HasError());
EXPECT_EQ(SpdyFramer::SPDY_NO_ERROR, framer.error_code())
@@ -3658,8 +3511,12 @@
reinterpret_cast<unsigned char*>(control_frame.data()),
control_frame.size());
EXPECT_EQ(1, visitor.headers_frame_count_);
- EXPECT_EQ(0, visitor.control_frame_header_data_count_);
- EXPECT_EQ(0, visitor.zero_length_control_frame_header_data_count_);
+ // control_frame_header_data_count_ depends on the random sequence
+ // produced by rand(), so adding, removing or running single tests
+ // alters this value. The best we can do is assert that it happens
+ // at least twice.
+ EXPECT_LE(2, visitor.control_frame_header_data_count_);
+ EXPECT_EQ(1, visitor.zero_length_control_frame_header_data_count_);
EXPECT_EQ(0, visitor.end_of_stream_count_);
EXPECT_EQ(headers, visitor.headers_);
}
@@ -3678,8 +3535,12 @@
reinterpret_cast<unsigned char*>(control_frame.data()),
control_frame.size());
EXPECT_EQ(1, visitor.headers_frame_count_);
- EXPECT_EQ(0, visitor.control_frame_header_data_count_);
- EXPECT_EQ(0, visitor.zero_length_control_frame_header_data_count_);
+ // control_frame_header_data_count_ depends on the random sequence
+ // produced by rand(), so adding, removing or running single tests
+ // alters this value. The best we can do is assert that it happens
+ // at least twice.
+ EXPECT_LE(2, visitor.control_frame_header_data_count_);
+ EXPECT_EQ(1, visitor.zero_length_control_frame_header_data_count_);
EXPECT_EQ(1, visitor.end_of_stream_count_);
EXPECT_EQ(headers, visitor.headers_);
}
@@ -3715,8 +3576,9 @@
EXPECT_TRUE(visitor.header_buffer_valid_);
EXPECT_EQ(0, visitor.error_count_);
EXPECT_EQ(1, visitor.syn_frame_count_);
- EXPECT_EQ(0, visitor.zero_length_control_frame_header_data_count_);
+ EXPECT_EQ(1, visitor.zero_length_control_frame_header_data_count_);
EXPECT_EQ(0, visitor.end_of_stream_count_);
+ EXPECT_LT(kBigValueSize, visitor.header_buffer_length_);
}
TEST_P(SpdyFramerTest, ControlFrameTooLarge) {
@@ -3791,7 +3653,7 @@
EXPECT_EQ(0, visitor.error_count_);
EXPECT_EQ(1, visitor.headers_frame_count_);
EXPECT_EQ(1, visitor.continuation_count_);
- EXPECT_EQ(0, visitor.zero_length_control_frame_header_data_count_);
+ EXPECT_EQ(1, visitor.zero_length_control_frame_header_data_count_);
}
TEST_P(SpdyFramerTest, TooLargePushPromiseFrameUsesContinuation) {
@@ -3821,7 +3683,7 @@
EXPECT_EQ(0, visitor.error_count_);
EXPECT_EQ(1, visitor.push_promise_frame_count_);
EXPECT_EQ(1, visitor.continuation_count_);
- EXPECT_EQ(0, visitor.zero_length_control_frame_header_data_count_);
+ EXPECT_EQ(1, visitor.zero_length_control_frame_header_data_count_);
}
// Check that the framer stops delivering header data chunks once the visitor
@@ -3845,10 +3707,25 @@
visitor.SimulateInFramer(
reinterpret_cast<unsigned char*>(control_frame.data()),
control_frame.size());
- // It's up to the visitor to ignore extraneous header data; the framer
- // won't throw an error.
- EXPECT_GT(visitor.header_bytes_received_, visitor.header_buffer_size_);
- EXPECT_EQ(1, visitor.end_of_stream_count_);
+ EXPECT_FALSE(visitor.header_buffer_valid_);
+ EXPECT_EQ(1, visitor.error_count_);
+ EXPECT_EQ(SpdyFramer::SPDY_CONTROL_PAYLOAD_TOO_LARGE,
+ visitor.framer_.error_code())
+ << SpdyFramer::ErrorCodeToString(visitor.framer_.error_code());
+
+ // The framer should have stoped delivering chunks after the visitor
+ // signaled "stop" by returning false from OnControlFrameHeaderData().
+ //
+ // control_frame_header_data_count_ depends on the random sequence
+ // produced by rand(), so adding, removing or running single tests
+ // alters this value. The best we can do is assert that it happens
+ // at least kHeaderBufferChunks + 1.
+ EXPECT_LE(kHeaderBufferChunks + 1,
+ static_cast<unsigned>(visitor.control_frame_header_data_count_));
+ EXPECT_EQ(0, visitor.zero_length_control_frame_header_data_count_);
+
+ // The framer should not have sent half-close to the visitor.
+ EXPECT_EQ(0, visitor.end_of_stream_count_);
}
TEST_P(SpdyFramerTest, DecompressCorruptHeaderBlock) {
@@ -4302,7 +4179,7 @@
EXPECT_EQ(1, visitor.headers_frame_count_);
EXPECT_EQ(2, visitor.continuation_count_);
EXPECT_EQ(1, visitor.fin_flag_count_);
- EXPECT_EQ(0, visitor.zero_length_control_frame_header_data_count_);
+ EXPECT_EQ(1, visitor.zero_length_control_frame_header_data_count_);
EXPECT_EQ(1, visitor.end_of_stream_count_);
EXPECT_THAT(visitor.headers_,
@@ -4352,7 +4229,7 @@
EXPECT_EQ(1u, visitor.last_push_promise_stream_);
EXPECT_EQ(42u, visitor.last_push_promise_promised_stream_);
EXPECT_EQ(2, visitor.continuation_count_);
- EXPECT_EQ(0, visitor.zero_length_control_frame_header_data_count_);
+ EXPECT_EQ(1, visitor.zero_length_control_frame_header_data_count_);
EXPECT_EQ(0, visitor.end_of_stream_count_);
EXPECT_THAT(visitor.headers_,
@@ -4959,8 +4836,8 @@
EXPECT_CALL(debug_visitor, OnReceiveCompressedFrame(8, SYN_STREAM, _));
EXPECT_CALL(visitor, OnSynStream(8, 3, 1, flags & CONTROL_FLAG_FIN,
flags & CONTROL_FLAG_UNIDIRECTIONAL));
- EXPECT_CALL(visitor, OnHeaderFrameStart(8)).Times(1);
- EXPECT_CALL(visitor, OnHeaderFrameEnd(8, _)).Times(1);
+ EXPECT_CALL(visitor, OnControlFrameHeaderData(8, _, _))
+ .WillRepeatedly(testing::Return(true));
if (flags & DATA_FLAG_FIN) {
EXPECT_CALL(visitor, OnStreamEnd(_));
} else {
@@ -5006,8 +4883,8 @@
EXPECT_CALL(visitor, OnError(_));
} else {
EXPECT_CALL(visitor, OnSynReply(37, flags & CONTROL_FLAG_FIN));
- EXPECT_CALL(visitor, OnHeaderFrameStart(37)).Times(1);
- EXPECT_CALL(visitor, OnHeaderFrameEnd(37, _)).Times(1);
+ EXPECT_CALL(visitor, OnControlFrameHeaderData(37, _, _))
+ .WillRepeatedly(testing::Return(true));
if (flags & DATA_FLAG_FIN) {
EXPECT_CALL(visitor, OnStreamEnd(_));
}
@@ -5205,10 +5082,8 @@
}
EXPECT_CALL(visitor, OnHeaders(stream_id, has_priority, priority,
parent_stream_id, exclusive, fin, end));
- EXPECT_CALL(visitor, OnHeaderFrameStart(57)).Times(1);
- if (end) {
- EXPECT_CALL(visitor, OnHeaderFrameEnd(57, _)).Times(1);
- }
+ EXPECT_CALL(visitor, OnControlFrameHeaderData(57, _, _))
+ .WillRepeatedly(testing::Return(true));
if (flags & DATA_FLAG_FIN &&
(IsSpdy3() || flags & HEADERS_FLAG_END_HEADERS)) {
EXPECT_CALL(visitor, OnStreamEnd(_));
@@ -5304,13 +5179,9 @@
bool end = flags & PUSH_PROMISE_FLAG_END_PUSH_PROMISE;
EXPECT_CALL(debug_visitor,
OnReceiveCompressedFrame(client_id, PUSH_PROMISE, _));
- EXPECT_CALL(visitor,
- OnPushPromise(client_id, promised_id,
- flags & PUSH_PROMISE_FLAG_END_PUSH_PROMISE));
- EXPECT_CALL(visitor, OnHeaderFrameStart(client_id)).Times(1);
- if (end) {
- EXPECT_CALL(visitor, OnHeaderFrameEnd(client_id, _)).Times(1);
- }
+ EXPECT_CALL(visitor, OnPushPromise(client_id, promised_id, end));
+ EXPECT_CALL(visitor, OnControlFrameHeaderData(client_id, _, _))
+ .WillRepeatedly(testing::Return(true));
framer.ProcessInput(frame.data(), frame.size());
EXPECT_EQ(SpdyFramer::SPDY_READY_FOR_FRAME, framer.state());
@@ -5338,7 +5209,8 @@
EXPECT_CALL(debug_visitor, OnSendCompressedFrame(42, HEADERS, _, _));
EXPECT_CALL(debug_visitor, OnReceiveCompressedFrame(42, HEADERS, _));
EXPECT_CALL(visitor, OnHeaders(42, false, 0, 0, false, false, false));
- EXPECT_CALL(visitor, OnHeaderFrameStart(42)).Times(1);
+ EXPECT_CALL(visitor, OnControlFrameHeaderData(42, _, _))
+ .WillRepeatedly(testing::Return(true));
SpdyHeadersIR headers_ir(42);
headers_ir.SetHeader("foo", "bar");
@@ -5352,10 +5224,8 @@
EXPECT_CALL(debug_visitor, OnReceiveCompressedFrame(42, CONTINUATION, _));
EXPECT_CALL(visitor, OnContinuation(42, flags & HEADERS_FLAG_END_HEADERS));
- bool end = flags & HEADERS_FLAG_END_HEADERS;
- if (end) {
- EXPECT_CALL(visitor, OnHeaderFrameEnd(42, _)).Times(1);
- }
+ EXPECT_CALL(visitor, OnControlFrameHeaderData(42, _, _))
+ .WillRepeatedly(testing::Return(true));
framer.ProcessInput(frame0.data(), frame0.size());
framer.ProcessInput(frame.data(), frame.size());
@@ -5392,8 +5262,7 @@
EXPECT_CALL(debug_visitor, OnReceiveCompressedFrame(1, SYN_STREAM, _));
EXPECT_CALL(visitor, OnSynStream(1, 0, 1, false, false));
- EXPECT_CALL(visitor, OnHeaderFrameStart(1)).Times(1);
- EXPECT_CALL(visitor, OnHeaderFrameEnd(1, _)).Times(1);
+ EXPECT_CALL(visitor, OnControlFrameHeaderData(1, NULL, 0));
framer.ProcessInput(frame.data(), framer.GetSynStreamMinimumSize());
EXPECT_EQ(SpdyFramer::SPDY_READY_FOR_FRAME, framer.state());
« no previous file with comments | « net/spdy/spdy_framer.cc ('k') | net/spdy/spdy_network_transaction_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698