Skip to content

Commit 17dadea

Browse files
robertlazarskiclaude
robertlazarski
and
claude
committed
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 <noreply@anthropic.com>
1 parent 02845d0 commit 17dadea

5 files changed

Lines changed: 65 additions & 63 deletions

File tree

‎modules/json/src/org/apache/axis2/json/gson/rpc/JsonInOnlyRPCMessageReceiver.java‎

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import java.io.IOException;
3333
import java.lang.reflect.InvocationTargetException;
3434
import java.lang.reflect.Method;
35-
import java.util.UUID;
3635

3736
public class JsonInOnlyRPCMessageReceiver extends RPCInOnlyMessageReceiver {
3837
private static final Log log = LogFactory.getLog(JsonInOnlyRPCMessageReceiver.class);
@@ -71,33 +70,17 @@ public void invokeBusinessLogic(MessageContext inMessage) throws AxisFault {
7170
}
7271

7372
public void invokeService(JsonReader jsonReader, Object serviceObj, String operation_name, String enableJSONOnly) throws AxisFault {
74-
String msg;
7573
Class implClass = serviceObj.getClass();
7674
Method[] allMethods = implClass.getDeclaredMethods();
7775
Method method = JsonUtils.getOpMethod(operation_name, allMethods);
7876
Class[] paramClasses = method.getParameterTypes();
7977
try {
8078
int paramCount = paramClasses.length;
8179
JsonUtils.invokeServiceClass(jsonReader, serviceObj, method, paramClasses, paramCount, enableJSONOnly);
82-
} catch (IllegalAccessException e) {
83-
msg = "Does not have access to " +
84-
"the definition of the specified class, field, method or constructor";
85-
log.error(msg, e);
86-
throw AxisFault.makeFault(e);
87-
88-
} catch (InvocationTargetException e) {
89-
msg = "Exception occurred while trying to invoke service method " +
90-
(method != null ? method.getName() : "null");
91-
log.error(msg, e);
92-
throw AxisFault.makeFault(e);
80+
} catch (IllegalAccessException | InvocationTargetException e) {
81+
throw JsonUtils.createSecureFault(e, operation_name, false);
9382
} catch (IOException e) {
94-
// Correlation ID: full error context is logged server-side; only the
95-
// opaque reference is returned to the client so malformed-request
96-
// failures remain safe under penetration testing.
97-
String errorRef = UUID.randomUUID().toString();
98-
log.error("[errorRef=" + errorRef + "] Bad Request parsing JSON-RPC body " +
99-
"for operation '" + operation_name + "': " + e.getMessage(), e);
100-
throw new AxisFault("Bad Request [errorRef=" + errorRef + "]");
83+
throw JsonUtils.createSecureFault(e, operation_name, true);
10184
}
10285
}
10386
}

‎modules/json/src/org/apache/axis2/json/gson/rpc/JsonRpcMessageReceiver.java‎

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import java.io.IOException;
3232
import java.lang.reflect.InvocationTargetException;
3333
import java.lang.reflect.Method;
34-
import java.util.UUID;
3534

3635

3736
public class JsonRpcMessageReceiver extends RPCMessageReceiver {
@@ -70,7 +69,6 @@ public void invokeBusinessLogic(MessageContext inMessage, MessageContext outMess
7069
}
7170

7271
public void invokeService(JsonReader jsonReader, Object serviceObj, String operation_name, MessageContext outMes, String enableJSONOnly) throws AxisFault {
73-
String msg;
7472
Class implClass = serviceObj.getClass();
7573
Method[] allMethods = implClass.getDeclaredMethods();
7674
Method method = JsonUtils.getOpMethod(operation_name, allMethods);
@@ -83,25 +81,10 @@ public void invokeService(JsonReader jsonReader, Object serviceObj, String opera
8381
outMes.setProperty(JsonConstant.RETURN_OBJECT, retObj);
8482
outMes.setProperty(JsonConstant.RETURN_TYPE, method.getReturnType());
8583

86-
} catch (IllegalAccessException e) {
87-
msg = "Does not have access to " +
88-
"the definition of the specified class, field, method or constructor";
89-
log.error(msg, e);
90-
throw AxisFault.makeFault(e);
91-
92-
} catch (InvocationTargetException e) {
93-
msg = "Exception occurred while trying to invoke service method " +
94-
(method != null ? method.getName() : "null");
95-
log.error(msg, e);
96-
throw AxisFault.makeFault(e);
84+
} catch (IllegalAccessException | InvocationTargetException e) {
85+
throw JsonUtils.createSecureFault(e, operation_name, false);
9786
} catch (IOException e) {
98-
// Correlation ID: full error context is logged server-side; only the
99-
// opaque reference is returned to the client so malformed-request
100-
// failures remain safe under penetration testing.
101-
String errorRef = UUID.randomUUID().toString();
102-
log.error("[errorRef=" + errorRef + "] Bad Request parsing JSON-RPC body " +
103-
"for operation '" + operation_name + "': " + e.getMessage(), e);
104-
throw new AxisFault("Bad Request [errorRef=" + errorRef + "]");
87+
throw JsonUtils.createSecureFault(e, operation_name, true);
10588
}
10689
}
10790
}

