Skip to content

Commit 84e0ac0

Browse files
committed
Fix custom route request forwarding regressions
1 parent ce060f1 commit 84e0ac0

7 files changed

Lines changed: 296 additions & 6 deletions

File tree

include/fastmcpp/app.hpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include <chrono>
1111
#include <functional>
1212
#include <initializer_list>
13+
#include <map>
1314
#include <memory>
1415
#include <optional>
1516
#include <string>
@@ -33,6 +34,8 @@ struct CustomRouteRequest
3334
std::string path;
3435
std::string body;
3536
std::unordered_map<std::string, std::string> headers;
37+
std::string target;
38+
std::multimap<std::string, std::string> query_params;
3639
};
3740

3841
/// HTTP response returned by a custom-route handler.
@@ -49,7 +52,7 @@ struct CustomRouteResponse
4952
/// fixed forwarding from mounted servers).
5053
struct CustomRoute
5154
{
52-
std::string method; // GET, POST, etc. (uppercase)
55+
std::string method; // GET, POST, PUT, DELETE, PATCH
5356
std::string path; // Absolute path, e.g. "/health" — must start with '/'
5457
std::function<CustomRouteResponse(const CustomRouteRequest&)> handler;
5558
};

include/fastmcpp/providers/transforms/catalog.hpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,9 @@ class CatalogTransform : public Transform
108108
if (!entry.available_versions.empty())
109109
{
110110
fastmcpp::Json meta =
111-
entry.item.meta().has_value() ? *entry.item.meta() : fastmcpp::Json::object();
111+
entry.item.meta().has_value() && entry.item.meta()->is_object()
112+
? *entry.item.meta()
113+
: fastmcpp::Json::object();
112114
fastmcpp::Json fm = meta.contains("fastmcp") && meta["fastmcp"].is_object()
113115
? meta["fastmcp"]
114116
: fastmcpp::Json::object();
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
#pragma once
2+
3+
#include "fastmcpp/exceptions.hpp"
4+
5+
#include <algorithm>
6+
#include <array>
7+
#include <cctype>
8+
#include <string>
9+
10+
namespace fastmcpp::util::http
11+
{
12+
13+
inline std::string normalize_custom_route_method(std::string method)
14+
{
15+
if (method.empty())
16+
throw ValidationError("CustomRoute.method is required");
17+
18+
std::transform(method.begin(), method.end(), method.begin(),
19+
[](unsigned char ch) { return static_cast<char>(std::toupper(ch)); });
20+
21+
static constexpr std::array<const char*, 5> kSupportedMethods = {"GET", "POST", "PUT", "DELETE",
22+
"PATCH"};
23+
const auto supported = std::find(kSupportedMethods.begin(), kSupportedMethods.end(), method);
24+
if (supported != kSupportedMethods.end())
25+
return method;
26+
27+
if (method == "HEAD" || method == "OPTIONS")
28+
throw ValidationError("CustomRoute.method '" + method +
29+
"' is reserved and not supported for custom routes");
30+
31+
throw ValidationError("CustomRoute.method '" + method +
32+
"' is unsupported; expected GET, POST, PUT, DELETE, or PATCH");
33+
}
34+
35+
} // namespace fastmcpp::util::http

src/app.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "fastmcpp/mcp/handler.hpp"
77
#include "fastmcpp/providers/provider.hpp"
88
#include "fastmcpp/resources/template.hpp"
9+
#include "fastmcpp/util/http_methods.hpp"
910
#include "fastmcpp/util/json_schema.hpp"
1011
#include "fastmcpp/util/schema_build.hpp"
1112

@@ -287,11 +288,11 @@ FastMCP& FastMCP::add_custom_route(CustomRoute route)
287288
{
288289
if (route.path.empty() || route.path.front() != '/')
289290
throw ValidationError("CustomRoute.path must start with '/' (got '" + route.path + "')");
290-
if (route.method.empty())
291-
throw ValidationError("CustomRoute.method is required");
292291
if (!route.handler)
293292
throw ValidationError("CustomRoute.handler is required");
294293

294+
route.method = util::http::normalize_custom_route_method(std::move(route.method));
295+
295296
// Re-registering the same (method, path) replaces the previous entry —
296297
// matches Python `@server.custom_route()` decorator semantics.
297298
for (auto& existing : custom_routes_)

src/server/http_server.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#include "fastmcpp/app.hpp"
44
#include "fastmcpp/exceptions.hpp"
5+
#include "fastmcpp/util/http_methods.hpp"
56
#include "fastmcpp/util/json.hpp"
67

78
#include <httplib.h>
@@ -11,6 +12,14 @@ namespace fastmcpp::server
1112

1213
void HttpServerWrapper::set_custom_routes(std::vector<fastmcpp::CustomRoute> routes)
1314
{
15+
for (auto& route : routes)
16+
{
17+
route.method = fastmcpp::util::http::normalize_custom_route_method(std::move(route.method));
18+
if (route.path.empty() || route.path.front() != '/')
19+
throw ValidationError("CustomRoute.path must start with '/' (got '" + route.path + "')");
20+
if (!route.handler)
21+
throw ValidationError("CustomRoute.handler is required");
22+
}
1423
custom_routes_ = std::move(routes);
1524
}
1625

@@ -107,9 +116,11 @@ bool HttpServerWrapper::start()
107116

108117
apply_additional_response_headers(res);
109118
fastmcpp::CustomRouteRequest cr;
110-
cr.method = route.method;
119+
cr.method = req.method;
111120
cr.path = req.path;
112121
cr.body = req.body;
122+
cr.target = req.target;
123+
cr.query_params = req.params;
113124
for (const auto& [k, v] : req.headers)
114125
cr.headers[k] = v;
115126
try

tests/app/custom_route_forwarding.cpp

Lines changed: 135 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,10 @@ static int test_register_replaces_duplicate()
6565
{
6666
std::cout << " test_register_replaces_duplicate..." << std::endl;
6767
FastMCP app("a", "1.0.0");
68-
app.add_custom_route(make_route("GET", "/x", "first"));
68+
app.add_custom_route(make_route("get", "/x", "first"));
6969
app.add_custom_route(make_route("GET", "/x", "second"));
7070
ASSERT_EQ(app.custom_routes().size(), 1u, "still one route");
71+
ASSERT_EQ(app.custom_routes().front().method, std::string("GET"), "method normalized to uppercase");
7172
auto resp = app.custom_routes().front().handler({"GET", "/x", "", {}});
7273
ASSERT_EQ(resp.body, std::string("second"), "second handler wins");
7374
std::cout << " PASS" << std::endl;
@@ -89,6 +90,28 @@ static int test_validation_rejects_bad_inputs()
8990
}
9091
ASSERT_TRUE(threw, "missing leading slash rejected");
9192

93+
threw = false;
94+
try
95+
{
96+
app.add_custom_route(make_route("", "/x", "x"));
97+
}
98+
catch (const fastmcpp::ValidationError&)
99+
{
100+
threw = true;
101+
}
102+
ASSERT_TRUE(threw, "missing method rejected");
103+
104+
threw = false;
105+
try
106+
{
107+
app.add_custom_route(make_route("HEAD", "/x", "x"));
108+
}
109+
catch (const fastmcpp::ValidationError&)
110+
{
111+
threw = true;
112+
}
113+
ASSERT_TRUE(threw, "unsupported method rejected");
114+
92115
threw = false;
93116
CustomRoute no_handler;
94117
no_handler.method = "GET";
@@ -106,6 +129,28 @@ static int test_validation_rejects_bad_inputs()
106129
return 0;
107130
}
108131

132+
static int test_http_wrapper_rejects_unsupported_custom_route_method()
133+
{
134+
std::cout << " test_http_wrapper_rejects_unsupported_custom_route_method..." << std::endl;
135+
136+
auto core = std::make_shared<server::Server>();
137+
server::HttpServerWrapper http(core, "127.0.0.1", 0);
138+
139+
bool threw = false;
140+
try
141+
{
142+
http.set_custom_routes({make_route("HEAD", "/health", "ok")});
143+
}
144+
catch (const fastmcpp::ValidationError&)
145+
{
146+
threw = true;
147+
}
148+
149+
ASSERT_TRUE(threw, "direct wrapper route registration rejects unsupported methods");
150+
std::cout << " PASS" << std::endl;
151+
return 0;
152+
}
153+
109154
static int test_aggregate_from_mounted_child()
110155
{
111156
std::cout << " test_aggregate_from_mounted_child..." << std::endl;
@@ -198,6 +243,93 @@ static int test_http_end_to_end_serves_route()
198243
return 0;
199244
}
200245

246+
static int test_http_custom_route_preserves_query_params()
247+
{
248+
std::cout << " test_http_custom_route_preserves_query_params..." << std::endl;
249+
250+
CustomRouteRequest captured;
251+
bool called = false;
252+
253+
FastMCP child("child", "1.0.0");
254+
CustomRoute query_route;
255+
query_route.method = "GET";
256+
query_route.path = "/search";
257+
query_route.handler = [&](const CustomRouteRequest& req)
258+
{
259+
called = true;
260+
captured = req;
261+
262+
CustomRouteResponse resp;
263+
resp.body = "query ok";
264+
resp.content_type = "text/plain";
265+
return resp;
266+
};
267+
child.add_custom_route(std::move(query_route));
268+
269+
FastMCP parent("parent", "1.0.0");
270+
parent.mount(child, "kids");
271+
272+
auto core = std::make_shared<server::Server>(parent.server());
273+
274+
int port = 0;
275+
std::unique_ptr<server::HttpServerWrapper> http;
276+
for (int candidate = 18481; candidate <= 18500; ++candidate)
277+
{
278+
auto trial = std::make_unique<server::HttpServerWrapper>(core, "127.0.0.1", candidate);
279+
trial->set_custom_routes(parent.all_custom_routes());
280+
if (trial->start())
281+
{
282+
port = trial->port();
283+
http = std::move(trial);
284+
break;
285+
}
286+
}
287+
ASSERT_TRUE(http && port > 0, "HTTP server started");
288+
289+
std::this_thread::sleep_for(std::chrono::milliseconds(150));
290+
291+
httplib::Client client("127.0.0.1", port);
292+
client.set_connection_timeout(std::chrono::seconds(2));
293+
client.set_read_timeout(std::chrono::seconds(2));
294+
295+
auto resp = client.Get("/kids/search?q=a&q=b&lang=en");
296+
ASSERT_TRUE(resp != nullptr, "GET with query params returned a response");
297+
ASSERT_EQ(resp->status, 200, "query route served");
298+
ASSERT_EQ(resp->body, std::string("query ok"), "query route body");
299+
300+
ASSERT_TRUE(called, "handler was invoked");
301+
ASSERT_EQ(captured.method, std::string("GET"), "request method preserved");
302+
ASSERT_EQ(captured.path, std::string("/kids/search"), "path preserved without query string");
303+
ASSERT_EQ(captured.target, std::string("/kids/search?q=a&q=b&lang=en"),
304+
"raw target preserves query string");
305+
ASSERT_EQ(captured.query_params.count("q"), 2u, "repeated query param preserved");
306+
ASSERT_EQ(captured.query_params.count("lang"), 1u, "single query param preserved");
307+
308+
auto q_range = captured.query_params.equal_range("q");
309+
bool seen_q_a = false;
310+
bool seen_q_b = false;
311+
size_t q_values = 0;
312+
for (auto it = q_range.first; it != q_range.second; ++it)
313+
{
314+
++q_values;
315+
if (it->second == "a")
316+
seen_q_a = true;
317+
if (it->second == "b")
318+
seen_q_b = true;
319+
}
320+
ASSERT_EQ(q_values, 2u, "two q values captured");
321+
ASSERT_TRUE(seen_q_a, "q=a preserved");
322+
ASSERT_TRUE(seen_q_b, "q=b preserved");
323+
324+
auto lang_it = captured.query_params.find("lang");
325+
ASSERT_TRUE(lang_it != captured.query_params.end(), "lang key present");
326+
ASSERT_EQ(lang_it->second, std::string("en"), "lang value preserved");
327+
328+
http->stop();
329+
std::cout << " PASS" << std::endl;
330+
return 0;
331+
}
332+
201333
static int test_http_custom_route_requires_auth()
202334
{
203335
std::cout << " test_http_custom_route_requires_auth..." << std::endl;
@@ -296,9 +428,11 @@ int main()
296428
failures += test_register_basic();
297429
failures += test_register_replaces_duplicate();
298430
failures += test_validation_rejects_bad_inputs();
431+
failures += test_http_wrapper_rejects_unsupported_custom_route_method();
299432
failures += test_aggregate_from_mounted_child();
300433
failures += test_aggregate_dedups_collisions();
301434
failures += test_http_end_to_end_serves_route();
435+
failures += test_http_custom_route_preserves_query_params();
302436
failures += test_http_custom_route_requires_auth();
303437
failures += test_http_custom_route_options_advertises_methods();
304438
std::cout << std::endl;

0 commit comments

Comments
 (0)