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/08/25 13:53:00 UTC

(axis-axis2-c-core) 06/08: Give shared request objects exactly one owner at teardown

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 d5d60c9fd106c949c99e5fb17047b1bd00737b5e
Author: Robert Lazarski <ro...@gmail.com>
AuthorDate: Tue Aug 25 03:23:44 2026 -1000

    Give shared request objects exactly one owner at teardown
    
    The OUT context aliases the IN context's transport stream, transport
    info and To epr, so freeing it without the reset gives the IN context a
    second free of each, and both failure exits left it registered in the
    op_ctx for worker cleanup to free again. The JSON response was owned by
    two properties, and set_fault_to wrote the To field.
    
    Co-Authored-By: Claude Opus 5 (1M context) <no...@anthropic.com>
---
 src/core/context/msg_ctx.c                         |  7 +++-
 src/core/receivers/axis2_json_rpc_msg_recv.c       | 15 +++++++--
 src/core/receivers/msg_recv.c                      | 39 ++++++++++++++++++++--
 .../transport/http/common/simple_http_svr_conn.c   | 27 +++++++++++++++
 4 files changed, 82 insertions(+), 6 deletions(-)

diff --git a/src/core/context/msg_ctx.c b/src/core/context/msg_ctx.c
index f4c4a4074..387fc8794 100644
--- a/src/core/context/msg_ctx.c
+++ b/src/core/context/msg_ctx.c
@@ -775,7 +775,12 @@ axis2_msg_ctx_set_fault_to(
     AXIS2_PARAM_CHECK(env->error, msg_ctx, AXIS2_FAILURE);
     if(msg_ctx->msg_info_headers)
     {
-        return axis2_msg_info_headers_set_to(msg_ctx->msg_info_headers, env, reference);
+        /* set_fault_to, not set_to. This delegated to the To setter, so the
+         * field written was never the field axis2_msg_ctx_get_fault_to reads
+         * back -- fault_to stayed NULL while the epr landed in the To slot,
+         * where msg_info_headers_free treats it as its own and frees it out
+         * from under whoever passed it in. */
+        return axis2_msg_info_headers_set_fault_to(msg_ctx->msg_info_headers, env, reference);
     }
 
     return AXIS2_SUCCESS;
diff --git a/src/core/receivers/axis2_json_rpc_msg_recv.c b/src/core/receivers/axis2_json_rpc_msg_recv.c
index a930f8e86..42da1f7ae 100644
--- a/src/core/receivers/axis2_json_rpc_msg_recv.c
+++ b/src/core/receivers/axis2_json_rpc_msg_recv.c
@@ -913,8 +913,16 @@ response_ready:
             "[JSON RPC MSG RECV] DEBUG: About to ALSO set JSON_RESPONSE property in in_msg_ctx: %p", (void*)in_msg_ctx);
         if (in_msg_ctx && in_msg_ctx != out_msg_ctx) {
             // Create another property object for in_msg_ctx (don't reuse the same property)
+            /* This must be a copy of the string, not the same pointer. A property
+             * created here defaults to own_value AXIS2_TRUE with no free_func, so
+             * axutil_property_free hands its value straight to AXIS2_FREE. Both
+             * contexts are torn down at the end of the operation, so sharing one
+             * allocation between the two properties frees it twice. Copying is
+             * what keeps "one allocation, one owner" true on the normal in-out
+             * path, which is every request where out_msg_ctx is distinct. */
             axutil_property_t* json_prop_in = axutil_property_create(env);
-            axutil_property_set_value(json_prop_in, env, json_response);
+            axis2_char_t* json_response_in = axutil_strdup(env, json_response);
+            axutil_property_set_value(json_prop_in, env, json_response_in);
             axis2_msg_ctx_set_property(in_msg_ctx, env, "JSON_RESPONSE", json_prop_in);
             AXIS2_LOG_ERROR(env->log, AXIS2_LOG_SI,
                 "[JSON RPC MSG RECV] DEBUG: Set JSON_RESPONSE property in in_msg_ctx: %p", (void*)in_msg_ctx);
@@ -927,7 +935,10 @@ response_ready:
                 void* verification_value_in = axutil_property_get_value(verification_prop_in, env);
                 AXIS2_LOG_ERROR(env->log, AXIS2_LOG_SI,
                     "[JSON RPC MSG RECV] DEBUG: Verification - JSON_RESPONSE value in in_msg_ctx: %p", verification_value_in);
-                if (verification_value_in == json_response) {
+                /* Compare against the copy handed to this property, not the
+                 * out_msg_ctx allocation -- they are deliberately no longer the
+                 * same pointer. */
+                if (verification_value_in == json_response_in) {
                     AXIS2_LOG_ERROR(env->log, AXIS2_LOG_SI,
                         "[JSON RPC MSG RECV] DEBUG: SUCCESS: JSON_RESPONSE property correctly stored in in_msg_ctx");
                 } else {
diff --git a/src/core/receivers/msg_recv.c b/src/core/receivers/msg_recv.c
index c64ab7eb2..bcd1e731d 100644
--- a/src/core/receivers/msg_recv.c
+++ b/src/core/receivers/msg_recv.c
@@ -387,6 +387,40 @@ axis2_msg_recv_delete_svc_obj(
     return axutil_class_loader_delete_dll(env, dll_desc);
 }
 
+/**
+ * Discard the OUT context after it has been registered with the op_ctx.
+ *
+ * Two things have to happen before the free, and every failure exit below
+ * needs both. The reset is what breaks the aliasing that create_out_msg_ctx
+ * set up: the OUT context shares its transport_out_stream, out_transport_info
+ * and To epr with the IN context, and axis2_msg_ctx_free releases all of them
+ * unconditionally, so freeing without the reset hands the IN context's teardown
+ * a second free of each. Clearing the op_ctx slot matters just as much --
+ * axis2_op_ctx_add_msg_ctx stores the bare pointer and takes no reference, so a
+ * slot left populated is a dangling pointer that worker cleanup reads and frees
+ * again.
+ */
+static void
+axis2_msg_recv_discard_out_msg_ctx(
+    const axutil_env_t * env,
+    axis2_op_ctx_t * op_ctx,
+    axis2_msg_ctx_t * out_msg_ctx)
+{
+    axis2_core_utils_reset_out_msg_ctx(env, out_msg_ctx);
+
+    if(op_ctx)
+    {
+        axis2_msg_ctx_t **msg_ctx_map = axis2_op_ctx_get_msg_ctx_map(op_ctx, env);
+
+        if(msg_ctx_map && msg_ctx_map[AXIS2_WSDL_MESSAGE_LABEL_OUT] == out_msg_ctx)
+        {
+            msg_ctx_map[AXIS2_WSDL_MESSAGE_LABEL_OUT] = NULL;
+        }
+    }
+
+    axis2_msg_ctx_free(out_msg_ctx, env);
+}
+
 static axis2_status_t AXIS2_CALL
 axis2_msg_recv_receive_impl(
     axis2_msg_recv_t * msg_recv,
@@ -434,8 +468,7 @@ axis2_msg_recv_receive_impl(
     status = axis2_msg_recv_invoke_business_logic(msg_recv, env, msg_ctx, out_msg_ctx);
     if(AXIS2_SUCCESS != status)
     {
-        axis2_core_utils_reset_out_msg_ctx(env, out_msg_ctx);
-        axis2_msg_ctx_free(out_msg_ctx, env);
+        axis2_msg_recv_discard_out_msg_ctx(env, op_ctx, out_msg_ctx);
         return status;
     }
     svc_ctx = axis2_op_ctx_get_parent(op_ctx, env);
@@ -443,7 +476,7 @@ axis2_msg_recv_receive_impl(
     engine = axis2_engine_create(env, conf_ctx);
     if(!engine)
     {
-        axis2_msg_ctx_free(out_msg_ctx, env);
+        axis2_msg_recv_discard_out_msg_ctx(env, op_ctx, out_msg_ctx);
         return AXIS2_FAILURE;
     }
     if(axis2_msg_ctx_get_soap_envelope(out_msg_ctx, env))
diff --git a/src/core/transport/http/common/simple_http_svr_conn.c b/src/core/transport/http/common/simple_http_svr_conn.c
index 4aa71e61b..0fb654570 100644
--- a/src/core/transport/http/common/simple_http_svr_conn.c
+++ b/src/core/transport/http/common/simple_http_svr_conn.c
@@ -411,6 +411,17 @@ axis2_simple_http_svr_conn_get_peer_ip(
     return axutil_network_handler_get_peer_ip(env, svr_conn->socket);
 }
 
+/* Longest request or header line this server will assemble, in bytes.
+ *
+ * There was no bound at all: the loop below appends every 2047-byte read onto
+ * str_line until it finds a CRLF, so a peer that never sends one decides how
+ * much the server allocates -- and each append is a fresh copy of everything
+ * accumulated so far, so the cost is quadratic well before memory runs out.
+ * maxRequestSize does not help here; it bounds bodies, not the header block.
+ * 8KB matches the httpd defaults for LimitRequestLine and
+ * LimitRequestFieldSize, which is the limit clients are already built for. */
+#define AXIS2_HTTP_MAX_LINE_LENGTH (8 * 1024)
+
 static axis2_char_t *
 axis2_simple_http_svr_conn_read_line(
     axis2_simple_http_svr_conn_t * svr_conn,
@@ -419,6 +430,7 @@ axis2_simple_http_svr_conn_read_line(
     axis2_char_t* str_line = NULL;
     axis2_char_t tmp_buf[2048];
     int read = -1;
+    size_t accumulated = 0;
 
     /* peek for 2047 characters to verify whether it contains CRLF character */
     while((read = axutil_stream_peek(svr_conn->stream, env, tmp_buf, 2048 - 1)) > 0)
@@ -486,6 +498,21 @@ axis2_simple_http_svr_conn_read_line(
             if(read > 0)
             {
                 axis2_char_t* tmp_str_line = NULL;
+
+                accumulated += (size_t)read;
+                if(accumulated > AXIS2_HTTP_MAX_LINE_LENGTH)
+                {
+                    AXIS2_LOG_WARNING(env->log, AXIS2_LOG_SI,
+                        "Refusing HTTP line longer than %d bytes",
+                        AXIS2_HTTP_MAX_LINE_LENGTH);
+                    if(str_line)
+                    {
+                        AXIS2_FREE(env->allocator, str_line);
+                        str_line = NULL;
+                    }
+                    return NULL;
+                }
+
                 tmp_buf[read] = AXIS2_ESC_NULL;
                 tmp_str_line = axutil_stracat(env, str_line, tmp_buf);
                 if(tmp_str_line)