Skip to content

Commit

Permalink
briandilley#290: Fix JSON-RPC id output in case of method type conver…
Browse files Browse the repository at this point in the history 
…sion 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.
  • Loading branch information
cyb3r4nt committed Jun 10, 2022
1 parent 3706d07 commit 1c1b899
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 1c1b899

Please sign in to comment.