search

eclipse-ee4j / mojarra

Mojarra, a Jakarta Faces implementation
Other
162 stars 110 forks source link

fix NPE if getResourcePaths return null #5194

Closed zfqjava closed 1 year ago

zfqjava commented 1 year ago

java.lang.NullPointerException: Cannot invoke "java.util.Collection.size()" because "c" is null at java.base/java.util.ArrayDeque.addAll(ArrayDeque.java:322) at com.sun.faces.application.resource.ResourcePathsIterator.visit(ResourcePathsIterator.java:67) at com.sun.faces.application.resource.ResourcePathsIterator.tryTake(ResourcePathsIterator.java:79) at com.sun.faces.application.resource.ResourcePathsIterator.hasNext(ResourcePathsIterator.java:51)

codylerum commented 1 year ago

Should ExternalContext#getResourcePaths be returning an empty set rather than a null?

https://jakarta.ee/specifications/platform/10/apidocs/jakarta/faces/context/externalcontext#getResourcePaths-java.lang.String-

Doc says

Return the Set of resource paths for all application resources whose resource path starts with the specified argument.

Says it will return a Set, not that it will return a null

melloware commented 1 year ago

Agreed this should return a new empty Stack if bill

BalusC commented 1 year ago

This is exactly how ServletContext#getResourcePaths() is implemented: https://jakarta.ee/specifications/platform/10/apidocs/jakarta/servlet/servletcontext#getResourcePaths(java.lang.String) The ExternalContext#getResourcePaths() is merely a facade thereof (i.e. it isn't intended to manipulate the results of underlying delegates).

We can of course deviate from this, but this is a breaking change and should be done in a major version only. I'd for now prefer to update the javadoc of ExternalContext#getResourcePaths() for clarity.

BalusC commented 1 year ago

Note that this PR is made for master branch, not for 2.3/3.0/4.0. If this was not your original intent, adjust accordingly.

BalusC commented 1 year ago

No comment a week. Merging anyway.

To backport you'll need to create a PR for the desired version.