From 1c1b8991e2fcc208bd1c0b84d248d749f3f408fd Mon Sep 17 00:00:00 2001 From: cyb3r4nt Date: Sat, 11 Jun 2022 02:28:30 +0300 Subject: [PATCH] #290: Fix JSON-RPC id output in case of method type conversion errors. Server used to output null as id value if some parameter conversion has been failed. Request id context was lost due to exceptions and null was sent to client. Fix contains more grained exception handling and restore of initial request context. Added also tests, which check for expected id result in the responses. --- .../jsonrpc4j/JsonRpcBasicServer.java | 70 ++++++++-- .../JsonRpcServerAnnotatedParamTest.java | 64 +++++++-- .../server/JsonRpcServerBatchTest.java | 126 ++++++++++++++---- 3 files changed, 211 insertions(+), 49 deletions(-) diff --git a/src/main/java/com/googlecode/jsonrpc4j/JsonRpcBasicServer.java b/src/main/java/com/googlecode/jsonrpc4j/JsonRpcBasicServer.java index aa1db675..837a10f8 100644 --- a/src/main/java/com/googlecode/jsonrpc4j/JsonRpcBasicServer.java +++ b/src/main/java/com/googlecode/jsonrpc4j/JsonRpcBasicServer.java @@ -464,6 +464,9 @@ private JsonResponse handleObject(final ObjectNode node) return new JsonResponse(null, JsonError.OK.code); } catch (JsonParseException | JsonMappingException e) { throw e; // rethrow this, it will be handled as PARSE_ERROR later + } catch (ParameterConvertException pce) { + handler.error = pce.getCause(); + return handleParameterConvertError(pce, id, jsonRpc); } catch (Throwable e) { handler.error = e; return handleError(id, jsonRpc, methodArgs, e); @@ -471,6 +474,16 @@ private JsonResponse handleObject(final ObjectNode node) } } + private JsonResponse handleParameterConvertError(ParameterConvertException pce, Object id, String jsonRpc) { + String errorMsg = "Failed to read method parameter at index " + pce.paramIndex; + JsonError jsonError = new JsonError( + JsonError.METHOD_PARAMS_INVALID.code, + errorMsg, + null + ); + return createResponseError(jsonRpc, id, jsonError); + } + private JsonResponse handleError(Object id, String jsonRpc, AMethodWithItsArgs methodArgs, Throwable e) { Throwable unwrappedException = getException(e); @@ -596,10 +609,7 @@ private Object invokePrimitiveVarargs(Object target, Method method, List type = JsonUtil.getJavaTypeForJsonType(jsonNode); - Object object = mapper.convertValue(jsonNode, type); - logger.debug("[{}] param: {} -> {}", method.getName(), i, type.getName()); + Object object = convertAndLogParam(method, params, i); Array.set(convertedParams, i, object); } @@ -610,16 +620,32 @@ private Object invokeNonPrimitiveVarargs(Object target, Method method, List type = JsonUtil.getJavaTypeForJsonType(jsonNode); - Object object = mapper.convertValue(jsonNode, type); - logger.debug("[{}] param: {} -> {}", method.getName(), i, type.getName()); + Object object = convertAndLogParam(method, params, i); convertedParams[i] = object; } return method.invoke(target, new Object[] { convertedParams }); } + private Object convertAndLogParam(Method method, List params, int paramIndex) { + JsonNode jsonNode = params.get(paramIndex); + Class type = JsonUtil.getJavaTypeForJsonType(jsonNode); + Object object; + try { + object = mapper.convertValue(jsonNode, type); + } catch (IllegalArgumentException e) { + logger.debug( + "[{}] Failed to convert param: {} -> {}", + method.getName(), + paramIndex, + type.getName() + ); + throw new ParameterConvertException(paramIndex, e); + } + logger.debug("[{}] param: {} -> {}", method.getName(), paramIndex, type.getName()); + return object; + } + private boolean hasReturnValue(Method m) { return m.getGenericReturnType() != null; } @@ -631,10 +657,21 @@ private Object[] convertJsonToParameters(Method m, List params) throws for (int i = 0; i < parameterTypes.length; i++) { JsonParser paramJsonParser = mapper.treeAsTokens(params.get(i)); JavaType paramJavaType = mapper.getTypeFactory().constructType(parameterTypes[i]); - - convertedParams[i] = mapper.readerFor(paramJavaType) - .with(DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY) - .readValue(paramJsonParser); + ObjectReader reader = + mapper + .readerFor(paramJavaType) + .with(DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY); + try { + convertedParams[i] = reader.readValue(paramJsonParser); + } catch (JsonParseException | JsonMappingException e) { + logger.debug( + "[{}] Failed to convert param: {} -> {}", + m.getName(), + i, + parameterTypes[i].getTypeName() + ); + throw new ParameterConvertException(i, e); + } } return convertedParams; } @@ -1329,6 +1366,15 @@ public int getNameCount() { } + private static class ParameterConvertException extends RuntimeException { + private final int paramIndex; + + private ParameterConvertException(int index, Throwable throwable) { + super(throwable); + this.paramIndex = index; + } + } + public List getInterceptorList() { return interceptorList; } diff --git a/src/test/java/com/googlecode/jsonrpc4j/server/JsonRpcServerAnnotatedParamTest.java b/src/test/java/com/googlecode/jsonrpc4j/server/JsonRpcServerAnnotatedParamTest.java index 6aad452f..66026ac1 100644 --- a/src/test/java/com/googlecode/jsonrpc4j/server/JsonRpcServerAnnotatedParamTest.java +++ b/src/test/java/com/googlecode/jsonrpc4j/server/JsonRpcServerAnnotatedParamTest.java @@ -14,26 +14,20 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.util.UUID; import static com.googlecode.jsonrpc4j.ErrorResolver.JsonError.METHOD_PARAMS_INVALID; import static com.googlecode.jsonrpc4j.ErrorResolver.JsonError.PARSE_ERROR; +import static com.googlecode.jsonrpc4j.JsonRpcBasicServer.ID; import static com.googlecode.jsonrpc4j.JsonRpcBasicServer.RESULT; -import static com.googlecode.jsonrpc4j.util.Util.decodeAnswer; -import static com.googlecode.jsonrpc4j.util.Util.error; -import static com.googlecode.jsonrpc4j.util.Util.errorCode; -import static com.googlecode.jsonrpc4j.util.Util.intParam1; -import static com.googlecode.jsonrpc4j.util.Util.intParam2; -import static com.googlecode.jsonrpc4j.util.Util.mapper; -import static com.googlecode.jsonrpc4j.util.Util.messageWithMapParamsStream; -import static com.googlecode.jsonrpc4j.util.Util.param1; -import static com.googlecode.jsonrpc4j.util.Util.param2; -import static com.googlecode.jsonrpc4j.util.Util.param3; -import static com.googlecode.jsonrpc4j.util.Util.param4; -import static org.junit.Assert.assertEquals; +import static com.googlecode.jsonrpc4j.util.Util.*; +import static org.junit.Assert.*; @RunWith(EasyMockRunner.class) public class JsonRpcServerAnnotatedParamTest { - + + static final String METHOD_WITH_DIFFERENT_TYPES = "methodWithDifferentTypes"; + @Mock(type = MockType.NICE) private ServiceInterfaceWithParamNameAnnotation mockService; private ByteArrayOutputStream byteArrayOutputStream; @@ -180,7 +174,43 @@ public void callParseErrorJson() throws Exception { jsonRpcServerAnnotatedParam.handleRequest(Util.invalidJsonStream(), byteArrayOutputStream); assertEquals(PARSE_ERROR.code, errorCode(error(byteArrayOutputStream)).asInt()); } - + + @Test + public void callMethodWithIncompatibleParamTypeAndExpectInvalidParamsError() throws Exception { + final Object invalidDouble = "callMeDouble"; + jsonRpcServerAnnotatedParam.handleRequest( + createStream( + messageWithListParams( + 3, + METHOD_WITH_DIFFERENT_TYPES, + false, invalidDouble, UUID.randomUUID() + ) + ), + byteArrayOutputStream + ); + assertEquals(METHOD_PARAMS_INVALID.code, errorCode(error(byteArrayOutputStream)).asInt()); + } + + @Test + public void callMethodWithIncompatibleParamTypeAndExpectProperJsonRpcIdResponse() throws Exception { + final Object invalidUUID = "iWantToBeAnUUID"; + final int jsonRpcId = 4; + jsonRpcServerAnnotatedParam.handleRequest( + createStream( + messageWithListParams( + jsonRpcId, + METHOD_WITH_DIFFERENT_TYPES, + true, 3.14, invalidUUID + ) + ), + byteArrayOutputStream + ); + JsonNode responseId = decodeAnswer(byteArrayOutputStream).get(ID); + assertNotNull(responseId); + assertTrue(responseId.isInt()); + assertEquals(4, responseId.asInt()); + } + public interface ServiceInterfaceWithParamNameAnnotation { String testMethod(@JsonRpcParam("param1") String param1); @@ -195,5 +225,11 @@ public interface ServiceInterfaceWithParamNameAnnotation { String overloadedMethod(@JsonRpcParam("param1") int intParam1, @JsonRpcParam("param2") int intParam2); String methodWithoutRequiredParam(@JsonRpcParam("param1") String stringParam1, @JsonRpcParam(value = "param2") String stringParam2); + + String methodWithDifferentTypes( + @JsonRpcParam("param1") Boolean booleanParam1, + @JsonRpcParam("param2") Double doubleParam2, + @JsonRpcParam("param3") UUID doubleParam3 + ); } } diff --git a/src/test/java/com/googlecode/jsonrpc4j/server/JsonRpcServerBatchTest.java b/src/test/java/com/googlecode/jsonrpc4j/server/JsonRpcServerBatchTest.java index 8cc4c241..7d11b859 100644 --- a/src/test/java/com/googlecode/jsonrpc4j/server/JsonRpcServerBatchTest.java +++ b/src/test/java/com/googlecode/jsonrpc4j/server/JsonRpcServerBatchTest.java @@ -3,6 +3,9 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ArrayNode; import com.googlecode.jsonrpc4j.JsonRpcServer; +import com.googlecode.jsonrpc4j.server.JsonRpcServerAnnotatedParamTest.ServiceInterfaceWithParamNameAnnotation; +import com.googlecode.jsonrpc4j.util.Util; + import org.easymock.EasyMock; import org.easymock.Mock; import org.easymock.MockType; @@ -12,19 +15,18 @@ import org.springframework.util.StreamUtils; import javax.servlet.http.HttpServletResponse; +import java.io.IOException; import java.io.InputStream; +import java.util.Arrays; +import java.util.List; +import java.util.Locale; +import java.util.UUID; -import static com.googlecode.jsonrpc4j.JsonRpcBasicServer.DATA; -import static com.googlecode.jsonrpc4j.JsonRpcBasicServer.ERROR; -import static com.googlecode.jsonrpc4j.JsonRpcBasicServer.ERROR_MESSAGE; -import static com.googlecode.jsonrpc4j.JsonRpcBasicServer.RESULT; -import static com.googlecode.jsonrpc4j.util.Util.decodeAnswer; -import static com.googlecode.jsonrpc4j.util.Util.getFromArrayWithId; -import static com.googlecode.jsonrpc4j.util.Util.messageWithListParams; -import static com.googlecode.jsonrpc4j.util.Util.multiMessageOfStream; -import static com.googlecode.jsonrpc4j.util.Util.toByteArrayOutputStream; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static com.googlecode.jsonrpc4j.ErrorResolver.JsonError.METHOD_PARAMS_INVALID; +import static com.googlecode.jsonrpc4j.JsonRpcBasicServer.*; +import static com.googlecode.jsonrpc4j.server.JsonRpcServerAnnotatedParamTest.METHOD_WITH_DIFFERENT_TYPES; +import static com.googlecode.jsonrpc4j.util.Util.*; +import static org.junit.Assert.*; public abstract class JsonRpcServerBatchTest { @Mock(type = MockType.NICE) @@ -41,12 +43,7 @@ public void parallelBatchProcessingTest() throws Exception { messageWithListParams(1, "testMethod", "Parameter1"), messageWithListParams(2, "testMethod", "Parameter2")); - MockHttpServletRequest request = new MockHttpServletRequest("POST", "/test-post"); - MockHttpServletResponse response = new MockHttpServletResponse(); - - request.setContent(StreamUtils.copyToByteArray(inputStream)); - - jsonRpcServer.handle(request, response); + MockHttpServletResponse response = handleRequest(inputStream); assertEquals(HttpServletResponse.SC_OK, response.getStatus()); JsonNode answer = decodeAnswer(toByteArrayOutputStream(response.getContentAsByteArray())); @@ -65,12 +62,7 @@ public void parallelBatchProcessingBulkErrorTest() throws Exception { messageWithListParams(1, "testMethod", "Parameter1"), messageWithListParams(2, "testMethod", "Parameter2")); - MockHttpServletRequest request = new MockHttpServletRequest("POST", "/test-post"); - MockHttpServletResponse response = new MockHttpServletResponse(); - - request.setContent(StreamUtils.copyToByteArray(inputStream)); - - jsonRpcServer.handle(request, response); + MockHttpServletResponse response = handleRequest(inputStream); assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, response.getStatus()); JsonNode answer = decodeAnswer(toByteArrayOutputStream(response.getContentAsByteArray())); @@ -78,4 +70,92 @@ public void parallelBatchProcessingBulkErrorTest() throws Exception { assertEquals("Error", getFromArrayWithId(answer, 1).get(ERROR).get(DATA).get(ERROR_MESSAGE).asText()); assertEquals("Result2", getFromArrayWithId(answer, 2).get(RESULT).asText()); } + + @Test + public void parallelBatchWithInvalidRequestInsideShouldPreserveJsonRpcId() throws Exception { + ServiceInterfaceWithParamNameAnnotation mockServiceWithParams = + EasyMock.strictMock(ServiceInterfaceWithParamNameAnnotation.class); + + jsonRpcServer = new JsonRpcServer( + Util.mapper, + mockServiceWithParams, + ServiceInterfaceWithParamNameAnnotation.class + ); + + final String validResult = "passResult"; + final String validResultOne = validResult + "1"; + final String validResultThree = validResult + "3"; + + EasyMock + .expect( + mockServiceWithParams.methodWithDifferentTypes( + EasyMock.anyBoolean(), + EasyMock.anyDouble(), + EasyMock.anyObject(UUID.class) + ) + ) + .andReturn(validResultOne); + EasyMock + .expect( + mockServiceWithParams.methodWithDifferentTypes( + EasyMock.anyBoolean(), + EasyMock.anyDouble(), + EasyMock.anyObject(UUID.class) + ) + ) + .andReturn(validResultThree); + EasyMock.replay(mockServiceWithParams); + + final List validParams = Arrays.asList(true, 3.14, UUID.randomUUID()); + final String invalidBoolean = "truth"; + + InputStream inputStream = multiMessageOfStream( + messageOfStream(1, METHOD_WITH_DIFFERENT_TYPES, validParams), + messageOfStream( + 2, + METHOD_WITH_DIFFERENT_TYPES, + Arrays.asList(invalidBoolean, 3.14, UUID.randomUUID()) + ), + messageOfStream(3, METHOD_WITH_DIFFERENT_TYPES, validParams) + ); + + MockHttpServletResponse response = handleRequest(inputStream); + + EasyMock.verify(mockServiceWithParams); + + assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, response.getStatus()); + JsonNode answer = decodeAnswer(toByteArrayOutputStream(response.getContentAsByteArray())); + assertTrue(answer instanceof ArrayNode); + + assertEquals(validResultOne, getFromArrayWithId(answer, 1).get(RESULT).asText()); + assertEquals(validResultThree, getFromArrayWithId(answer, 3).get(RESULT).asText()); + + JsonNode secondResponse = getFromArrayWithId(answer, 2); + JsonNode responseId = secondResponse.get(ID); + assertNotNull(responseId); + assertTrue(responseId.isInt()); + assertEquals(2, responseId.asInt()); + + JsonNode error = secondResponse.get(ERROR); + assertNotNull(error); + + assertEquals(METHOD_PARAMS_INVALID.code, errorCode(error).asInt()); + + String errorMessage = error.get(ERROR_MESSAGE).asText(); + assertTrue(errorMessage.toLowerCase(Locale.ROOT).contains("index")); + assertTrue(errorMessage.contains("0")); + } + + private static MockHttpServletRequest createRequest(InputStream inputStream) throws IOException { + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/test-post"); + request.setContent(StreamUtils.copyToByteArray(inputStream)); + return request; + } + + private MockHttpServletResponse handleRequest(InputStream inputStream) throws IOException { + MockHttpServletRequest request = createRequest(inputStream); + MockHttpServletResponse response = new MockHttpServletResponse(); + jsonRpcServer.handle(request, response); + return response; + } }