[MAGNOLIA-6440] Duplicated resources on classpath may prevent magnolia from start with a NPE Created: 15/Nov/15  Updated: 11/Mar/16  Resolved: 04/Mar/16

Status: Closed
Project: Magnolia
Component/s: None
Affects Version/s: 5.4.1, 5.4.2, 5.4.3
Fix Version/s: None

Type: Bug Priority: Major
Reporter: Fabrizio Giustina Assignee: Unassigned
Resolution: Obsolete Votes: 0
Labels: papercut
Remaining Estimate: 1d
Time Spent: 7d 4.5h
Original Estimate: 5d

Issue Links:
dependency
depends upon MAGNOLIA-6523 Refactor ClasspathResourceOrigin and ... Closed
supersession
is superseded by MAGNOLIA-6581 Classpath service won't serve resourc... Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:
Sprint: Basel 34
Story Points: 8

 Description   

... this can really be an headache for many users: if there are 2 classpath resources with the same name, one pointing to a directory and one to a file magnolia initialization fails with a NPE and without any useful message that could help to diagnose the problem.

Sample test to reproduce the problem:

  • add a file named "test" to a jar/module
  • add a folder named "test" in WEB-INF/classes

Magnolia will not start anymore with the following stacktrace:

ERROR MagnoliaServletContextListener (MagnoliaServletContextListener.java:177) Oops, Magnolia could not be started
com.google.inject.CreationException: Guice creation errors:

1) Error injecting constructor, java.lang.NullPointerException
  at info.magnolia.resourceloader.DefaultResourceOrigins.<init>(DefaultResourceOrigins.java:64)
  at info.magnolia.resourceloader.DefaultResourceOrigins.class(DefaultResourceOrigins.java:52)
  while locating info.magnolia.resourceloader.DefaultResourceOrigins

1 error
	at com.google.inject.internal.Errors.throwCreationExceptionIfErrorsExist(Errors.java:435)
	at com.google.inject.internal.InternalInjectorCreator.injectDynamically(InternalInjectorCreator.java:183)
	at com.google.inject.internal.InternalInjectorCreator.build(InternalInjectorCreator.java:109)
	at com.google.inject.Guice.createInjector(Guice.java:95)
	at com.google.inject.Guice.createInjector(Guice.java:83)
	at info.magnolia.objectfactory.guice.GuiceComponentProviderBuilder.build(GuiceComponentProviderBuilder.java:145)
	at info.magnolia.objectfactory.guice.GuiceComponentProviderBuilder.build(GuiceComponentProviderBuilder.java:155)
	at info.magnolia.cms.beans.config.ConfigLoader.load(ConfigLoader.java:153)
	at info.magnolia.init.MagnoliaServletContextListener$1.doExec(MagnoliaServletContextListener.java:250)
	at info.magnolia.context.MgnlContext$VoidOp.exec(MgnlContext.java:421)
	at info.magnolia.context.MgnlContext$VoidOp.exec(MgnlContext.java:418)
	at info.magnolia.context.MgnlContext.doInSystemContext(MgnlContext.java:392)
	at info.magnolia.init.MagnoliaServletContextListener.startServer(MagnoliaServletContextListener.java:247)
	at info.magnolia.init.MagnoliaServletContextListener.contextInitialized(MagnoliaServletContextListener.java:173)
	at info.magnolia.init.MagnoliaServletContextListener.contextInitialized(MagnoliaServletContextListener.java:127)
	at org.apache.catalina.core.StandardContext.listenerStart(StandardContext.java:4939)
	at org.apache.catalina.core.StandardContext.startInternal(StandardContext.java:5434)
	at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:150)
	at org.apache.catalina.core.ContainerBase.addChildInternal(ContainerBase.java:901)
	at org.apache.catalina.core.ContainerBase.addChild(ContainerBase.java:877)
	at org.apache.catalina.core.StandardHost.addChild(StandardHost.java:633)
	at org.apache.catalina.startup.HostConfig.deployDescriptor(HostConfig.java:656)
	at org.apache.catalina.startup.HostConfig$DeployDescriptor.run(HostConfig.java:1635)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.NullPointerException
	at info.magnolia.resourceloader.classpath.ClasspathResource.addChild(ClasspathResource.java:125)
	at info.magnolia.resourceloader.classpath.ClasspathResource.setParent(ClasspathResource.java:99)
	at info.magnolia.resourceloader.classpath.ClasspathResourceOrigin.createResourcesFor(ClasspathResourceOrigin.java:330)
	at info.magnolia.resourceloader.classpath.ClasspathResourceOrigin.collectResources(ClasspathResourceOrigin.java:229)
	at info.magnolia.resourceloader.classpath.ClasspathResourceOrigin.<init>(ClasspathResourceOrigin.java:100)
	at info.magnolia.resourceloader.classpath.ClasspathResourceOriginFactory.create(ClasspathResourceOriginFactory.java:18)
	at info.magnolia.resourceloader.DefaultResourceOrigins.<init>(DefaultResourceOrigins.java:70)
	at info.magnolia.resourceloader.DefaultResourceOrigins$$FastClassByGuice$$6454fe2d.newInstance(<generated>)
	at com.google.inject.internal.cglib.reflect.$FastConstructor.newInstance(FastConstructor.java:40)
	at com.google.inject.internal.DefaultConstructionProxyFactory$1.newInstance(DefaultConstructionProxyFactory.java:60)
	at com.google.inject.internal.ConstructorInjector.construct(ConstructorInjector.java:85)
	at com.google.inject.internal.ConstructorBindingImpl$Factory.get(ConstructorBindingImpl.java:254)
	at com.google.inject.internal.ProviderToInternalFactoryAdapter$1.call(ProviderToInternalFactoryAdapter.java:46)
	at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1031)
	at com.google.inject.internal.ProviderToInternalFactoryAdapter.get(ProviderToInternalFactoryAdapter.java:40)
	at com.google.inject.Scopes$1$1.get(Scopes.java:65)
	at com.google.inject.internal.InternalFactoryToProviderAdapter.get(InternalFactoryToProviderAdapter.java:40)
	at com.google.inject.internal.InternalInjectorCreator$1.call(InternalInjectorCreator.java:204)
	at com.google.inject.internal.InternalInjectorCreator$1.call(InternalInjectorCreator.java:198)
	at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1024)
	at com.google.inject.internal.InternalInjectorCreator.loadEagerSingletons(InternalInjectorCreator.java:198)
	at com.google.inject.internal.InternalInjectorCreator.injectDynamically(InternalInjectorCreator.java:179)
	... 26 more