‎modules/json/src/org/apache/axis2/json/gson/rpc/JsonUtils.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,12 @@
2525
import com.google.gson.Gson;
2626
import com.google.gson.stream.JsonReader;
2727

28+
import org.apache.axis2.AxisFault;
29+
2830
import java.io.IOException;
2931
import java.lang.reflect.InvocationTargetException;
3032
import java.lang.reflect.Method;
33+
import java.util.UUID;
3134

3235

3336
public class JsonUtils {
@@ -85,6 +88,33 @@ public static Object invokeServiceClass(JsonReader jsonReader,
8588

8689
}
8790

91+
/**
92+
* Build a secure {@link AxisFault} for any unexpected failure in the JSON-RPC
93+
* message receivers. The full context is logged server-side under an opaque
94+
* correlation ID; only {@code "Bad Request [errorRef=<uuid>]"} or
95+
* {@code "Internal Server Error [errorRef=<uuid>]"} is returned to the caller.
96+
* This prevents information disclosure under penetration testing (CWE-209).
97+
*
98+
* @param e the caught exception
99+
* @param operationName the Axis2 operation being dispatched (may be null)
100+
* @param isParsingError {@code true} for malformed-input IOExceptions,
101+
* {@code false} for internal reflection/invocation failures
102+
* @return an AxisFault safe to send to the client
103+
*/
104+
static AxisFault createSecureFault(Exception e, String operationName, boolean isParsingError) {
105+
String errorRef = UUID.randomUUID().toString();
106+
String opDisplay = operationName != null ? operationName : "<unknown>";
107+
if (isParsingError) {
108+
log.error("[errorRef=" + errorRef + "] Bad Request parsing JSON-RPC body " +
109+
"for operation '" + opDisplay + "': " + e.getMessage(), e);
110+
return new AxisFault("Bad Request [errorRef=" + errorRef + "]");
111+
} else {
112+
log.error("[errorRef=" + errorRef + "] Internal error invoking operation '" +
113+
opDisplay + "': " + e.getMessage(), e);
114+
return new AxisFault("Internal Server Error [errorRef=" + errorRef + "]");
115+
}
116+
}
117+
88118
public static Method getOpMethod(String methodName, Method[] methodSet) {
89119
for (Method method : methodSet) {
90120
String mName = method.getName();

‎modules/json/test/org/apache/axis2/json/gson/UtilTest.java‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,26 +33,32 @@
3333

3434
public class UtilTest {
3535

36+
/** Immutable holder for an HTTP response used in error-path tests. */
37+
public static class TestResponse {
38+
public final int statusCode;
39+
public final String body;
40+
public TestResponse(int statusCode, String body) {
41+
this.statusCode = statusCode;
42+
this.body = body;
43+
}
44+
}
45+
3646
/**
37-
* Post {@code jsonString} to {@code strURL} and return a two-element array:
38-
* {@code [statusCode, responseBody]}. Unlike {@link #post}, this method
39-
* does NOT throw on non-2xx status codes — callers that test error paths
40-
* need the response body even when HTTP 500 is returned.
47+
* Post {@code jsonString} to {@code strURL} and return the full response.
48+
* Unlike {@link #post}, this method does NOT throw on non-2xx status codes —
49+
* callers that test error paths need the response body even on HTTP 500.
4150
*/
42-
public static Object[] postForResponse(String jsonString, String strURL)
51+
public static TestResponse postForResponse(String jsonString, String strURL)
4352
throws IOException {
4453
HttpEntity stringEntity = new StringEntity(jsonString, ContentType.APPLICATION_JSON);
4554
HttpPost httpPost = new HttpPost(strURL);
4655
httpPost.setEntity(stringEntity);
47-
CloseableHttpClient httpclient = HttpClients.createDefault();
48-
try {
49-
CloseableHttpResponse response = httpclient.execute(httpPost);
56+
try (CloseableHttpClient httpclient = HttpClients.createDefault();
57+
CloseableHttpResponse response = httpclient.execute(httpPost)) {
5058
int status = response.getCode();
5159
HttpEntity entity = response.getEntity();
5260
String body = entity != null ? EntityUtils.toString(entity, "UTF-8") : "";
53-
return new Object[]{status, body};
54-
} finally {
55-
httpclient.close();
61+
return new TestResponse(status, body);
5662
}
5763
}
5864

‎modules/json/test/org/apache/axis2/json/gson/rpc/JSONRPCIntegrationTest.java‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@ public void testJsonInOnlyRPCMessageReceiver() throws Exception {
6363
@Test
6464
public void testMalformedJsonBodyReturnsBadRequest() throws Exception {
6565
String echoPersonUrl = server.getEndpoint("JSONPOJOService") + "echoPerson";
66-
Object[] result = UtilTest.postForResponse("NOT_VALID_JSON", echoPersonUrl);
67-
String body = (String) result[1];
66+
UtilTest.TestResponse result = UtilTest.postForResponse("NOT_VALID_JSON", echoPersonUrl);
67+
String body = result.body;
6868
Assert.assertTrue("Response must contain 'Bad Request' for malformed JSON body",
6969
body.contains("Bad Request"));
7070
}
@@ -77,8 +77,8 @@ public void testMalformedJsonBodyReturnsBadRequest() throws Exception {
7777
@Test
7878
public void testMalformedJsonBodyIncludesCorrelationId() throws Exception {
7979
String echoPersonUrl = server.getEndpoint("JSONPOJOService") + "echoPerson";
80-
Object[] result = UtilTest.postForResponse("NOT_VALID_JSON", echoPersonUrl);
81-
String body = (String) result[1];
80+
UtilTest.TestResponse result = UtilTest.postForResponse("NOT_VALID_JSON", echoPersonUrl);
81+
String body = result.body;
8282
Assert.assertTrue("Response must contain 'errorRef=' correlation ID",
8383
body.contains("errorRef="));
8484
}
@@ -91,8 +91,8 @@ public void testMalformedJsonBodyIncludesCorrelationId() throws Exception {
9191
@Test
9292
public void testMalformedJsonBodyCorrelationIdIsUuid() throws Exception {
9393
String echoPersonUrl = server.getEndpoint("JSONPOJOService") + "echoPerson";
94-
Object[] result = UtilTest.postForResponse("NOT_VALID_JSON", echoPersonUrl);
95-
String body = (String) result[1];
94+
UtilTest.TestResponse result = UtilTest.postForResponse("NOT_VALID_JSON", echoPersonUrl);
95+
String body = result.body;
9696
Assert.assertTrue("errorRef in fault must be a UUID",
9797
UUID_PATTERN.matcher(body).find());
9898
}
@@ -107,8 +107,8 @@ public void testMissingOuterArrayReturnsBadRequestWithCorrelationId() throws Exc
107107
// Valid JSON but wrong envelope: missing the [{...}] wrapper
108108
String badEnvelope = "{\"echoPerson\":{\"name\":\"Simon\"}}";
109109
String echoPersonUrl = server.getEndpoint("JSONPOJOService") + "echoPerson";
110-
Object[] result = UtilTest.postForResponse(badEnvelope, echoPersonUrl);
111-
String body = (String) result[1];
110+
UtilTest.TestResponse result = UtilTest.postForResponse(badEnvelope, echoPersonUrl);
111+
String body = result.body;
112112
Assert.assertTrue("Wrong-envelope request must return 'Bad Request'",
113113
body.contains("Bad Request"));
114114
Assert.assertTrue("Wrong-envelope response must contain an errorRef",
@@ -123,8 +123,8 @@ public void testMissingOuterArrayReturnsBadRequestWithCorrelationId() throws Exc
123123
@Test
124124
public void testMalformedJsonBodyDoesNotLeakExceptionClassName() throws Exception {
125125
String echoPersonUrl = server.getEndpoint("JSONPOJOService") + "echoPerson";
126-
Object[] result = UtilTest.postForResponse("NOT_VALID_JSON", echoPersonUrl);
127-
String body = (String) result[1];
126+
UtilTest.TestResponse result = UtilTest.postForResponse("NOT_VALID_JSON", echoPersonUrl);
127+
String body = result.body;
128128
Assert.assertFalse("Response must not leak 'MalformedJsonException'",
129129
body.contains("MalformedJsonException"));
130130
Assert.assertFalse("Response must not leak 'IOException'",
@@ -140,8 +140,8 @@ public void testMalformedJsonBodyDoesNotLeakExceptionClassName() throws Exceptio
140140
@Test
141141
public void testInOnlyMalformedJsonBodyReturnsBadRequestWithCorrelationId() throws Exception {
142142
String pingUrl = server.getEndpoint("JSONPOJOService") + "ping";
143-
Object[] result = UtilTest.postForResponse("NOT_VALID_JSON", pingUrl);
144-
String body = (String) result[1];
143+
UtilTest.TestResponse result = UtilTest.postForResponse("NOT_VALID_JSON", pingUrl);
144+
String body = result.body;
145145
Assert.assertTrue("InOnly malformed request must return 'Bad Request'",
146146
body.contains("Bad Request"));
147147
Assert.assertTrue("InOnly malformed request must contain an errorRef",

0 commit comments

Comments
 (0)