Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions source/common/http/http1/codec_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ class StreamEncoderImpl : public virtual StreamEncoder,
// Http::Stream
void addCallbacks(StreamCallbacks& callbacks) override { addCallbacks_(callbacks); }
void removeCallbacks(StreamCallbacks& callbacks) override { removeCallbacks_(callbacks); }
// After this is called, for the HTTP/1 codec, the connection should be closed, i.e. no further
// progress may be made with the codec.
void resetStream(StreamResetReason reason) override;
void readDisable(bool disable) override;
uint32_t bufferLimit() override;
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

29 changes: 24 additions & 5 deletions test/common/http/codec_impl_fuzz_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ fromHttp2Settings(const test::common::http::Http2Settings& settings) {
return options;
}

using StreamResetCallbackFn = std::function<void()>;

// Internal representation of stream state. Encapsulates the stream state, mocks
// and encoders for both the request/response.
class HttpStream : public LinkedObject<HttpStream> {
Expand Down Expand Up @@ -122,12 +124,14 @@ class HttpStream : public LinkedObject<HttpStream> {
} request_, response_;

HttpStream(ClientConnection& client, const TestRequestHeaderMapImpl& request_headers,
bool end_stream) {
bool end_stream, StreamResetCallbackFn stream_reset_callback)
: stream_reset_callback_(stream_reset_callback) {
request_.request_encoder_ = &client.newStream(response_.response_decoder_);
ON_CALL(request_.stream_callbacks_, onResetStream(_, _))
.WillByDefault(InvokeWithoutArgs([this] {
ENVOY_LOG_MISC(trace, "reset request for stream index {}", stream_index_);
resetStream();
stream_reset_callback_();
}));
ON_CALL(response_.stream_callbacks_, onResetStream(_, _))
.WillByDefault(InvokeWithoutArgs([this] {
Expand All @@ -138,6 +142,7 @@ class HttpStream : public LinkedObject<HttpStream> {
// HTTP/1 codec.
request_.request_encoder_->getStream().resetStream(StreamResetReason::LocalReset);
resetStream();
stream_reset_callback_();
}));
ON_CALL(request_.request_decoder_, decodeHeaders_(_, true))
.WillByDefault(InvokeWithoutArgs([this] {
Expand Down Expand Up @@ -325,6 +330,7 @@ class HttpStream : public LinkedObject<HttpStream> {
}

int32_t stream_index_{-1};
StreamResetCallbackFn stream_reset_callback_;
};

// Buffer between client and server H1/H2 codecs. This models each write operation
Expand Down Expand Up @@ -472,9 +478,14 @@ void codecFuzz(const test::common::http::CodecImplFuzzTestCase& input, HttpVersi
}
};

// We track whether the connection should be closed for HTTP/1, since stream resets imply
// connection closes.
bool should_close_connection = false;

constexpr auto max_actions = 1024;
try {
for (int i = 0; i < std::min(max_actions, input.actions().size()); ++i) {
for (int i = 0; i < std::min(max_actions, input.actions().size()) && !should_close_connection;
++i) {
const auto& action = input.actions(i);
ENVOY_LOG_MISC(trace, "action {} with {} streams", action.DebugString(), streams.size());
switch (action.action_selector_case()) {
Expand All @@ -492,7 +503,12 @@ void codecFuzz(const test::common::http::CodecImplFuzzTestCase& input, HttpVersi
HttpStreamPtr stream = std::make_unique<HttpStream>(
*client,
fromSanitizedHeaders<TestRequestHeaderMapImpl>(action.new_stream().request_headers()),
action.new_stream().end_stream());
action.new_stream().end_stream(), [&should_close_connection, http2]() {
// HTTP/1 codec has stream reset implying connection close.
if (!http2) {
should_close_connection = true;
}
});
stream->moveIntoListBack(std::move(stream), pending_streams);
break;
}
Expand Down Expand Up @@ -535,11 +551,14 @@ void codecFuzz(const test::common::http::CodecImplFuzzTestCase& input, HttpVersi
// Maybe nothing is set?
break;
}
if (DebugMode) {
if (DebugMode && !should_close_connection) {
client_server_buf_drain();
}
}
client_server_buf_drain();
// Drain all remaining buffers, unless the connection is effectively closed.
if (!should_close_connection) {
client_server_buf_drain();
Comment thread
asraa marked this conversation as resolved.
}
if (http2) {
dynamic_cast<Http2::TestClientConnectionImpl&>(*client).goAway();
dynamic_cast<Http2::TestServerConnectionImpl&>(*server).goAway();
Expand Down