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

jakarta.faces.WEBAPP_CONTRACTS_DIRECTORY spec/impl mismatch #5329

Closed
BalusC opened this issue Oct 8, 2023 · 1 comment
Closed

jakarta.faces.WEBAPP_CONTRACTS_DIRECTORY spec/impl mismatch #5329

BalusC opened this issue Oct 8, 2023 · 1 comment

Comments

@BalusC
Copy link
Contributor

BalusC commented Oct 8, 2023

Discovered while working on #5328

Spec says it may not start with /

 * <p class="changed_added_2_2">
 * If a <code>&lt;context-param&gt;</code> with the param name equal to the value of
 * {@link #WEBAPP_CONTRACTS_DIRECTORY_PARAM_NAME} exists, the runtime must interpret its value as a path, relative to
 * the web app root, where resource library contracts are to be located. This param value must not start with a "/",
 * though it may contain "/" characters. If no such <code>&lt;context-param&gt;</code> exists, or its value is invalid,
 * the value "contracts", without the quotes, must be used by the runtime as the value.
 * </p>

When set as such in web.xml

    <context-param>
        <param-name>jakarta.faces.WEBAPP_CONTRACTS_DIRECTORY</param-name>
        <param-value>foo</param-value>
    </context-param>

Impl throws exception because it does not start with /

Caused by: java.lang.IllegalArgumentException: Path [foo] does not start with a "/" character
	at org.apache.catalina.core.ApplicationContext.getResourcePaths(ApplicationContext.java:571)
	at org.apache.catalina.core.ApplicationContextFacade.getResourcePaths(ApplicationContextFacade.java:183)
	at org.apache.catalina.core.StandardContext$NoPluggabilityServletContext.getResourcePaths(StandardContext.java:6447)
	at com.sun.faces.config.initfacescontext.ServletContextAdapter.getResourcePaths(ServletContextAdapter.java:248)
	at com.sun.faces.config.WebConfiguration.discoverResourceLibraryContracts(WebConfiguration.java:401)
	at com.sun.faces.config.WebConfiguration.doPostBringupActions(WebConfiguration.java:389)
	at com.sun.faces.config.ConfigureListener.contextInitialized(ConfigureListener.java:227)
	... 30 more

This was earlier discovered and pointed out in #5118

Wow, that's an awkward oversight there.

The original spec issue is this: jakartaee/faces#996
There's even a comment pointing out this deviation: jakartaee/faces#996 (comment)
I guess the deviation was caused by mix-up of two initial proposals as commented here: jakartaee/faces#996 (comment)

But in hindsight not fully fixed because the code is not DRY wrt obtaining contracts param. The same code was copypasted in several other classes.

@BalusC
Copy link
Contributor Author

BalusC commented Oct 8, 2023

I also noticed that these pieces of logic in CompositionHandler and IncludeHandler do not take into account that the path can be a relative path and incorrectly assume them always to be the absolute path.

if (path.startsWith(webConfig.getOptionValue(WebConfiguration.WebContextInitParameter.WebAppContractsDirectory))) {
    throw new TagAttributeException(tag, src, "Invalid src, contract resources cannot be accessed this way : " + path);
}

BalusC added a commit that referenced this issue Oct 8, 2023
single place and fixed a couple of misassumptions about it always being
an absolute path #5329
BalusC added a commit that referenced this issue Oct 14, 2023
BalusC added a commit that referenced this issue Oct 14, 2023
@BalusC BalusC closed this as completed Oct 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants