From 6ec2477ac7d694be9f95f76be4c8a651a018578f Mon Sep 17 00:00:00 2001 From: cyb3r4nt <104218001+cyb3r4nt@users.noreply.github.com> Date: Thu, 19 Jan 2023 16:28:20 +0200 Subject: [PATCH 1/4] #306: refactor JsonRpcServer.handle() and remove unused gzip encoding parts Signed-off-by: cyb3r4nt <104218001+cyb3r4nt@users.noreply.github.com> --- .../googlecode/jsonrpc4j/JsonRpcServer.java | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/googlecode/jsonrpc4j/JsonRpcServer.java b/src/main/java/com/googlecode/jsonrpc4j/JsonRpcServer.java index 1a45d20..16fdd49 100644 --- a/src/main/java/com/googlecode/jsonrpc4j/JsonRpcServer.java +++ b/src/main/java/com/googlecode/jsonrpc4j/JsonRpcServer.java @@ -117,28 +117,33 @@ public void handle(HttpServletRequest request, HttpServletResponse response) thr OutputStream output = response.getOutputStream(); InputStream input = getRequestStream(request); int result = ErrorResolver.JsonError.PARSE_ERROR.code; - int contentLength = 0; + ByteArrayOutputStream byteOutput = new ByteArrayOutputStream(); try { - String acceptEncoding = request.getHeader(ACCEPT_ENCODING); - result = handleRequest0(input, output, acceptEncoding, response, byteOutput); - - contentLength = byteOutput.size(); + result = handleRequest(input, byteOutput); } catch (Throwable t) { - if (StreamEndedException.class.isInstance(t)) { + if (t instanceof StreamEndedException) { logger.debug("Bad request: empty contents!"); } else { logger.error(t.getMessage(), t); } } - int httpStatusCode = httpStatusCodeProvider == null ? DefaultHttpStatusCodeProvider.INSTANCE.getHttpStatusCode(result) - : httpStatusCodeProvider.getHttpStatusCode(result); - response.setStatus(httpStatusCode); - response.setContentLength(contentLength); + + response.setStatus(resolveHttpStatusCode(result)); + response.setContentLength(byteOutput.size()); byteOutput.writeTo(output); output.flush(); } + private int resolveHttpStatusCode(int result) { + int httpStatusCode; + if (this.httpStatusCodeProvider != null) { + return this.httpStatusCodeProvider.getHttpStatusCode(result); + } else { + return DefaultHttpStatusCodeProvider.INSTANCE.getHttpStatusCode(result); + } + } + private InputStream getRequestStream(HttpServletRequest request) throws IOException { InputStream input; if (request.getMethod().equals("POST")) { @@ -151,10 +156,6 @@ private InputStream getRequestStream(HttpServletRequest request) throws IOExcept return input; } - private int handleRequest0(InputStream input, OutputStream output, String contentEncoding, HttpServletResponse response, ByteArrayOutputStream byteOutput) throws IOException { - return handleRequest(input, byteOutput); - } - private static InputStream createInputStream(HttpServletRequest request) throws IOException { String method = request.getParameter(METHOD); String id = request.getParameter(ID); From 08fc46281c4c5df292212d7278b301bc2933d800 Mon Sep 17 00:00:00 2001 From: cyb3r4nt <104218001+cyb3r4nt@users.noreply.github.com> Date: Thu, 19 Jan 2023 17:42:33 +0200 Subject: [PATCH 2/4] #306: extract common HTTP request and response interfaces inside JsonRpcServer Common logic may be used to handle the HTTP requests. This is a preparation for using jakarta.servlet.http namespace objects. There are no much requirements for those interfaces, only some common methods, which are present in most Portlet API requests were not changed, because there is a comment, which assumes that HTTP status code is assigned outside. Added a TODO comment there with possible solution proposal. Signed-off-by: cyb3r4nt <104218001+cyb3r4nt@users.noreply.github.com> --- .../googlecode/jsonrpc4j/JsonRpcServer.java | 87 +++++++++++++++++-- 1 file changed, 82 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/googlecode/jsonrpc4j/JsonRpcServer.java b/src/main/java/com/googlecode/jsonrpc4j/JsonRpcServer.java index 16fdd49..4e03106 100644 --- a/src/main/java/com/googlecode/jsonrpc4j/JsonRpcServer.java +++ b/src/main/java/com/googlecode/jsonrpc4j/JsonRpcServer.java @@ -87,6 +87,8 @@ public void handle(ResourceRequest request, ResourceResponse response) throws IO OutputStream output = response.getPortletOutputStream(); handleRequest(input, output); // fix to not flush within handleRequest() but outside so http status code can be set + // TODO: this logic may be changed to use handleCommon() method, + // HTTP status code may be provided by the HttpStatusCodeProvider extension output.flush(); } @@ -112,7 +114,11 @@ private static InputStream createInputStream(ResourceRequest request) throws IOE * @throws IOException on error */ public void handle(HttpServletRequest request, HttpServletResponse response) throws IOException { - logger.debug("Handling HttpServletRequest {}", request); + handleCommon(new JavaxHttpServletRequest(request), new JavaxHttpServletResponse(response)); + } + + private void handleCommon(CommonHttpServletRequest request, CommonHttpServletResponse response) throws IOException { + logger.debug("Handling HttpServletRequest {}", request.unwrap()); response.setContentType(contentType); OutputStream output = response.getOutputStream(); InputStream input = getRequestStream(request); @@ -144,11 +150,11 @@ private int resolveHttpStatusCode(int result) { } } - private InputStream getRequestStream(HttpServletRequest request) throws IOException { + private InputStream getRequestStream(CommonHttpServletRequest request) throws IOException { InputStream input; - if (request.getMethod().equals("POST")) { + if ("POST".equals(request.getMethod())) { input = request.getInputStream(); - } else if (request.getMethod().equals("GET")) { + } else if ("GET".equals(request.getMethod())) { input = createInputStream(request); } else { throw new IOException("Invalid request method, only POST and GET is supported"); @@ -156,7 +162,7 @@ private InputStream getRequestStream(HttpServletRequest request) throws IOExcept return input; } - private static InputStream createInputStream(HttpServletRequest request) throws IOException { + private static InputStream createInputStream(CommonHttpServletRequest request) throws IOException { String method = request.getParameter(METHOD); String id = request.getParameter(ID); String params = request.getParameter(PARAMS); @@ -171,4 +177,75 @@ public void setContentType(String contentType) { this.contentType = contentType; } + private interface CommonHttpServletRequest { + Object unwrap(); + InputStream getInputStream() throws IOException; + String getMethod(); + String getParameter(String name); + } + + private static class JavaxHttpServletRequest implements CommonHttpServletRequest { + + private final HttpServletRequest request; + + private JavaxHttpServletRequest(HttpServletRequest request) { + this.request = request; + } + + @Override + public HttpServletRequest unwrap() { + return this.request; + } + + @Override + public InputStream getInputStream() throws IOException { + return this.request.getInputStream(); + } + + @Override + public String getMethod() { + return this.request.getMethod(); + } + + @Override + public String getParameter(String name) { + return this.request.getParameter(name); + } + } + + private interface CommonHttpServletResponse { + void setContentType(String type); + void setStatus(int sc); + void setContentLength(int len); + OutputStream getOutputStream() throws IOException; + } + + private static class JavaxHttpServletResponse implements CommonHttpServletResponse { + + private final HttpServletResponse response; + + private JavaxHttpServletResponse(HttpServletResponse response) { + this.response = response; + } + + @Override + public void setContentType(String type) { + this.response.setContentType(type); + } + + @Override + public void setStatus(int sc) { + this.response.setStatus(sc); + } + + @Override + public void setContentLength(int len) { + this.response.setContentLength(len); + } + + @Override + public OutputStream getOutputStream() throws IOException { + return this.response.getOutputStream(); + } + } } From 59107e8ab1d281f86262361cc02aaa90192e6f50 Mon Sep 17 00:00:00 2001 From: cyb3r4nt <104218001+cyb3r4nt@users.noreply.github.com> Date: Thu, 19 Jan 2023 18:31:48 +0200 Subject: [PATCH 3/4] #306: add support for jakarta.servlet-api request objects into JsonRpcServer Both javax and jakarta objects/APIs have similar method signatures and differ only with types. Two new classes were added to adapt to the common interface. Not very last version of jakarta.servlet-api v5.0.0 was added, because project JDK target is Java 8. jakarta.servlet-api v5.0.0 comes from the Jakarta EE 9, which is still compatible with JDK 8. Signed-off-by: cyb3r4nt <104218001+cyb3r4nt@users.noreply.github.com> --- build.gradle | 3 + .../googlecode/jsonrpc4j/JsonRpcServer.java | 82 ++++++++++++++++++- 2 files changed, 83 insertions(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index 186e388..c9eed1c 100644 --- a/build.gradle +++ b/build.gradle @@ -96,6 +96,9 @@ dependencies { servletSupportImplementation 'javax.portlet:portlet-api:3.0.1' servletSupportImplementation 'javax.servlet:javax.servlet-api:4.0.1' + // TODO: Jakarta EE 9 and jakarta.servlet-api 5.x are still compatible with Java SE 8, + // update jakarta.servlet-api to version 6+ when JDK baseline is increased to 11+ + servletSupportImplementation 'jakarta.servlet:jakarta.servlet-api:5.0.0' implementation "com.fasterxml.jackson.core:jackson-core:${jacksonVersion}" implementation "com.fasterxml.jackson.core:jackson-databind:${jacksonVersion}" diff --git a/src/main/java/com/googlecode/jsonrpc4j/JsonRpcServer.java b/src/main/java/com/googlecode/jsonrpc4j/JsonRpcServer.java index 4e03106..d58ba39 100644 --- a/src/main/java/com/googlecode/jsonrpc4j/JsonRpcServer.java +++ b/src/main/java/com/googlecode/jsonrpc4j/JsonRpcServer.java @@ -114,7 +114,27 @@ private static InputStream createInputStream(ResourceRequest request) throws IOE * @throws IOException on error */ public void handle(HttpServletRequest request, HttpServletResponse response) throws IOException { - handleCommon(new JavaxHttpServletRequest(request), new JavaxHttpServletResponse(response)); + handleCommon( + new JavaxHttpServletRequest(request), + new JavaxHttpServletResponse(response) + ); + } + + /** + * Handles a servlet request. + * + * @param request the {@link jakarta.servlet.http.HttpServletRequest} + * @param response the {@link jakarta.servlet.http.HttpServletResponse} + * @throws IOException on error + */ + public void handle( + jakarta.servlet.http.HttpServletRequest request, + jakarta.servlet.http.HttpServletResponse response + ) throws IOException { + handleCommon( + new JakartaHttpServletRequest(request), + new JakartaHttpServletResponse(response) + ); } private void handleCommon(CommonHttpServletRequest request, CommonHttpServletResponse response) throws IOException { @@ -193,7 +213,36 @@ private JavaxHttpServletRequest(HttpServletRequest request) { } @Override - public HttpServletRequest unwrap() { + public Object unwrap() { + return this.request; + } + + @Override + public InputStream getInputStream() throws IOException { + return this.request.getInputStream(); + } + + @Override + public String getMethod() { + return this.request.getMethod(); + } + + @Override + public String getParameter(String name) { + return this.request.getParameter(name); + } + } + + private static class JakartaHttpServletRequest implements CommonHttpServletRequest { + + private final jakarta.servlet.http.HttpServletRequest request; + + private JakartaHttpServletRequest(jakarta.servlet.http.HttpServletRequest request) { + this.request = request; + } + + @Override + public Object unwrap() { return this.request; } @@ -248,4 +297,33 @@ public OutputStream getOutputStream() throws IOException { return this.response.getOutputStream(); } } + + private static class JakartaHttpServletResponse implements CommonHttpServletResponse { + + private final jakarta.servlet.http.HttpServletResponse response; + + private JakartaHttpServletResponse(jakarta.servlet.http.HttpServletResponse response) { + this.response = response; + } + + @Override + public void setContentType(String type) { + this.response.setContentType(type); + } + + @Override + public void setStatus(int sc) { + this.response.setStatus(sc); + } + + @Override + public void setContentLength(int len) { + this.response.setContentLength(len); + } + + @Override + public OutputStream getOutputStream() throws IOException { + return this.response.getOutputStream(); + } + } } From 8e0befff78e9b90f46ebfa0af253309a00235edc Mon Sep 17 00:00:00 2001 From: cyb3r4nt <104218001+cyb3r4nt@users.noreply.github.com> Date: Thu, 19 Jan 2023 21:48:57 +0200 Subject: [PATCH 4/4] #306: remove unused imports and internal variables from JsonRpcServer Removed @SuppressWarnings from top-level place on top of the class. Currently only two handle() methods are shown as unused, but these are public API methods, which meant to be called by the clients. Signed-off-by: cyb3r4nt <104218001+cyb3r4nt@users.noreply.github.com> --- src/main/java/com/googlecode/jsonrpc4j/JsonRpcServer.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/main/java/com/googlecode/jsonrpc4j/JsonRpcServer.java b/src/main/java/com/googlecode/jsonrpc4j/JsonRpcServer.java index d58ba39..98c4968 100644 --- a/src/main/java/com/googlecode/jsonrpc4j/JsonRpcServer.java +++ b/src/main/java/com/googlecode/jsonrpc4j/JsonRpcServer.java @@ -13,18 +13,14 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.util.zip.GZIPInputStream; -import java.util.zip.GZIPOutputStream; /** * A JSON-RPC request server reads JSON-RPC requests from an input stream and writes responses to an output stream. * Supports handler and servlet requests. */ -@SuppressWarnings("unused") public class JsonRpcServer extends JsonRpcBasicServer { private static final Logger logger = LoggerFactory.getLogger(JsonRpcServer.class); - private static final String GZIP = "gzip"; private String contentType = JSONRPC_CONTENT_TYPE; /** @@ -162,7 +158,6 @@ private void handleCommon(CommonHttpServletRequest request, CommonHttpServletRes } private int resolveHttpStatusCode(int result) { - int httpStatusCode; if (this.httpStatusCodeProvider != null) { return this.httpStatusCodeProvider.getHttpStatusCode(result); } else {