Add custom HttpStatusCodeProvider to adjust to JSON-RPC 2.0 conventions

rskj-core/src/main/java/co/rsk/rpc/netty/Web3HttpServer.java

@@ -81,7 +81,7 @@ protected void initChannel(SocketChannel ch) throws Exception {
                     p.addLast(jsonRpcWeb3FilterHandler);
                     p.addLast(new Web3HttpMethodFilterHandler());
                     p.addLast(jsonRpcWeb3ServerHandler);
-                    p.addLast(new Web3ResultHttpResponseHandler());
+                    p.addLast(buildWeb3ResultHttpResponseHandler());
                 }
             });
         try {
@@ -92,6 +92,11 @@ protected void initChannel(SocketChannel ch) throws Exception {
         }
     }
 
+    private Web3ResultHttpResponseHandler buildWeb3ResultHttpResponseHandler() {
+        Web3HttpStatusCodeProvider web3HttpStatusCodeProvider = new Web3HttpStatusCodeProvider();
+        return new Web3ResultHttpResponseHandler(web3HttpStatusCodeProvider);
+    }
+
     @Override
     public void stop() {
         bossGroup.shutdownGracefully();

rskj-core/src/main/java/co/rsk/rpc/netty/Web3HttpStatusCodeProvider.java

@@ -0,0 +1,43 @@
+package co.rsk.rpc.netty;
+
+import com.googlecode.jsonrpc4j.HttpStatusCodeProvider;
+import io.netty.handler.codec.http.HttpResponseStatus;
+
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static com.googlecode.jsonrpc4j.ErrorResolver.JsonError.*;
+
+public class Web3HttpStatusCodeProvider implements HttpStatusCodeProvider {
+
+    private static final List<Integer> BAD_REQUEST_JSON_ERRORS = Stream.of(METHOD_PARAMS_INVALID, INVALID_REQUEST, PARSE_ERROR)
+            .map(jsonError -> jsonError.code)
+            .collect(Collectors.toList());
+
+    /**
+     * Only invalid JSON-RPC requests (e.g. a malformed JSON) qualify for a Bad Request HTTP status.
+     * In any other case, the HTTP protocol remains entirely independent from JSON-RPC (since its 2.0 version).
+     * Reference: https://www.jsonrpc.org/
+     */
+    @Override
+    public int getHttpStatusCode(int resultCode) {
+        return isBadRequestResultCode(resultCode) ?
+                HttpResponseStatus.BAD_REQUEST.code() :
+                HttpResponseStatus.OK.code();
+    }
+
+    private static boolean isBadRequestResultCode(int resultCode) {
+        return BAD_REQUEST_JSON_ERRORS.contains(resultCode);
+    }
+
+    /**
+     * The default implementation wrongly assumes that an HTTP status code is mapped to a single JSON RPC error - this
+     * is not always the case. Either way, this is not used in the current project.
+     */
+    @Override
+    public Integer getJsonRpcCode(int httpStatusCode) {
+        throw new UnsupportedOperationException("Cannot possibly determine a single JSON RPC from an HTTP status code");
+    }
+
+}

rskj-core/src/main/java/co/rsk/rpc/netty/Web3ResultHttpResponseHandler.java

@@ -17,23 +17,30 @@
  */
 package co.rsk.rpc.netty;
 
-import com.googlecode.jsonrpc4j.DefaultHttpStatusCodeProvider;
+import com.googlecode.jsonrpc4j.HttpStatusCodeProvider;
 import io.netty.channel.ChannelFutureListener;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.SimpleChannelInboundHandler;
 import io.netty.handler.codec.http.DefaultFullHttpResponse;
 import io.netty.handler.codec.http.HttpResponseStatus;
 import io.netty.handler.codec.http.HttpVersion;
+
 import static io.netty.handler.codec.http.HttpHeaders.Names.CONTENT_TYPE;
 import static io.netty.handler.codec.http.HttpHeaders.Values.APPLICATION_JSON;
 
 public class Web3ResultHttpResponseHandler extends SimpleChannelInboundHandler<Web3Result> {
 
+    private final HttpStatusCodeProvider httpStatusCodeProvider;
+
+    public Web3ResultHttpResponseHandler(HttpStatusCodeProvider httpStatusCodeProvider) {
+        this.httpStatusCodeProvider = httpStatusCodeProvider;
+    }
+
     @Override
     protected void channelRead0(ChannelHandlerContext ctx, Web3Result msg) {
         DefaultFullHttpResponse response = new DefaultFullHttpResponse(
                 HttpVersion.HTTP_1_1,
-                HttpResponseStatus.valueOf(DefaultHttpStatusCodeProvider.INSTANCE.getHttpStatusCode(msg.getCode())),
+                HttpResponseStatus.valueOf(httpStatusCodeProvider.getHttpStatusCode(msg.getCode())),
                 msg.getContent()
         );
 

rskj-core/src/test/java/co/rsk/rpc/netty/Web3HttpStatusCodeProviderTest.java

@@ -0,0 +1,50 @@
+package co.rsk.rpc.netty;
+
+import com.googlecode.jsonrpc4j.HttpStatusCodeProvider;
+import io.netty.handler.codec.http.HttpResponseStatus;
+import org.junit.Before;
+import org.junit.Test;
+
+import static com.googlecode.jsonrpc4j.ErrorResolver.JsonError.*;
+import static org.junit.Assert.assertEquals;
+
+public class Web3HttpStatusCodeProviderTest {
+
+    private HttpStatusCodeProvider httpStatusCodeProvider;
+
+    @Before
+    public void setup() throws Exception {
+        httpStatusCodeProvider = new Web3HttpStatusCodeProvider();
+    }
+
+    @Test
+    public void getHttpStatusCodeReturnsExpectedHttpStatusCodes() {
+        assertThatOkHttpStatusIsReturnedWhenExpected(httpStatusCodeProvider);
+        assertThatBadRequestHttpStatusIsReturnedWhenExpected(httpStatusCodeProvider);
+    }
+
+    private void assertThatBadRequestHttpStatusIsReturnedWhenExpected(HttpStatusCodeProvider httpStatusCodeProvider) {
+        assertReturnedHttpStatusCodeIsExpected(httpStatusCodeProvider, OK.code, HttpResponseStatus.OK.code());
+        assertReturnedHttpStatusCodeIsExpected(httpStatusCodeProvider, METHOD_NOT_FOUND.code, HttpResponseStatus.OK.code());
+        assertReturnedHttpStatusCodeIsExpected(httpStatusCodeProvider, INTERNAL_ERROR.code, HttpResponseStatus.OK.code());
+        assertReturnedHttpStatusCodeIsExpected(httpStatusCodeProvider, ERROR_NOT_HANDLED.code, HttpResponseStatus.OK.code());
+        assertReturnedHttpStatusCodeIsExpected(httpStatusCodeProvider, BULK_ERROR.code, HttpResponseStatus.OK.code());
+    }
+
+    private void assertThatOkHttpStatusIsReturnedWhenExpected(HttpStatusCodeProvider httpStatusCodeProvider) {
+        assertReturnedHttpStatusCodeIsExpected(httpStatusCodeProvider, METHOD_PARAMS_INVALID.code, HttpResponseStatus.BAD_REQUEST.code());
+        assertReturnedHttpStatusCodeIsExpected(httpStatusCodeProvider, INVALID_REQUEST.code, HttpResponseStatus.BAD_REQUEST.code());
+        assertReturnedHttpStatusCodeIsExpected(httpStatusCodeProvider, PARSE_ERROR.code, HttpResponseStatus.BAD_REQUEST.code());
+    }
+
+    private void assertReturnedHttpStatusCodeIsExpected(HttpStatusCodeProvider httpStatusCodeProvider,
+                                                        int jsonRpcResultCode, int expectedHttpStatusCode) {
+        int httpStatusCode = httpStatusCodeProvider.getHttpStatusCode(jsonRpcResultCode);
+        assertEquals(expectedHttpStatusCode, httpStatusCode);
+    }
+
+    @Test(expected = UnsupportedOperationException.class)
+    public void getJsonRpcCodeThrowsUnsupportedOperationException() {
+        httpStatusCodeProvider.getJsonRpcCode(INVALID_REQUEST.code);
+    }
+}