Skip to content

Commit 4b2b851

Browse files
chansikparkWor Ker
andauthored
Fix HTTP 414 errors hanging until timeout (yhirose#2260)
* Fix HTTP 414 errors hanging until timeout * All errors (status code 400+) close the connection * 🧹 --------- Co-authored-by: Wor Ker <worker@factory>
1 parent 551f96d commit 4b2b851

File tree

2 files changed

+46
-3
lines changed

2 files changed

+46
-3
lines changed

httplib.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7692,7 +7692,8 @@ inline bool Server::write_response_core(Stream &strm, bool close_connection,
76927692
if (need_apply_ranges) { apply_ranges(req, res, content_type, boundary); }
76937693

76947694
// Prepare additional headers
7695-
if (close_connection || req.get_header_value("Connection") == "close") {
7695+
if (close_connection || req.get_header_value("Connection") == "close" ||
7696+
400 <= res.status) { // Don't leave connections open after errors
76967697
res.set_header("Connection", "close");
76977698
} else {
76987699
std::string s = "timeout=";
@@ -8403,8 +8404,6 @@ Server::process_request(Stream &strm, const std::string &remote_addr,
84038404

84048405
// Check if the request URI doesn't exceed the limit
84058406
if (req.target.size() > CPPHTTPLIB_REQUEST_URI_MAX_LENGTH) {
8406-
Headers dummy;
8407-
detail::read_headers(strm, dummy);
84088407
res.status = StatusCode::UriTooLong_414;
84098408
output_error_log(Error::ExceedUriMaxLength, &req);
84108409
return write_response(strm, close_connection, req, res);

test/test.cc

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4289,10 +4289,21 @@ TEST_F(ServerTest, TooLongRequest) {
42894289
}
42904290
request += "_NG";
42914291

4292+
auto start = std::chrono::high_resolution_clock::now();
4293+
4294+
cli_.set_keep_alive(true);
42924295
auto res = cli_.Get(request.c_str());
42934296

4297+
auto end = std::chrono::high_resolution_clock::now();
4298+
auto elapsed =
4299+
std::chrono::duration_cast<std::chrono::milliseconds>(end - start)
4300+
.count();
4301+
42944302
ASSERT_TRUE(res);
42954303
EXPECT_EQ(StatusCode::UriTooLong_414, res->status);
4304+
EXPECT_LE(elapsed, 100);
4305+
EXPECT_EQ("close", res->get_header_value("Connection"));
4306+
EXPECT_FALSE(cli_.is_socket_open());
42964307
}
42974308

42984309
TEST_F(ServerTest, AlmostTooLongRequest) {
@@ -4363,10 +4374,21 @@ TEST_F(ServerTest, LongHeader) {
43634374
}
43644375

43654376
TEST_F(ServerTest, LongQueryValue) {
4377+
auto start = std::chrono::high_resolution_clock::now();
4378+
4379+
cli_.set_keep_alive(true);
43664380
auto res = cli_.Get(LONG_QUERY_URL.c_str());
43674381

4382+
auto end = std::chrono::high_resolution_clock::now();
4383+
auto elapsed =
4384+
std::chrono::duration_cast<std::chrono::milliseconds>(end - start)
4385+
.count();
4386+
43684387
ASSERT_TRUE(res);
43694388
EXPECT_EQ(StatusCode::UriTooLong_414, res->status);
4389+
EXPECT_LE(elapsed, 100);
4390+
EXPECT_EQ("close", res->get_header_value("Connection"));
4391+
EXPECT_FALSE(cli_.is_socket_open());
43704392
}
43714393

43724394
TEST_F(ServerTest, TooLongQueryValue) {
@@ -4460,6 +4482,7 @@ TEST_F(ServerTest, HeaderCountExceedsLimit) {
44604482
}
44614483

44624484
// This should fail due to exceeding header count limit
4485+
cli_.set_keep_alive(true);
44634486
auto res = cli_.Get("/hi", headers);
44644487

44654488
// The request should either fail or return 400 Bad Request
@@ -4470,6 +4493,9 @@ TEST_F(ServerTest, HeaderCountExceedsLimit) {
44704493
// Or the request should fail entirely
44714494
EXPECT_FALSE(res);
44724495
}
4496+
4497+
EXPECT_EQ("close", res->get_header_value("Connection"));
4498+
EXPECT_FALSE(cli_.is_socket_open());
44734499
}
44744500

44754501
TEST_F(ServerTest, PercentEncoding) {
@@ -4524,6 +4550,7 @@ TEST_F(ServerTest, HeaderCountSecurityTest) {
45244550
}
45254551

45264552
// Try to POST with excessive headers
4553+
cli_.set_keep_alive(true);
45274554
auto res = cli_.Post("/", attack_headers, "test_data", "text/plain");
45284555

45294556
// Should either fail or return 400 Bad Request due to security limit
@@ -4534,6 +4561,9 @@ TEST_F(ServerTest, HeaderCountSecurityTest) {
45344561
// Request failed, which is the expected behavior for DoS protection
45354562
EXPECT_FALSE(res);
45364563
}
4564+
4565+
EXPECT_EQ("close", res->get_header_value("Connection"));
4566+
EXPECT_FALSE(cli_.is_socket_open());
45374567
}
45384568

45394569
TEST_F(ServerTest, MultipartFormData) {
@@ -5854,6 +5884,20 @@ TEST_F(ServerTest, TooManyRedirect) {
58545884
EXPECT_EQ(Error::ExceedRedirectCount, res.error());
58555885
}
58565886

5887+
TEST_F(ServerTest, BadRequestLineCancelsKeepAlive) {
5888+
Request req;
5889+
req.method = "FOOBAR";
5890+
req.path = "/hi";
5891+
5892+
cli_.set_keep_alive(true);
5893+
auto res = cli_.send(req);
5894+
5895+
ASSERT_TRUE(res);
5896+
EXPECT_EQ(StatusCode::BadRequest_400, res->status);
5897+
EXPECT_EQ("close", res->get_header_value("Connection"));
5898+
EXPECT_FALSE(cli_.is_socket_open());
5899+
}
5900+
58575901
TEST_F(ServerTest, StartTime) { auto res = cli_.Get("/test-start-time"); }
58585902

58595903
#ifdef CPPHTTPLIB_ZLIB_SUPPORT

0 commit comments

Comments
 (0)