Github user michaelandrepearce commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2334#discussion_r220697913
— Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/protocol/ProtocolHandler.java —
@@ -196,6 +196,10 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) t
}
ProtocolManager protocolManagerToUse = protocolMap.get(protocolToUse);
+ if (protocolManagerToUse == null) {
+ ActiveMQServerLogger.LOGGER.failedToFindProtocolManager(ctx.channel().remoteAddress().toString(), ctx.channel().localAddress().toString(), protocolToUse, protocolMap.keySet().toString());
— End diff –
Question, how sure/guarenteed is it that these objects will def be wont be null them selves, aka doing this logger wont cause an NPE.
ctx.channel().remoteAddress().toString()
ctx.channel().localAddress().toString()
is it worth null checking the dot train?
e.g. is it worth doing?
ctx.channel() == null ? null : ctx.channel().localAddress() == null ? null : ctx.channel().localAddress().toString
GitHub user jbertram opened a pull request:
https://github.com/apache/activemq-artemis/pull/2334
ARTEMIS-2098potential NPE when decoding protocolYou can merge this pull request into a Git repository by running:
$ git pull https://github.com/jbertram/activemq-artemis
ARTEMIS-2098Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/2334.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2334
commit 9283e6227581078c18d5b73261e8685d437e1cb0
Author: Justin Bertram <jbertram@...>
Date: 2018-09-26T19:27:14Z
ARTEMIS-2098potential NPE when decoding protocol