[
https://issues.apache.org/jira/browse/LOG4J2-2721?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16975967#comment-16975967
]
Rémi C. commented on LOG4J2-2721:
---------------------------------
I really think this complexity should be encapsulate in MapMessage.java instead
of writing a million times at different place in an application
String.valueOf() because you are change you class type for your logs. This
require to all developers to be aware of this issue and I do not consider this
"Lean".
Other point, in a prod environment, the user in the header will not be null,
but in a local environment when you test it is. This provide code that will
work in all environments.
{code:java}
@SuppressWarnings("unchecked")
public M with(final String candidateKey, final String value) {
final String key = toKey(candidateKey);
put(key, String.valueOf(value));
return (M) this;
}
{code}
I just add your solution, which is certainly the good way to validate a null
but my point is that it should be encapsulate in MapMessage.java.
Another point is that duplicating code is a code smell.
When we do String.valueOf(myNullValue); it returns "null" which is an expected
behavior.
But when we do Short.valueOf(myNullValue); (Note : Short type or String type -
Short.valueOf(ObjectType) method does not exist). it returns a NPE for a Short
type and a NumberFormatException for a String type.
> My point is that it is an expected behavior that it crash for a Short, but
not for a String because the java class String handle the null by default.
> Thread crash when parameter is a null value for StringMapMessage
> ----------------------------------------------------------------
>
> Key: LOG4J2-2721
> URL: https://issues.apache.org/jira/browse/LOG4J2-2721
> Project: Log4j 2
> Issue Type: Bug
> Components: API
> Affects Versions: 2.12.1
> Reporter: Rémi C.
> Priority: Major
> Original Estimate: 4h
> Remaining Estimate: 4h
>
> {code:java}
> logger.info(myMarker, new StringMapMessage()
> .with("message", "Test message")
> .with("event.action", null)
> .with("event.category", "General"));{code}
> This will crash. It is not supposed to happen, but sometimes a parameter can
> be null unexpectedly.
>
> MapMessage should be "null safe".
>
> [https://github.com/apache/logging-log4j2/blob/master/log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java]
> line 732
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
