You are viewing a plain text version of this content. The canonical link for it is here.
Posted to c-commits@axis.apache.org by ro...@apache.org on 2026/04/07 20:44:29 UTC
(axis-axis2-c-core) 19/21: Apply Gemini review findings — C mcp_catalog_handler + D1 handler wiring
This is an automated email from the ASF dual-hosted git repository.
robertlazarski pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/axis-axis2-c-core.git
commit 3915982cca5c13941a88bd138e18e8121c43df6a
Author: Robert Lazarski <ro...@gmail.com>
AuthorDate: Tue Apr 7 06:54:21 2026 -1000
Apply Gemini review findings — C mcp_catalog_handler + D1 handler wiring
Critical:
- mcp_catalog_check_and_respond: return AXIS2_FALSE (not AXIS2_TRUE) when
mcp_catalog_generate_json returns NULL; previously told caller a response
was ready but *out_json was NULL.
Major:
- mcp_catalog_check_and_respond: add !env guard before AXIS2_LOG_DEBUG
dereference; change strstr path match to suffix match (strcmp on tail
of path) to prevent path-confusion info-disclosure via embedded subpath.
- financial_benchmark_service_handler.c: remove redundant outer strstr
(mcp_catalog_check_and_respond is now the sole path gatekeeper); use
catalog_json ? strlen(catalog_json) : 0u in log call for null safety.
Minor:
- mcp_catalog_handler.c: add #include <stdio.h> (snprintf guarantee);
drop unnecessary (axutil_qname_t *) non-const casts on svc_qname and
op_qname (axutil_qname_get_localpart accepts const).
Co-Authored-By: Claude Sonnet 4.6 <no...@anthropic.com>
---
.../src/financial_benchmark_service_handler.c | 12 ++++----
.../src/mcp_catalog_handler.c | 35 ++++++++++++++++------
2 files changed, 33 insertions(+), 14 deletions(-)
diff --git a/samples/user_guide/financial-benchmark-service/src/financial_benchmark_service_handler.c b/samples/user_guide/financial-benchmark-service/src/financial_benchmark_service_handler.c
index ceaf7f84b..429746f57 100644
--- a/samples/user_guide/financial-benchmark-service/src/financial_benchmark_service_handler.c
+++ b/samples/user_guide/financial-benchmark-service/src/financial_benchmark_service_handler.c
@@ -151,20 +151,22 @@ financial_benchmark_service_invoke_json(
if (to)
request_uri = axis2_endpoint_ref_get_address(to, env);
- if (request_uri && strstr(request_uri, MCP_CATALOG_PATH))
+ if (request_uri)
{
axis2_conf_ctx_t *conf_ctx = axis2_msg_ctx_get_conf_ctx(msg_ctx, env);
axis2_conf_t *conf = conf_ctx
? axis2_conf_ctx_get_conf(conf_ctx, env)
: NULL;
+ /* mcp_catalog_check_and_respond does suffix-based path matching
+ * internally (no redundant strstr here). Returns AXIS2_FALSE when
+ * path does not match OR when catalog generation fails (NULL output),
+ * so catalog_json is guaranteed non-NULL on AXIS2_TRUE. */
axis2_char_t *catalog_json = NULL;
- if (mcp_catalog_check_and_respond(env, conf, request_uri,
- &catalog_json)
- && catalog_json)
+ if (mcp_catalog_check_and_respond(env, conf, request_uri, &catalog_json))
{
AXIS2_LOG_INFO(env->log,
"FinancialBenchmarkService: Returning MCP catalog (%zu bytes)",
- strlen(catalog_json));
+ catalog_json ? strlen(catalog_json) : 0u);
return catalog_json; /* caller (msg_recv) frees via AXIS2_FREE */
}
}
diff --git a/samples/user_guide/financial-benchmark-service/src/mcp_catalog_handler.c b/samples/user_guide/financial-benchmark-service/src/mcp_catalog_handler.c
index b1720fa83..ff4866457 100644
--- a/samples/user_guide/financial-benchmark-service/src/mcp_catalog_handler.c
+++ b/samples/user_guide/financial-benchmark-service/src/mcp_catalog_handler.c
@@ -56,6 +56,7 @@
#include <axutil_log.h>
#include <json-c/json.h>
+#include <stdio.h> /* snprintf */
#include <string.h>
#include <ctype.h>
@@ -297,8 +298,7 @@ mcp_catalog_generate_json(
const axutil_qname_t *svc_qname = axis2_svc_get_qname(svc, env);
if (!svc_qname) continue;
const axis2_char_t *svc_name =
- axutil_qname_get_localpart(
- (axutil_qname_t *)svc_qname, env);
+ axutil_qname_get_localpart(svc_qname, env);
if (!svc_name) continue;
/* Skip system services */
@@ -322,8 +322,7 @@ mcp_catalog_generate_json(
const axutil_qname_t *op_qname = axis2_op_get_qname(op, env);
if (!op_qname) continue;
const axis2_char_t *op_name =
- axutil_qname_get_localpart(
- (axutil_qname_t *)op_qname, env);
+ axutil_qname_get_localpart(op_qname, env);
if (!op_name) continue;
/* Build endpoint path: POST /services/SvcName/opName */
@@ -436,15 +435,33 @@ mcp_catalog_check_and_respond(
const axis2_char_t *request_path,
axis2_char_t **out_json)
{
- if (!request_path || !out_json) return AXIS2_FALSE;
-
- /* Match suffix: path may be "/axis2/_mcp/openapi-mcp.json" or just the
- * bare catalog path — check strstr so context-path prefixes are handled. */
- if (!strstr(request_path, MCP_CATALOG_PATH)) return AXIS2_FALSE;
+ if (!env || !request_path || !out_json) return AXIS2_FALSE;
+
+ /* Suffix match: the path must END with MCP_CATALOG_PATH so that a
+ * path like /evil/_mcp/openapi-mcp.json/../../admin (which contains
+ * the catalog path as a substring but not as a suffix) does not match.
+ * strstr-anywhere matching would allow path-confusion info disclosure. */
+ size_t path_len = strlen(request_path);
+ size_t catalog_len = strlen(MCP_CATALOG_PATH);
+ if (path_len < catalog_len) return AXIS2_FALSE;
+ if (strcmp(request_path + path_len - catalog_len, MCP_CATALOG_PATH) != 0)
+ return AXIS2_FALSE;
AXIS2_LOG_DEBUG(env->log, AXIS2_LOG_SI,
"[MCP] Catalog request matched path '%s'", request_path);
*out_json = mcp_catalog_generate_json(conf, env);
+
+ /* mcp_catalog_generate_json returns NULL on conf==NULL, env==NULL, or OOM.
+ * Return AXIS2_FALSE rather than AXIS2_TRUE with a NULL response pointer
+ * so the caller falls through to normal dispatch instead of sending NULL. */
+ if (!*out_json) {
+ AXIS2_LOG_ERROR(env->log, AXIS2_LOG_SI,
+ "[MCP] mcp_catalog_generate_json returned NULL for path '%s' "
+ "(conf=%p) — falling through to normal dispatch",
+ request_path, (void *)conf);
+ return AXIS2_FALSE;
+ }
+
return AXIS2_TRUE;
}