ramanathan1504 commented on PR #4172: URL: https://github.com/apache/logging-log4j2/pull/4172#issuecomment-5004896289
@SebTardif, thanks for the fix. we'll need a unit test reproducing the NPE. Also, could you check if the other parsers in this switch statement (parseLogger, etc.) need the exact same null check? Let's fix them all at once. Lastly, let's make sure we log a warning when appender == null so it doesn't fail silently for the user. 1. Unit Test --> add a test case with an invalid XML configuration that triggers this scenario to prevent future regressions. 2. Audit sibling methods: Please check the other tags in this parse() switch block (e.g., parseCategory, parseLogger). If they can also return null on invalid configurations, they need the exact same null-guard. 3. Logging: Does parseAppender() emit a warning via the StatusLogger when it fails? If not, please add a LOGGER.warn(...) inside your if block so we don't silently skip configuration errors. 4. Name check: Can appender.getName() ever be null here? If so, we should guard against that as well so map insertion doesn't fail. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
