Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix NPE if getResourcePaths return null #5194

Merged
merged 2 commits into from
Jan 26, 2023
Merged

Conversation

zfqjava
Copy link
Contributor

@zfqjava zfqjava commented Dec 23, 2022

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
Copy link
Contributor

codylerum commented Jan 13, 2023

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
Copy link
Contributor

Agreed this should return a new empty Stack if bill

@BalusC
Copy link
Contributor

BalusC commented Jan 20, 2023

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
Copy link
Contributor

BalusC commented Jan 20, 2023

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
Copy link
Contributor

BalusC commented Jan 26, 2023

No comment a week. Merging anyway.

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

@BalusC BalusC closed this Jan 26, 2023
@BalusC BalusC reopened this Jan 26, 2023
@BalusC BalusC merged commit 91958aa into eclipse-ee4j:master Jan 26, 2023
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.

4 participants