eclipse-ee4j / mojarra

Mojarra, a Jakarta Faces implementation
Other
158 stars 107 forks source link

jakarta.faces.WEBAPP_CONTRACTS_DIRECTORY spec/impl mismatch #5329

Closed BalusC closed 8 months ago

BalusC commented 9 months ago

Discovered while working on https://github.com/eclipse-ee4j/mojarra/pull/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 https://github.com/eclipse-ee4j/mojarra/issues/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 commented 9 months ago

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);
}