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:30 UTC

(axis-axis2-c-core) 20/21: Apply Gemini D2 review findings

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 9a9f53001fe8fe5ce663934bc8ce091660a615c3
Author: Robert Lazarski <ro...@gmail.com>
AuthorDate: Tue Apr 7 06:56:36 2026 -1000

    Apply Gemini D2 review findings
    
    Major:
    - mcp_catalog_handler.c: capture json_cstr length eagerly into json_len
      before axutil_strdup/json_object_put so the log message does not use
      a pointer into freed json-c memory; add (axis2_char_t *) cast on the
      axutil_strdup void* return value.
    
    Minor:
    - financial_benchmark_service_handler.c: replace raw axutil_strdup error
      string in process_json_only() with axis2_json_secure_fault() for
      consistent correlation-ID error surface.
    - axis2_json_secure_fault.c: add #include <stdlib.h> explicitly; update
      fallback PRNG comment to note clock() returns (clock_t)-1 (not 0) on
      POSIX failure, and document the non-thread-safe characteristic.
    
    (Finding 6 — AXIS2_LOG_INFO missing AXIS2_LOG_SI — confirmed false
    positive: AXIS2_LOG_INFO maps to axutil_log_impl_log_info which takes
    (log, format, ...) with no file/line args, unlike AXIS2_LOG_WARNING/ERROR.)
    
    Co-Authored-By: Claude Sonnet 4.6 <no...@anthropic.com>
---
 .../financial-benchmark-service/src/axis2_json_secure_fault.c  |  7 ++++++-
 .../src/financial_benchmark_service_handler.c                  |  7 +++++--
 .../financial-benchmark-service/src/mcp_catalog_handler.c      | 10 ++++++++--
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/samples/user_guide/financial-benchmark-service/src/axis2_json_secure_fault.c b/samples/user_guide/financial-benchmark-service/src/axis2_json_secure_fault.c
index bce2a3c23..4e166a93e 100644
--- a/samples/user_guide/financial-benchmark-service/src/axis2_json_secure_fault.c
+++ b/samples/user_guide/financial-benchmark-service/src/axis2_json_secure_fault.c
@@ -44,6 +44,7 @@
 
 #include <stdio.h>
 #include <stdint.h>
+#include <stdlib.h>  /* size_t */
 #include <string.h>
 #include <time.h>
 
@@ -79,7 +80,11 @@ axis2_json_corr_id_generate(char *buf, size_t len)
 
     if (!got_random) {
         /* Fallback: mix time, clock, and pid with XOR shifts.
-         * Not cryptographically random — adequate for log correlation only. */
+         * Not cryptographically random — adequate for log correlation only.
+         * Note: clock() returns (clock_t)-1 on POSIX when unavailable, not 0;
+         * that sentinel still contributes non-zero bits after the XOR mix.
+         * Not thread-safe: concurrent fallback calls at the same instant may
+         * produce duplicate IDs; this is acceptable for log correlation. */
         uint64_t v = (uint64_t)time(NULL);
         v ^= (uint64_t)clock() << 17;
 #ifdef __linux__
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 429746f57..41d742942 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
@@ -229,9 +229,12 @@ financial_benchmark_service_process_json_only(
 
     if (!json_request)
     {
+        char corr_id[AXIS2_JSON_CORR_ID_LEN];
+        axis2_json_corr_id_generate(corr_id, sizeof(corr_id));
         AXIS2_LOG_ERROR(env->log, AXIS2_LOG_SI,
-                       "FinancialBenchmarkService: No JSON request provided");
-        return axutil_strdup(env, "{\"error\": \"No JSON request provided\"}");
+            "[FinancialBenchmark][%s] process_json_only: no JSON request",
+            corr_id);
+        return axis2_json_secure_fault(env, corr_id, "no request body");
     }
 
     /* Try to extract operation from JSON */
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 ff4866457..669607556 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
@@ -396,9 +396,15 @@ mcp_catalog_generate_json(
     const char *json_cstr =
             json_object_to_json_string_ext(root, JSON_C_TO_STRING_PLAIN);
 
+    /* Record length before json_object_put() invalidates json_cstr.
+     * json_object_to_json_string_ext() returns a pointer into root's internal
+     * buffer; any call to json_object_put(root) after this point makes it
+     * a dangling pointer.  Capture size eagerly so the log message is safe. */
+    size_t json_len = json_cstr ? strlen(json_cstr) : 0;
+
     axis2_char_t *result = NULL;
     if (json_cstr) {
-        result = axutil_strdup(env, json_cstr);
+        result = (axis2_char_t *)axutil_strdup(env, json_cstr);
         if (!result) {
             /* axutil_strdup OOM — log with correlation ID and return fault */
             char corr_id[AXIS2_JSON_CORR_ID_LEN];
@@ -406,7 +412,7 @@ mcp_catalog_generate_json(
             AXIS2_LOG_ERROR(env->log, AXIS2_LOG_SI,
                 "[MCP][%s] axutil_strdup failed serialising catalog "
                 "(%zu bytes) — out of memory",
-                corr_id, strlen(json_cstr));
+                corr_id, json_len);
             json_object_put(root);
             return axis2_json_secure_fault(env, corr_id,
                 "catalog generation failed — see server log");