Depending on the order of the resource loaded the error may be more informative, when the directory is loaded before the file you get something like:

2015-11-15 12:02:34,463 ERROR ModuleManagerImpl (ModuleManagerImpl.java:380) Can't start module rendering
java.lang.IllegalArgumentException: Multiple entries with same key: /LICENSE=ClasspathResource{origin=classpath,path=/LICENSE,(directory)} and /LICENSE=ClasspathResource{origin=classpath,path=/LICENSE,file}

The first situation (file caught first) is definitively more problematic since it's really hard to debug... and unfortunately it may happen pretty easily if you have any dependency which contains files in the root folder.



 Comments   
Comment by Ngoc Nguyenthanh [ 19/Nov/15 ]

Steps to get the error:

  • Add a file named "stephen" without any extension (actually it's a text file) in resource of a module (I tried with travel-demo, magnolia-core, magnolia-i18n.
  • Rebuild the module.
  • Rebuild the community web app with appropriate versions of modules above.
  • Navigation to lib folder of webapp, extract the jar files. Double check to make sure "stephen" file is available in the root folder of the jar.
  • Start the server. Make sure it has been installed successfully.
  • Add a empty folder named "stephen" in WEB-INF/classes. Add a file to this folder also - important step.
  • Restart the server and you should get the error message
Comment by Ngoc Nguyenthanh [ 20/Nov/15 ]

We have 2 class-paths

  • A file: /stephen without extension in root of jar
  • /stephen/hello in WEB-INF/classes

When we collect resources from class-paths info.magnolia.resourceloader.classpath.ClasspathResourceOrigin#collectResources by using org.reflections.Reflections#getResources. The method will return a Set<String> contains 2 paths above.
Next, we create ClasspathResource by info.magnolia.resourceloader.classpath.ClasspathResourceOrigin#createResourcesFor for each resource paths.
The problem comes from info/magnolia/resourceloader/classpath/ClasspathResourceOrigin.java:329. Because the resource list is a {{ Map<String, ClasspathResource>}} with path is the key. Then it will return wrong parent of "hello" file. Now /stephen is a file in the root folder of jar not a folder inside WEB-INF/classes. And it leads to info.magnolia.resourceloader.classpath.ClasspathResource#setParent failed when addChild
Depends on the order of the resources path list stephen can be a file or folder.

Comment by Ngoc Nguyenthanh [ 20/Nov/15 ]

mgeljic Phew, The issue related to class paths, even the order of the paths list.
I'm trying to write an unit-test to cover the bug but I can not find a good test case for it. I don't know how to mock it or create a conflicted paths list.
When we collect resources, resources list is conflicted. We need to keep the file and folder in separated.
So far, I have no ideas for unit test and solution for this issue.
Could you please give me some advices?

Comment by Ngoc Nguyenthanh [ 01/Dec/15 ]

I fixed by:

  • In the entry point info.magnolia.resourceloader.classpath.ClasspathResourceOrigin#getByPath serve for file if we have file and directory are same path.
  • Remove unique path checking in info.magnolia.resourceloader.layered.LayeredResourceOrigin#aggregateSet
Comment by Ngoc Nguyenthanh [ 05/Jan/16 ]

Copying comments from PR
ilgun After conversation with mgeljic, we both agreed on another possible solution.
Having Set<ClasspathResource> resourceCache but overriding toString(), equals(), hashCode() (if not overridden yet.). (Reasons for overriding methods-> http://www.ideyatech.com/effective-java-equals-and-hashcode/)
With this approach at most we might have two resources with same path, but we have the chance to differentiate file and folder, but we will never have the more than one file-resource(i hope this term makes sense).

mgeljic equals() and hashCode() on ClasspathResource should be covered already with Lombok annotation @EqualsAndHashCode, but always worth double-checking/testing

Comment by Mikaël Geljić [ 04/Feb/16 ]

Current progress was declined in favor of bigger fundamental changes in classpath resource-loading. Linking to related issue and reopening until we verify that the new implementation handles this case gracefully.

Comment by Mikaël Geljić [ 04/Mar/16 ]

Closing that one as new implementation doesn't fail (it actually ignores the overlapping sub-directories, which is wrong as well)—see follow-up issue MAGNOLIA-6581.

Generated at Mon Feb 12 04:14:32 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.