Skip to content

Commit

Permalink
Merge pull request #291 from cyb3r4nt/jsonrpc4j-issue-290
Browse files Browse the repository at this point in the history 
#290: Fix JSON-RPC id output in case of method type conversion errors.
  • Loading branch information
@briandilley
briandilley authored May 24, 2023
2 parents 28e6910 + 1c1b899 commit 71949a2
Show file tree
Hide file tree
Showing 3 changed files with 211 additions and 49 deletions.
70 changes: 58 additions & 12 deletions src/main/java/com/googlecode/jsonrpc4j/JsonRpcBasicServer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -464,13 +464,26 @@ 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);
}
}
}

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);

Expand Down Expand Up @@ -596,10 +609,7 @@ private Object invokePrimitiveVarargs(Object target, Method method, List<JsonNod
Object convertedParams = Array.newInstance(componentType, params.size());

for (int i = 0; i < params.size(); i++) {
JsonNode jsonNode = params.get(i);
Class<?> 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);
}

Expand All @@ -610,16 +620,32 @@ private Object invokeNonPrimitiveVarargs(Object target, Method method, List<Json
Object[] convertedParams = (Object[]) Array.newInstance(componentType, params.size());

for (int i = 0; i < params.size(); i++) {
JsonNode jsonNode = params.get(i);
Class<?> 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<JsonNode> 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;
}
Expand All @@ -631,10 +657,21 @@ private Object[] convertJsonToParameters(Method m, List<JsonNode> 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;
}
Expand Down Expand Up @@ -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<JsonRpcInterceptor> getInterceptorList() {
return interceptorList;
}
Expand Down
64 changes: 50 additions & 14 deletions src/test/java/com/googlecode/jsonrpc4j/server/JsonRpcServerAnnotatedParamTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand All @@ -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
);
}
}
126 changes: 103 additions & 23 deletions src/test/java/com/googlecode/jsonrpc4j/server/JsonRpcServerBatchTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
Expand All @@ -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()));
Expand All @@ -65,17 +62,100 @@ 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()));
assertTrue(answer instanceof ArrayNode);
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<Object> 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;
}
}

0 comments on commit 71949a2

Please sign in to comment.