Skip to content

Commit f7a4c46

Browse files
gophergogocaleb2h
authored andcommitted
Test that client-mode HTTP codec actually disables body_timeout (#212)
The state-machine test already proves body_timeout == 0 disables the body timer. What it does not prove is that HttpCodecFilter in client mode is actually wired to pass 0. The client-mode fix is a single ternary in the filter's constructor; a regression (e.g. someone reverting to a flat 60s) would bring back the SSE crash silently. Add a narrow introspection accessor: - HttpCodecStateMachine::config() — read-only view of the as-configured values (immutable after construction). - HttpCodecFilter::bodyTimeout() — forwards to the state machine's configured body_timeout. Use it in two new tests: server mode retains 60s, client mode is 0.
1 parent 9c15517 commit f7a4c46

3 files changed

Lines changed: 59 additions & 0 deletions

File tree

include/mcp/filter/http_codec_filter.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,18 @@ class HttpCodecFilter : public network::Filter {
210210
*/
211211
void markSseGetSent() { sse_get_sent_ = true; }
212212

213+
/**
214+
* Body timeout actually configured on the state machine. In client mode
215+
* this is disabled (0) because SSE response streams may sit idle between
216+
* server-pushed events; in server mode it is bounded. Exposed for
217+
* introspection and for tests that guard against regressions in the
218+
* client/server wiring.
219+
*/
220+
std::chrono::milliseconds bodyTimeout() const {
221+
return state_machine_ ? state_machine_->config().body_timeout
222+
: std::chrono::milliseconds(0);
223+
}
224+
213225
private:
214226
// Inner class implementing MessageEncoder
215227
class MessageEncoderImpl : public MessageEncoder {

include/mcp/filter/http_codec_state_machine.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,12 @@ class HttpCodecStateMachine {
345345
*/
346346
static std::string getEventName(HttpCodecEvent event);
347347

348+
/**
349+
* Configured values (as passed into the constructor). Read-only; the
350+
* config is immutable after construction.
351+
*/
352+
const HttpCodecStateMachineConfig& config() const { return config_; }
353+
348354
protected:
349355
// ===== Protected Methods for Extension =====
350356

tests/filter/test_http_codec_filter.cc

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,6 +548,47 @@ TEST_F(HttpCodecFilterRealIoTest, StateMachineIntegration) {
548548
EXPECT_EQ(headers["url"], "/state-test");
549549
}
550550

551+
// ---------------------------------------------------------------------------
552+
// Body-timeout wiring by role
553+
//
554+
// Regression guard for the SSE crash: in client mode the codec is expected
555+
// to disable the body timer (body_timeout == 0) because SSE response bodies
556+
// are an open-ended stream that may sit idle between server-pushed events.
557+
// In server mode the 60s body timeout must stay on — request bodies are
558+
// expected to complete promptly. These tests read back the value actually
559+
// stored on the state machine so a regression in the is_server_ ternary in
560+
// http_codec_filter.cc is caught directly.
561+
// ---------------------------------------------------------------------------
562+
563+
class HttpCodecFilterBodyTimeoutTest : public ::testing::Test {
564+
protected:
565+
void SetUp() override {
566+
auto factory = event::createLibeventDispatcherFactory();
567+
dispatcher_ = factory->createDispatcher("codec-bt");
568+
// Run once so timers can be created (state machine arms from ctor).
569+
dispatcher_->run(event::RunType::NonBlock);
570+
callbacks_ = std::make_unique<TestRequestCallbacks>();
571+
}
572+
573+
std::unique_ptr<event::Dispatcher> dispatcher_;
574+
std::unique_ptr<TestRequestCallbacks> callbacks_;
575+
};
576+
577+
TEST_F(HttpCodecFilterBodyTimeoutTest, ServerModeKeepsBodyTimeoutBounded) {
578+
HttpCodecFilter filter(*callbacks_, *dispatcher_, /*is_server=*/true);
579+
EXPECT_EQ(filter.bodyTimeout(), 60000ms)
580+
<< "server mode must retain a bounded body timeout so request bodies "
581+
"that stall don't wedge a connection";
582+
}
583+
584+
TEST_F(HttpCodecFilterBodyTimeoutTest, ClientModeDisablesBodyTimeout) {
585+
HttpCodecFilter filter(*callbacks_, *dispatcher_, /*is_server=*/false);
586+
EXPECT_EQ(filter.bodyTimeout(), 0ms)
587+
<< "client mode must disable the body timer — an SSE response body is "
588+
"an open-ended stream and a bounded body timeout would fire during "
589+
"normal idle gaps, then crash the state machine on a live stream";
590+
}
591+
551592
} // namespace
552593
} // namespace filter
553594
} // namespace mcp

0 commit comments

Comments
 (0)