Skip to content

fix: null-check AppenderDynamicMBean setLayout instantiation - #4219

Closed
SebTardif wants to merge 2 commits into
apache:2.xfrom
SebTardif:fix/appender-mbean-null-layout
Closed

fix: null-check AppenderDynamicMBean setLayout instantiation#4219
SebTardif wants to merge 2 commits into
apache:2.xfrom
SebTardif:fix/appender-mbean-null-layout

Conversation

@SebTardif

Copy link
Copy Markdown
Contributor

What Problem This Solves

AppenderDynamicMBean.invoke("setLayout") calls OptionConverter.instantiateByClassName(..., null) and immediately passes the result to appender.setLayout(...). When the class name is invalid or not a Layout, instantiation returns null and the next line NPEs. Same pattern as LoggerDynamicMBean.addAppender (#4185).

Evidence

Failure scenario

final Layout layout =
    (Layout) OptionConverter.instantiateByClassName((String) params[0], Layout.class, null);
appender.setLayout(layout); // NPE when instantiate returns null

Red-green / tests

export JAVA_HOME=$(/usr/libexec/java_home -v 17)
./mvnw -pl log4j-1.2-api -am test \
  -Dtest=AppenderDynamicMBeanTest \
  -Dsurefire.failIfNoSpecifiedTests=false
  • Invalid class: no NPE, layout remains null
  • Valid PatternLayout: layout attached

Sibling audit

Summary

Null-check + error log on failed layout instantiation; unit tests + changelog.

OptionConverter.instantiateByClassName returns null when the class is missing
or not a Layout. setLayout immediately called appender.setLayout and could NPE
on invalid JMX setLayout class names. Log an error and skip attach instead.

Sibling of the LoggerDynamicMBean.addAppender guard (apache#4185).

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
@ramanathan1504

Copy link
Copy Markdown
Contributor

@SebTardif PR #4185 still not merged in 2.x, can you add this changes in the related one".
Thanks

@ramanathan1504

Copy link
Copy Markdown
Contributor

closing this by #4185 still not merged.thesePR changes its related to existing one
@SebTardif if you feel it should be separate, we will discuss here. Now am closing the PRand you have every rights to reopen if this needed separate.
Thanks

SebTardif added a commit to SebTardif/logging-log4j2 that referenced this pull request Aug 7, 2026
Folded from closed apache#4219 per review request; both JMX null-guards ship here.

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
@SebTardif

Copy link
Copy Markdown
Contributor Author

@ramanathan1504 Agreed. AppenderDynamicMBean setLayout null-check is now on #4185 (same JMX instantiateByClassName pattern). Leaving this PR closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants