[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3948: ARTEMIS-3677 mitigate NPE when browsing messages

[GitBox <gi...@apache.org>]

gemmellr commented on a change in pull request #3948:
URL: https://github.com/apache/activemq-artemis/pull/3948#discussion_r803878296



##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java
##########
@@ -876,7 +875,7 @@ public final void receiveBuffer(ByteBuf buffer) {
                // keep fields like _AMQ_ACTUAL_EXPIRY in their original type
                map.put(extraPropertiesPrefix + s.toString(), o);
             } else {
-               map.put(extraPropertiesPrefix + s.toString(), JsonUtil.truncate(o.toString(), valueSizeLimit));
+               map.put(extraPropertiesPrefix + s.toString(), JsonUtil.truncate(o != null ? o.toString() : o, valueSizeLimit));

Review comment:
       With the comment about wanting to keep original types, its not clear to me why it wouldnt just put the null in. Or omit it.

##########
File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/JsonUtil.java
##########
@@ -338,6 +338,9 @@ public static String truncateString(final String str, final int valueSizeLimit)
    }
 
    public static Object truncate(final Object value, final int valueSizeLimit) {
+      if (value == null) {
+         return "";

Review comment:
       Doesnt necessarily follow to me that the truncation of a null is an empty string. Why not return null? Or throw?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org