Skip to content

Commit bbe6a45

Browse files
committed
removed response.end() call from catchall handlers, as it is called externally now.
added regressiontest from bug report too.
1 parent 8a87cfd commit bbe6a45

2 files changed

Lines changed: 34 additions & 2 deletions

File tree

include/crow/routing.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,6 @@ namespace crow // NOTE: Already documented in "crow/app.h"
356356

357357
handler_ = ([f = std::move(f)](const request&, response& res) {
358358
res = response(f());
359-
res.end();
360359
});
361360
}
362361

@@ -372,7 +371,6 @@ namespace crow // NOTE: Already documented in "crow/app.h"
372371

373372
handler_ = ([f = std::move(f)](const request& req, response& res) {
374373
res = response(f(req));
375-
res.end();
376374
});
377375
}
378376

tests/unittest.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2338,6 +2338,40 @@ TEST_CASE("catchall_check_full_handling")
23382338
app.stop();
23392339
} // local_middleware
23402340

2341+
// Regression test: a catchall handler that returns a value (overloads 1 & 2 of CatchallRule)
2342+
// must not leave response::completed_ as true after the first request on a keep-alive connection.
2343+
// Before the fix, Router::handle() called res.end() unconditionally, but the CatchallRule wrapper
2344+
// also called res.end() — the first end() sent the response and cleared completed_ back to false
2345+
// via res.clear(); the second end() then set completed_=true with no cleanup callback, poisoning
2346+
// the connection so all subsequent requests on the same socket bypassed the router and returned 404.
2347+
TEST_CASE("catchall_keepalive_return_value_handler")
2348+
{
2349+
SimpleApp app;
2350+
2351+
CROW_CATCHALL_ROUTE(app)
2352+
([](const crow::request&) {
2353+
return response(200, "ok");
2354+
});
2355+
2356+
auto _ = app.bindaddr(LOCALHOST_ADDRESS).port(45451).run_async();
2357+
app.wait_for_server_start();
2358+
2359+
// Reuse the same TCP connection for two requests (HTTP/1.1 keep-alive by default).
2360+
// Before the fix, the second request would be answered with 404 because completed_
2361+
// was left as true on the Connection's response object after the first request.
2362+
HttpClient c(LOCALHOST_ADDRESS, 45451);
2363+
const std::string req = "GET /any HTTP/1.1\r\nHost: localhost\r\n\r\n";
2364+
2365+
c.send(req);
2366+
auto resp1 = c.receive();
2367+
CHECK(resp1.find("200 OK") != std::string::npos);
2368+
2369+
c.send(req);
2370+
auto resp2 = c.receive();
2371+
CHECK(resp2.find("200 OK") != std::string::npos);
2372+
2373+
app.stop();
2374+
} // catchall_keepalive_return_value_handler
23412375

23422376
TEST_CASE("blueprint")
23432377
{

0 commit comments

Comments
 (0)