From 592894a4387a50e1d128ff6a2695ff48f8cc32a5 Mon Sep 17 00:00:00 2001 From: Joshua Richter Date: Tue, 1 Sep 2026 22:31:03 -0400 Subject: [PATCH] fix(routes): take a route's handler from the last argument, not the first Express, Fastify, gin and Laravel all put middleware between the route path and the handler, and the handler comes last: routerGet("/users", requireAuth, rateLimit, listUsers); extract_handler_arg returned the FIRST argument that looked like a function reference, so requireAuth won and the HANDLES edge pointed at the middleware instead of listUsers. That is a wrong edge, not a missing one, and it misleads anyone tracing a request through the graph. The scan bound hid a second fault. An arrow function matches none of the accepted node kinds, so three inline middlewares pushed the real handler past MAX_HANDLER_SCAN and no handler came back at all. The loop now examines every argument and keeps the last eligible one. Nothing that follows a handler matches the accepted kinds either, since an options argument is an object node, so the last match is the handler. HANDLER_START_IDX stays, because argument 0 really is the path. MAX_HANDLER_SCAN had no other use and is gone. A two-argument route has one eligible argument, so first and last agree and the existing Express, Fastify, gin and Laravel route tests are unchanged. Both tests fail without the change, the first reporting "requireAuth" where "listUsers" belongs and the second reporting no handler at all. Signed-off-by: Joshua Richter --- internal/cbm/extract_calls.c | 20 +++++++++++----- tests/test_extraction.c | 45 ++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 6 deletions(-) diff --git a/internal/cbm/extract_calls.c b/internal/cbm/extract_calls.c index 6bb4a32bc..c190e36b7 100644 --- a/internal/cbm/extract_calls.c +++ b/internal/cbm/extract_calls.c @@ -21,8 +21,6 @@ enum { LEAN_MAX_PARENT_DEPTH = 20 }; /* Max positional args to scan for URL/string. */ enum { MAX_POSITIONAL_SCAN = 3 }; -/* Max positional args to scan for handler ref. */ -enum { MAX_HANDLER_SCAN = 4 }; /* Max string arg length before rejection. */ enum { MAX_STRING_ARG_LEN = CBM_SZ_512 }; /* Min printable ASCII (space). */ @@ -2349,8 +2347,17 @@ static const char *normalize_string_handler(CBMArena *a, const char *raw) { } static const char *extract_handler_arg(CBMExtractCtx *ctx, TSNode args) { + /* The LAST eligible argument wins, and every argument is examined. + * Express, Fastify, gin and Laravel all put middleware between the route + * path and the handler, so the first function-shaped argument is usually a + * middleware. Taking the first one pointed the HANDLES edge at the + * middleware; stopping the scan early missed the handler outright when the + * middleware was written inline, because an arrow function matches none of + * the kinds below. Nothing that follows a handler matches them either — an + * options argument is an object node — so the last match is the handler. */ + const char *handler = NULL; uint32_t nc = ts_node_named_child_count(args); - for (uint32_t ai = HANDLER_START_IDX; ai < nc && ai < MAX_HANDLER_SCAN; ai++) { + for (uint32_t ai = HANDLER_START_IDX; ai < nc; ai++) { TSNode arg2 = ts_node_named_child(args, ai); /* PHP wraps each argument in an `argument` node — unwrap to the value. */ if (strcmp(ts_node_type(arg2), "argument") == 0 && ts_node_named_child_count(arg2) > 0) { @@ -2362,17 +2369,18 @@ static const char *extract_handler_arg(CBMExtractCtx *ctx, TSNode args) { if (strcmp(ak2, "identifier") == 0 || strcmp(ak2, "member_expression") == 0 || strcmp(ak2, "selector_expression") == 0 || strcmp(ak2, "attribute") == 0 || strcmp(ak2, "field_expression") == 0 || strcmp(ak2, "name") == 0) { - return cbm_node_text(ctx->arena, arg2, ctx->source); + handler = cbm_node_text(ctx->arena, arg2, ctx->source); + continue; } if (is_string_like(ak2)) { const char *h = normalize_string_handler(ctx->arena, cbm_node_text(ctx->arena, arg2, ctx->source)); if (h && h[0]) { - return h; + handler = h; } } } - return NULL; + return handler; } // Extract JSX component refs (uppercase tags) as CALLS edges. diff --git a/tests/test_extraction.c b/tests/test_extraction.c index 5b8d16f61..e1a1643dd 100644 --- a/tests/test_extraction.c +++ b/tests/test_extraction.c @@ -3970,6 +3970,49 @@ TEST(extract_ts_url_builder_issue1009) { PASS(); } +/* A route registration names its middleware before its handler, and every + * framework here puts the handler last. The handler scan took the FIRST + * argument that looked like a function reference, so a named middleware won + * and the HANDLES edge pointed at the middleware instead of the handler. */ +TEST(extract_ts_route_handler_after_named_middleware) { + CBMFileResult *r = extract("function requireAuth(req: any, res: any, next: any) { next(); }\n" + "function rateLimit(req: any, res: any, next: any) { next(); }\n" + "function listUsers(req: any, res: any) { res.json([]); }\n" + "routerGet(\"/users\", requireAuth, rateLimit, listUsers);\n", + CBM_LANG_TYPESCRIPT, "t", "routes.ts"); + ASSERT_NOT_NULL(r); + const CBMCall *c = find_call_by_callee(r, "routerGet"); + ASSERT_NOT_NULL(c); + ASSERT_NOT_NULL(c->first_string_arg); + ASSERT_STR_EQ(c->first_string_arg, "/users"); + ASSERT_NOT_NULL(c->second_arg_name); + ASSERT_STR_EQ(c->second_arg_name, "listUsers"); + cbm_free_result(r); + PASS(); +} + +/* The same route with its middleware written inline. An arrow function is not + * one of the kinds the handler scan accepts, so three of them pushed the real + * handler past the scan bound and no handler came back at all. */ +TEST(extract_ts_route_handler_after_inline_middleware) { + CBMFileResult *r = extract("function listOrders(req: any, res: any) { res.json([]); }\n" + "routerGet(\"/orders\",\n" + " (req: any, res: any, next: any) => { next(); },\n" + " (req: any, res: any, next: any) => { next(); },\n" + " (req: any, res: any, next: any) => { next(); },\n" + " listOrders);\n", + CBM_LANG_TYPESCRIPT, "t", "orders.ts"); + ASSERT_NOT_NULL(r); + const CBMCall *c = find_call_by_callee(r, "routerGet"); + ASSERT_NOT_NULL(c); + ASSERT_NOT_NULL(c->first_string_arg); + ASSERT_STR_EQ(c->first_string_arg, "/orders"); + ASSERT_NOT_NULL(c->second_arg_name); + ASSERT_STR_EQ(c->second_arg_name, "listOrders"); + cbm_free_result(r); + PASS(); +} + /* Issue #1009 (composed builders): a builder whose template inlines an earlier * builder's call plus a query string: `return \`${basePath(id)}?${params}\``. * The known-substitution is inlined and the query string is truncated, so the @@ -6812,6 +6855,8 @@ SUITE(extraction) { RUN_TEST(extract_go_binary_concat_url_issue1249); RUN_TEST(extract_go_binary_concat_url_no_literal_suffix_issue1249); RUN_TEST(extract_ts_url_builder_issue1009); + RUN_TEST(extract_ts_route_handler_after_named_middleware); + RUN_TEST(extract_ts_route_handler_after_inline_middleware); RUN_TEST(extract_ts_url_builder_composed_issue1009); RUN_TEST(extract_c_url_builder_gated_issue1009); RUN_TEST(extract_ts_url_builder_mixed_returns_issue1009);