From 605ef1f1aca08e27220f17fb9da5d2f1e7ef9378 Mon Sep 17 00:00:00 2001 From: 51-code <146736881+51-code@users.noreply.github.com> Date: Mon, 19 Aug 2024 13:16:37 +0300 Subject: [PATCH] Fix #69: Coverity defects (#70) * Fix possible NPE in HttpServerHandler and pointless variable assignment in PayloadConfig * Grammar fix in HttpServerHandler * Apply spotless * Try/catch the remote address instead of checking if the channel is active * Remove unused remoteAddress from RelpConversion --- src/main/java/com/teragrep/lsh_01/HttpServerHandler.java | 7 ++++--- src/main/java/com/teragrep/lsh_01/IMessageHandler.java | 3 +-- src/main/java/com/teragrep/lsh_01/MessageProcessor.java | 5 +---- src/main/java/com/teragrep/lsh_01/RelpConversion.java | 2 +- .../java/com/teragrep/lsh_01/config/PayloadConfig.java | 2 +- 5 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/teragrep/lsh_01/HttpServerHandler.java b/src/main/java/com/teragrep/lsh_01/HttpServerHandler.java index ef225f9..3da8f50 100644 --- a/src/main/java/com/teragrep/lsh_01/HttpServerHandler.java +++ b/src/main/java/com/teragrep/lsh_01/HttpServerHandler.java @@ -29,8 +29,9 @@ import io.netty.handler.codec.http.HttpHeaderNames; import io.netty.handler.codec.http.HttpResponseStatus; import io.netty.handler.codec.http.HttpVersion; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; -import java.net.InetSocketAddress; import java.util.concurrent.ThreadPoolExecutor; import static io.netty.buffer.Unpooled.copiedBuffer; @@ -40,6 +41,8 @@ */ public class HttpServerHandler extends SimpleChannelInboundHandler { + private final static Logger LOGGER = LogManager.getLogger(HttpServerHandler.class); + private final IMessageHandler messageHandler; private final ThreadPoolExecutor executorGroup; private final HttpResponseStatus responseStatus; @@ -59,12 +62,10 @@ public HttpServerHandler( @Override public void channelRead0(ChannelHandlerContext ctx, FullHttpRequest msg) { - final String remoteAddress = ((InetSocketAddress) ctx.channel().remoteAddress()).getAddress().getHostAddress(); msg.retain(); final MessageProcessor messageProcessor = new MessageProcessor( ctx, msg, - remoteAddress, messageHandler, responseStatus, internalEndpointUrlConfig diff --git a/src/main/java/com/teragrep/lsh_01/IMessageHandler.java b/src/main/java/com/teragrep/lsh_01/IMessageHandler.java index 148b91a..8b4b4cf 100644 --- a/src/main/java/com/teragrep/lsh_01/IMessageHandler.java +++ b/src/main/java/com/teragrep/lsh_01/IMessageHandler.java @@ -31,11 +31,10 @@ public interface IMessageHandler { /** * This is triggered on every new message parsed by the http handler and should be executed in the ruby world. * - * @param remoteAddress * @param headers * @param body */ - boolean onNewMessage(String remoteAddress, Subject subject, Map headers, String body); + boolean onNewMessage(Subject subject, Map headers, String body); /** * @param token diff --git a/src/main/java/com/teragrep/lsh_01/MessageProcessor.java b/src/main/java/com/teragrep/lsh_01/MessageProcessor.java index 9e600a0..8476dd1 100644 --- a/src/main/java/com/teragrep/lsh_01/MessageProcessor.java +++ b/src/main/java/com/teragrep/lsh_01/MessageProcessor.java @@ -45,7 +45,6 @@ public class MessageProcessor implements RejectableRunnable { private final ChannelHandlerContext ctx; private final FullHttpRequest req; - private final String remoteAddress; private final IMessageHandler messageHandler; private final HttpResponseStatus responseStatus; private final InternalEndpointUrlConfig internalEndpointUrlConfig; @@ -56,14 +55,12 @@ public class MessageProcessor implements RejectableRunnable { MessageProcessor( ChannelHandlerContext ctx, FullHttpRequest req, - String remoteAddress, IMessageHandler messageHandler, HttpResponseStatus responseStatus, InternalEndpointUrlConfig internalEndpointUrlConfig ) { this.ctx = ctx; this.req = req; - this.remoteAddress = remoteAddress; this.messageHandler = messageHandler; this.responseStatus = responseStatus; this.internalEndpointUrlConfig = internalEndpointUrlConfig; @@ -137,7 +134,7 @@ private boolean isInternalEndpoint() { private FullHttpResponse processMessage(Subject subject) { final Map formattedHeaders = formatHeaders(req.headers()); final String body = req.content().toString(UTF8_CHARSET); - if (messageHandler.onNewMessage(remoteAddress, subject, formattedHeaders, body)) { + if (messageHandler.onNewMessage(subject, formattedHeaders, body)) { return generateResponse(messageHandler.responseHeaders()); } else { diff --git a/src/main/java/com/teragrep/lsh_01/RelpConversion.java b/src/main/java/com/teragrep/lsh_01/RelpConversion.java index ab48e2d..e1bccf2 100644 --- a/src/main/java/com/teragrep/lsh_01/RelpConversion.java +++ b/src/main/java/com/teragrep/lsh_01/RelpConversion.java @@ -64,7 +64,7 @@ public RelpConversion( this.payloadConfig = payloadConfig; } - public boolean onNewMessage(String remoteAddress, Subject subject, Map headers, String body) { + public boolean onNewMessage(Subject subject, Map headers, String body) { try { if (payloadConfig.splitEnabled) { Pattern splitPattern = Pattern.compile(payloadConfig.splitRegex); diff --git a/src/main/java/com/teragrep/lsh_01/config/PayloadConfig.java b/src/main/java/com/teragrep/lsh_01/config/PayloadConfig.java index 0f28584..dc2260a 100644 --- a/src/main/java/com/teragrep/lsh_01/config/PayloadConfig.java +++ b/src/main/java/com/teragrep/lsh_01/config/PayloadConfig.java @@ -39,7 +39,7 @@ public PayloadConfig() { public void validate() { if (splitEnabled) { try { - Pattern splitPattern = Pattern.compile(splitRegex); + Pattern.compile(splitRegex); } catch (PatternSyntaxException e) { throw new IllegalArgumentException(