Skip to content

Commit

Permalink
Fix #69: Coverity defects (#70)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
51-code authored Aug 19, 2024
1 parent 863b80a commit 605ef1f
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 11 deletions.
7 changes: 4 additions & 3 deletions src/main/java/com/teragrep/lsh_01/HttpServerHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -40,6 +41,8 @@
*/
public class HttpServerHandler extends SimpleChannelInboundHandler<FullHttpRequest> {

private final static Logger LOGGER = LogManager.getLogger(HttpServerHandler.class);

private final IMessageHandler messageHandler;
private final ThreadPoolExecutor executorGroup;
private final HttpResponseStatus responseStatus;
Expand All @@ -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
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/com/teragrep/lsh_01/IMessageHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, String> headers, String body);
boolean onNewMessage(Subject subject, Map<String, String> headers, String body);

/**
* @param token
Expand Down
5 changes: 1 addition & 4 deletions src/main/java/com/teragrep/lsh_01/MessageProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -137,7 +134,7 @@ private boolean isInternalEndpoint() {
private FullHttpResponse processMessage(Subject subject) {
final Map<String, String> 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 {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/teragrep/lsh_01/RelpConversion.java
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public RelpConversion(
this.payloadConfig = payloadConfig;
}

public boolean onNewMessage(String remoteAddress, Subject subject, Map<String, String> headers, String body) {
public boolean onNewMessage(Subject subject, Map<String, String> headers, String body) {
try {
if (payloadConfig.splitEnabled) {
Pattern splitPattern = Pattern.compile(payloadConfig.splitRegex);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit 605ef1f

Please sign in to comment.