Skip to content

Fix null context class loader handling - #105

Draft
Aayush10016 wants to merge 2 commits into
apache:masterfrom
Aayush10016:fix/issue-90-null-context-classloader
Draft

Fix null context class loader handling#105
Aayush10016 wants to merge 2 commits into
apache:masterfrom
Aayush10016:fix/issue-90-null-context-classloader

Conversation

@Aayush10016

Copy link
Copy Markdown

Summary

  • avoid dereferencing a null thread context class loader
  • return the existing unresolved-location result and diagnostic instead of throwing
  • add a regression test that restores the original context class loader after verification

Root cause

ClasspathResourceLocatorStrategy.resolve called getResource unconditionally on Thread.currentThread().getContextClassLoader(). The context class loader is allowed to be null in some runtime environments, which caused a NullPointerException.

Testing

  • mvn -Dtest=ClasspathResourceLocatorStrategyTest test
    • 5 tests passed
  • mvn -Prun-its verify
    • 80 tests passed
    • Checkstyle, Spotless, Apache RAT, packaging, and the integration-test profile succeeded

Fixes #90

ClassLoader cloader = Thread.currentThread().getContextClassLoader();

URL resource = cloader.getResource(locationSpecification);
URL resource = cloader != null ? cloader.getResource(locationSpecification) : null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't right. If getContextClassLoader() returns null, you should use the system classloader.

void shouldFailToResolveWhenContextClassLoaderIsNull() {
Thread thread = Thread.currentThread();
ClassLoader contextClassLoader = thread.getContextClassLoader();
MessageHolder mh = new DefaultMessageHolder();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mh --> messageHolder

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ClasspathResourceLocatorStrategy: NPE when context ClassLoader is null

2 participants