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-java-core.git
commit 17dadeae927f5333d7b5e2c1c06df2ad6f2ccbec Author: Robert Lazarski <[email protected]> AuthorDate: Tue Apr 7 01:29:27 2026 -1000 JSON-RPC error hardening: address Gemini review findings Finding 1 (code duplication): extract JsonUtils.createSecureFault() shared helper so both receivers use one implementation of the correlation ID pattern. Finding 2 (null operation_name): createSecureFault() substitutes "<unknown>" when operation_name is null, preventing confusing "null" in log lines. Finding 3 (Object[] not type-safe): replace postForResponse() return type with typed UtilTest.TestResponse; callers use result.body / result.statusCode instead of casting Object[]. Finding 4 (other catch blocks also leak): apply createSecureFault() to IllegalAccessException and InvocationTargetException paths in both receivers (was still using AxisFault.makeFault(e), which may serialise class names). Use multi-catch to reduce boilerplate. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> --- .../gson/rpc/JsonInOnlyRPCMessageReceiver.java | 23 +++-------------- .../json/gson/rpc/JsonRpcMessageReceiver.java | 23 +++-------------- .../org/apache/axis2/json/gson/rpc/JsonUtils.java | 30 ++++++++++++++++++++++ .../test/org/apache/axis2/json/gson/UtilTest.java | 28 ++++++++++++-------- .../json/gson/rpc/JSONRPCIntegrationTest.java | 24 ++++++++--------- 5 files changed, 65 insertions(+), 63 deletions(-) diff --git a/modules/json/src/org/apache/axis2/json/gson/rpc/JsonInOnlyRPCMessageReceiver.java b/modules/json/src/org/apache/axis2/json/gson/rpc/JsonInOnlyRPCMessageReceiver.java index 4b8873443f..7e528ea551 100644 --- a/modules/json/src/org/apache/axis2/json/gson/rpc/JsonInOnlyRPCMessageReceiver.java +++ b/modules/json/src/org/apache/axis2/json/gson/rpc/JsonInOnlyRPCMessageReceiver.java @@ -32,7 +32,6 @@ import org.apache.commons.logging.LogFactory; import java.io.IOException; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.util.UUID; public class JsonInOnlyRPCMessageReceiver extends RPCInOnlyMessageReceiver { private static final Log log = LogFactory.getLog(JsonInOnlyRPCMessageReceiver.class); @@ -71,7 +70,6 @@ public class JsonInOnlyRPCMessageReceiver extends RPCInOnlyMessageReceiver { } public void invokeService(JsonReader jsonReader, Object serviceObj, String operation_name, String enableJSONOnly) throws AxisFault { - String msg; Class implClass = serviceObj.getClass(); Method[] allMethods = implClass.getDeclaredMethods(); Method method = JsonUtils.getOpMethod(operation_name, allMethods); @@ -79,25 +77,10 @@ public class JsonInOnlyRPCMessageReceiver extends RPCInOnlyMessageReceiver { try { int paramCount = paramClasses.length; JsonUtils.invokeServiceClass(jsonReader, serviceObj, method, paramClasses, paramCount, enableJSONOnly); - } catch (IllegalAccessException e) { - msg = "Does not have access to " + - "the definition of the specified class, field, method or constructor"; - log.error(msg, e); - throw AxisFault.makeFault(e); - - } catch (InvocationTargetException e) { - msg = "Exception occurred while trying to invoke service method " + - (method != null ? method.getName() : "null"); - log.error(msg, e); - throw AxisFault.makeFault(e); + } catch (IllegalAccessException | InvocationTargetException e) { + throw JsonUtils.createSecureFault(e, operation_name, false); } catch (IOException e) { - // Correlation ID: full error context is logged server-side; only the - // opaque reference is returned to the client so malformed-request - // failures remain safe under penetration testing. - String errorRef = UUID.randomUUID().toString(); - log.error("[errorRef=" + errorRef + "] Bad Request parsing JSON-RPC body " + - "for operation '" + operation_name + "': " + e.getMessage(), e); - throw new AxisFault("Bad Request [errorRef=" + errorRef + "]"); + throw JsonUtils.createSecureFault(e, operation_name, true); } } } diff --git a/modules/json/src/org/apache/axis2/json/gson/rpc/JsonRpcMessageReceiver.java b/modules/json/src/org/apache/axis2/json/gson/rpc/JsonRpcMessageReceiver.java index 972cc9da21..fdf5233fcc 100644 --- a/modules/json/src/org/apache/axis2/json/gson/rpc/JsonRpcMessageReceiver.java +++ b/modules/json/src/org/apache/axis2/json/gson/rpc/JsonRpcMessageReceiver.java @@ -31,7 +31,6 @@ import org.apache.commons.logging.LogFactory; import java.io.IOException; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.util.UUID; public class JsonRpcMessageReceiver extends RPCMessageReceiver { @@ -70,7 +69,6 @@ public class JsonRpcMessageReceiver extends RPCMessageReceiver { } public void invokeService(JsonReader jsonReader, Object serviceObj, String operation_name, MessageContext outMes, String enableJSONOnly) throws AxisFault { - String msg; Class implClass = serviceObj.getClass(); Method[] allMethods = implClass.getDeclaredMethods(); Method method = JsonUtils.getOpMethod(operation_name, allMethods); @@ -83,25 +81,10 @@ public class JsonRpcMessageReceiver extends RPCMessageReceiver { outMes.setProperty(JsonConstant.RETURN_OBJECT, retObj); outMes.setProperty(JsonConstant.RETURN_TYPE, method.getReturnType()); - } catch (IllegalAccessException e) { - msg = "Does not have access to " + - "the definition of the specified class, field, method or constructor"; - log.error(msg, e); - throw AxisFault.makeFault(e); - - } catch (InvocationTargetException e) { - msg = "Exception occurred while trying to invoke service method " + - (method != null ? method.getName() : "null"); - log.error(msg, e); - throw AxisFault.makeFault(e); + } catch (IllegalAccessException | InvocationTargetException e) { + throw JsonUtils.createSecureFault(e, operation_name, false); } catch (IOException e) { - // Correlation ID: full error context is logged server-side; only the - // opaque reference is returned to the client so malformed-request - // failures remain safe under penetration testing. - String errorRef = UUID.randomUUID().toString(); - log.error("[errorRef=" + errorRef + "] Bad Request parsing JSON-RPC body " + - "for operation '" + operation_name + "': " + e.getMessage(), e); - throw new AxisFault("Bad Request [errorRef=" + errorRef + "]"); + throw JsonUtils.createSecureFault(e, operation_name, true); } } } diff --git a/modules/json/src/org/apache/axis2/json/gson/rpc/JsonUtils.java b/modules/json/src/org/apache/axis2/json/gson/rpc/JsonUtils.java index aca3de9ca9..8720d4a3ad 100644 --- a/modules/json/src/org/apache/axis2/json/gson/rpc/JsonUtils.java +++ b/modules/json/src/org/apache/axis2/json/gson/rpc/JsonUtils.java @@ -25,9 +25,12 @@ import org.apache.commons.logging.Log; import com.google.gson.Gson; import com.google.gson.stream.JsonReader; +import org.apache.axis2.AxisFault; + import java.io.IOException; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.UUID; public class JsonUtils { @@ -85,6 +88,33 @@ public class JsonUtils { } + /** + * Build a secure {@link AxisFault} for any unexpected failure in the JSON-RPC + * message receivers. The full context is logged server-side under an opaque + * correlation ID; only {@code "Bad Request [errorRef=<uuid>]"} or + * {@code "Internal Server Error [errorRef=<uuid>]"} is returned to the caller. + * This prevents information disclosure under penetration testing (CWE-209). + * + * @param e the caught exception + * @param operationName the Axis2 operation being dispatched (may be null) + * @param isParsingError {@code true} for malformed-input IOExceptions, + * {@code false} for internal reflection/invocation failures + * @return an AxisFault safe to send to the client + */ + static AxisFault createSecureFault(Exception e, String operationName, boolean isParsingError) { + String errorRef = UUID.randomUUID().toString(); + String opDisplay = operationName != null ? operationName : "<unknown>"; + if (isParsingError) { + log.error("[errorRef=" + errorRef + "] Bad Request parsing JSON-RPC body " + + "for operation '" + opDisplay + "': " + e.getMessage(), e); + return new AxisFault("Bad Request [errorRef=" + errorRef + "]"); + } else { + log.error("[errorRef=" + errorRef + "] Internal error invoking operation '" + + opDisplay + "': " + e.getMessage(), e); + return new AxisFault("Internal Server Error [errorRef=" + errorRef + "]"); + } + } + public static Method getOpMethod(String methodName, Method[] methodSet) { for (Method method : methodSet) { String mName = method.getName(); diff --git a/modules/json/test/org/apache/axis2/json/gson/UtilTest.java b/modules/json/test/org/apache/axis2/json/gson/UtilTest.java index 7592d39243..c8a94939a6 100644 --- a/modules/json/test/org/apache/axis2/json/gson/UtilTest.java +++ b/modules/json/test/org/apache/axis2/json/gson/UtilTest.java @@ -33,26 +33,32 @@ import java.io.IOException; public class UtilTest { + /** Immutable holder for an HTTP response used in error-path tests. */ + public static class TestResponse { + public final int statusCode; + public final String body; + public TestResponse(int statusCode, String body) { + this.statusCode = statusCode; + this.body = body; + } + } + /** - * Post {@code jsonString} to {@code strURL} and return a two-element array: - * {@code [statusCode, responseBody]}. Unlike {@link #post}, this method - * does NOT throw on non-2xx status codes — callers that test error paths - * need the response body even when HTTP 500 is returned. + * Post {@code jsonString} to {@code strURL} and return the full response. + * Unlike {@link #post}, this method does NOT throw on non-2xx status codes — + * callers that test error paths need the response body even on HTTP 500. */ - public static Object[] postForResponse(String jsonString, String strURL) + public static TestResponse postForResponse(String jsonString, String strURL) throws IOException { HttpEntity stringEntity = new StringEntity(jsonString, ContentType.APPLICATION_JSON); HttpPost httpPost = new HttpPost(strURL); httpPost.setEntity(stringEntity); - CloseableHttpClient httpclient = HttpClients.createDefault(); - try { - CloseableHttpResponse response = httpclient.execute(httpPost); + try (CloseableHttpClient httpclient = HttpClients.createDefault(); + CloseableHttpResponse response = httpclient.execute(httpPost)) { int status = response.getCode(); HttpEntity entity = response.getEntity(); String body = entity != null ? EntityUtils.toString(entity, "UTF-8") : ""; - return new Object[]{status, body}; - } finally { - httpclient.close(); + return new TestResponse(status, body); } } diff --git a/modules/json/test/org/apache/axis2/json/gson/rpc/JSONRPCIntegrationTest.java b/modules/json/test/org/apache/axis2/json/gson/rpc/JSONRPCIntegrationTest.java index 94984b8f60..514c62b327 100644 --- a/modules/json/test/org/apache/axis2/json/gson/rpc/JSONRPCIntegrationTest.java +++ b/modules/json/test/org/apache/axis2/json/gson/rpc/JSONRPCIntegrationTest.java @@ -63,8 +63,8 @@ public class JSONRPCIntegrationTest { @Test public void testMalformedJsonBodyReturnsBadRequest() throws Exception { String echoPersonUrl = server.getEndpoint("JSONPOJOService") + "echoPerson"; - Object[] result = UtilTest.postForResponse("NOT_VALID_JSON", echoPersonUrl); - String body = (String) result[1]; + UtilTest.TestResponse result = UtilTest.postForResponse("NOT_VALID_JSON", echoPersonUrl); + String body = result.body; Assert.assertTrue("Response must contain 'Bad Request' for malformed JSON body", body.contains("Bad Request")); } @@ -77,8 +77,8 @@ public class JSONRPCIntegrationTest { @Test public void testMalformedJsonBodyIncludesCorrelationId() throws Exception { String echoPersonUrl = server.getEndpoint("JSONPOJOService") + "echoPerson"; - Object[] result = UtilTest.postForResponse("NOT_VALID_JSON", echoPersonUrl); - String body = (String) result[1]; + UtilTest.TestResponse result = UtilTest.postForResponse("NOT_VALID_JSON", echoPersonUrl); + String body = result.body; Assert.assertTrue("Response must contain 'errorRef=' correlation ID", body.contains("errorRef=")); } @@ -91,8 +91,8 @@ public class JSONRPCIntegrationTest { @Test public void testMalformedJsonBodyCorrelationIdIsUuid() throws Exception { String echoPersonUrl = server.getEndpoint("JSONPOJOService") + "echoPerson"; - Object[] result = UtilTest.postForResponse("NOT_VALID_JSON", echoPersonUrl); - String body = (String) result[1]; + UtilTest.TestResponse result = UtilTest.postForResponse("NOT_VALID_JSON", echoPersonUrl); + String body = result.body; Assert.assertTrue("errorRef in fault must be a UUID", UUID_PATTERN.matcher(body).find()); } @@ -107,8 +107,8 @@ public class JSONRPCIntegrationTest { // Valid JSON but wrong envelope: missing the [{...}] wrapper String badEnvelope = "{\"echoPerson\":{\"name\":\"Simon\"}}"; String echoPersonUrl = server.getEndpoint("JSONPOJOService") + "echoPerson"; - Object[] result = UtilTest.postForResponse(badEnvelope, echoPersonUrl); - String body = (String) result[1]; + UtilTest.TestResponse result = UtilTest.postForResponse(badEnvelope, echoPersonUrl); + String body = result.body; Assert.assertTrue("Wrong-envelope request must return 'Bad Request'", body.contains("Bad Request")); Assert.assertTrue("Wrong-envelope response must contain an errorRef", @@ -123,8 +123,8 @@ public class JSONRPCIntegrationTest { @Test public void testMalformedJsonBodyDoesNotLeakExceptionClassName() throws Exception { String echoPersonUrl = server.getEndpoint("JSONPOJOService") + "echoPerson"; - Object[] result = UtilTest.postForResponse("NOT_VALID_JSON", echoPersonUrl); - String body = (String) result[1]; + UtilTest.TestResponse result = UtilTest.postForResponse("NOT_VALID_JSON", echoPersonUrl); + String body = result.body; Assert.assertFalse("Response must not leak 'MalformedJsonException'", body.contains("MalformedJsonException")); Assert.assertFalse("Response must not leak 'IOException'", @@ -140,8 +140,8 @@ public class JSONRPCIntegrationTest { @Test public void testInOnlyMalformedJsonBodyReturnsBadRequestWithCorrelationId() throws Exception { String pingUrl = server.getEndpoint("JSONPOJOService") + "ping"; - Object[] result = UtilTest.postForResponse("NOT_VALID_JSON", pingUrl); - String body = (String) result[1]; + UtilTest.TestResponse result = UtilTest.postForResponse("NOT_VALID_JSON", pingUrl); + String body = result.body; Assert.assertTrue("InOnly malformed request must return 'Bad Request'", body.contains("Bad Request")); Assert.assertTrue("InOnly malformed request must contain an errorRef",
