Skip to content

Commit

Permalink
Merge pull request #5330 from eclipse-ee4j/mojarra_issue_5329_contrac…
Browse files Browse the repository at this point in the history
…ts_directory_impl_is_broken

Fixed jakarta.faces.WEBAPP_CONTRACTS_DIRECTORY spec/impl mismatch
  • Loading branch information
BalusC authored Oct 14, 2023
2 parents c2034cc + 9f72851 commit c694635
Show file tree
Hide file tree
Showing 11 changed files with 71 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,12 @@ public static ApplicationAssociate getCurrentInstance() {
}

public static ApplicationAssociate getInstance() {
FacesContext facesContext = FacesContext.getCurrentInstance();
return getInstance(FacesContext.getCurrentInstance());
}

public static ApplicationAssociate getInstance(FacesContext facesContext) {
if (facesContext == null) {
return null;
return null;
}

return ApplicationAssociate.getInstance(facesContext.getExternalContext());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import static com.sun.faces.RIConstants.FLOW_IN_JAR_PREFIX;
import static com.sun.faces.config.WebConfiguration.META_INF_CONTRACTS_DIR;
import static com.sun.faces.config.WebConfiguration.WebContextInitParameter.FaceletsSuffix;
import static com.sun.faces.config.WebConfiguration.WebContextInitParameter.WebAppContractsDirectory;
import static jakarta.faces.application.ResourceVisitOption.TOP_LEVEL_VIEWS_ONLY;
import static java.util.Spliterator.DISTINCT;
import static java.util.Spliterators.spliteratorUnknownSize;
Expand Down Expand Up @@ -48,12 +47,12 @@ public class FaceletWebappResourceHelper extends ResourceHelper {

private static final String[] RESTRICTED_DIRECTORIES = { "/WEB-INF/", "/META-INF/" };

private final String webAppContractsDirectory;
private final ResourceHelper webappResourceHelper;
private final String[] configuredExtensions;

public FaceletWebappResourceHelper() {
public FaceletWebappResourceHelper(WebappResourceHelper webappResourceHelper) {
this.webappResourceHelper = webappResourceHelper;
WebConfiguration webConfig = WebConfiguration.getInstance();
webAppContractsDirectory = webConfig.getOptionValue(WebAppContractsDirectory);
configuredExtensions = webConfig.getOptionValue(FaceletsSuffix, " ");
}

Expand Down Expand Up @@ -150,9 +149,9 @@ private URL findResourceInfoConsideringContracts(FacesContext ctx, String baseRe

for (String contract : contracts) {
if (baseResourceName.startsWith("/")) {
resourceName = webAppContractsDirectory + "/" + contract + baseResourceName;
resourceName = getBaseContractsPath() + "/" + contract + baseResourceName;
} else {
resourceName = webAppContractsDirectory + "/" + contract + "/" + baseResourceName;
resourceName = getBaseContractsPath() + "/" + contract + "/" + baseResourceName;
}

url = Resource.getResourceUrl(ctx, resourceName);
Expand Down Expand Up @@ -226,7 +225,7 @@ public String getBaseResourcePath() {

@Override
public String getBaseContractsPath() {
return webAppContractsDirectory;
return webappResourceHelper.getBaseContractsPath();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package com.sun.faces.application.resource;

import static com.sun.faces.util.Util.ensureLeadingSlash;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -57,12 +59,15 @@ public class ResourceManager {
*/
private static final Pattern CONFIG_MIMETYPE_PATTERN = Pattern.compile("[a-z-]*/[a-z0-9.\\*-]*");

private FaceletWebappResourceHelper faceletWebappResourceHelper = new FaceletWebappResourceHelper();

/**
* {@link ResourceHelper} used for looking up webapp-based resources.
*/
private ResourceHelper webappResourceHelper = new WebappResourceHelper();
private WebappResourceHelper webappResourceHelper = new WebappResourceHelper();

/**
* {@link ResourceHelper} used for looking up webapp-based facelets resources.
*/
private FaceletWebappResourceHelper faceletWebappResourceHelper = new FaceletWebappResourceHelper(webappResourceHelper);

/**
* {@link ResourceHelper} used for looking up classpath-based resources.
Expand Down Expand Up @@ -175,6 +180,14 @@ public ResourceInfo findResource(String libraryName, String resourceName, String
public Stream<String> getViewResources(FacesContext facesContext, String path, int maxDepth, ResourceVisitOption... options) {
return faceletWebappResourceHelper.getViewResources(facesContext, path, maxDepth, options);
}

public String getBaseContractsPath() {
return faceletWebappResourceHelper.getBaseContractsPath();
}

public boolean isContractsResource(String path) {
return ensureLeadingSlash(path).startsWith(getBaseContractsPath());
}

// ----------------------------------------------------- Private Methods

Expand Down
4 changes: 2 additions & 2 deletions impl/src/main/java/com/sun/faces/config/WebConfiguration.java
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,8 @@ private void discoverResourceLibraryContracts() {
Set<String> candidates;

// Scan for "contractMappings" in the web app root
String contractsDirName = getOptionValue(WebContextInitParameter.WebAppContractsDirectory);
ApplicationAssociate associate = ApplicationAssociate.getCurrentInstance();
String contractsDirName = associate.getResourceManager().getBaseContractsPath();
assert null != contractsDirName;
candidates = extContex.getResourcePaths(contractsDirName);
if (null != candidates) {
Expand Down Expand Up @@ -437,7 +438,6 @@ private void discoverResourceLibraryContracts() {

Map<String, List<String>> contractMappings = new HashMap<>();

ApplicationAssociate associate = ApplicationAssociate.getCurrentInstance();
Map<String, List<String>> contractsFromConfig = associate.getResourceLibraryContracts();
List<String> contractsToExpose;

Expand Down
21 changes: 12 additions & 9 deletions impl/src/main/java/com/sun/faces/facelets/impl/DefaultFacelet.java
Original file line number Diff line number Diff line change
Expand Up @@ -230,14 +230,14 @@ public long getCreateTime() {
}

/**
* Delegates resolution to DefaultFaceletFactory reference. Also, caches URLs for relative paths.
* Delegates resolution to DefaultFaceletFactory reference. Also, caches URLs for absolute paths.
*
* @param path a relative url path
* @param relativePath a relative url path
* @return URL pointing to destination
* @throws IOException if there is a problem creating the URL for the path specified
*/
private URL getRelativePath(String path) throws IOException {
return factory.resolveURL(src, path);
private URL resolveURL(String relativePath) throws IOException {
return factory.resolveURL(src, relativePath);
}

/**
Expand Down Expand Up @@ -268,21 +268,21 @@ private void include(DefaultFaceletContext ctx, UIComponent parent) throws IOExc

/**
* Used for delegation by the DefaultFaceletContext. First pulls the URL from {@link #getRelativePath(String)
* getRelativePath(String)}, then calls
* getRelativePath(String)}, then validates that the path does not represent a contracts resource, then calls
* {@link #include(DefaultFaceletContext, jakarta.faces.component.UIComponent, String)}.
*
* @see FaceletContext#includeFacelet(UIComponent, String)
* @param ctx FaceletContext to pass to the included Facelet
* @param parent UIComponent to apply changes to
* @param path relative path to the desired Facelet from the FaceletContext
* @param relativePath relative path to the desired Facelet from the FaceletContext
* @throws IOException
* @throws FacesException
* @throws FaceletException
* @throws ELException
*/
public void include(DefaultFaceletContext ctx, UIComponent parent, String path) throws IOException {
public void include(DefaultFaceletContext ctx, UIComponent parent, String relativePath) throws IOException {
URL url;
if (path.equals(JAKARTA_FACES_ERROR_XHTML)) {
if (relativePath.equals(JAKARTA_FACES_ERROR_XHTML)) {
if (isDevelopment(ctx)) {
// try using this class' ClassLoader
url = getErrorFacelet(DefaultFacelet.class.getClassLoader());
Expand All @@ -293,7 +293,10 @@ public void include(DefaultFaceletContext ctx, UIComponent parent, String path)
return;
}
} else {
url = getRelativePath(path);
url = resolveURL(relativePath);
if (factory.isContractsResource(url)) {
throw new IOException("Contract resources cannot be accessed this way");
}
}
this.include(ctx, parent, url);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
import java.util.regex.Pattern;

import com.sun.faces.RIConstants;
import com.sun.faces.application.ApplicationAssociate;
import com.sun.faces.application.resource.ResourceManager;
import com.sun.faces.config.WebConfiguration;
import com.sun.faces.context.FacesFileNotFoundException;
import com.sun.faces.facelets.compiler.Compiler;
Expand Down Expand Up @@ -81,7 +83,9 @@ public class DefaultFaceletFactory {
// provides a custom one. The DefaultResourceResolver simply uses
// the ResourceHandler to do its work.
private DefaultResourceResolver resolver;
private ResourceManager manager;
private URL baseUrl;
private String baseUrlAsString;
private long refreshPeriod;
private FaceletCache<DefaultFacelet> cache;
private ConcurrentMap<String, FaceletCache<DefaultFacelet>> cachePerContract;
Expand All @@ -104,7 +108,9 @@ public final void init(FacesContext facesContext, Compiler compiler, DefaultReso
this.compiler = compiler;
cachePerContract = new ConcurrentHashMap<>();
this.resolver = resolver;
this.manager = ApplicationAssociate.getInstance(externalContext).getResourceManager();
baseUrl = resolver.resolveUrl("/");
baseUrlAsString = baseUrl.toExternalForm();
this.idMappers = config.isOptionEnabled(UseFaceletsID) ? null : new Cache<>(new IdMapperFactory());
refreshPeriod = refreshPeriod >= 0 ? refreshPeriod * 1000 : -1;
this.refreshPeriod = refreshPeriod;
Expand Down Expand Up @@ -167,6 +173,22 @@ public URL resolveURL(URL source, String path) throws IOException {
return new URL(source, path);
}

/**
* Returns true if given url is a contracts resource.
* @param url source url
* @return true if given url is a contracts resource.
*/
public boolean isContractsResource(URL url) {
String urlAsString = url.toExternalForm();

if (!urlAsString.startsWith(baseUrlAsString)) {
return false;
}

String path = urlAsString.substring(baseUrlAsString.length());
return manager.isContractsResource(path);
}

/**
* Create a Facelet from the passed URL. This method checks if the cached Facelet needs to be refreshed before
* returning. If so, uses the passed URL to build a new instance;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import java.util.logging.Level;
import java.util.logging.Logger;

import com.sun.faces.config.WebConfiguration;
import com.sun.faces.facelets.FaceletContextImplBase;
import com.sun.faces.facelets.TemplateClient;
import com.sun.faces.facelets.el.VariableMapperWrapper;
Expand Down Expand Up @@ -127,16 +126,12 @@ public void apply(FaceletContext ctxObj, UIComponent parent) throws IOException
if (path.trim().length() == 0) {
throw new TagAttributeException(tag, template, "Invalid path : " + path);
}
WebConfiguration webConfig = WebConfiguration.getInstance();
if (path.startsWith(webConfig.getOptionValue(WebConfiguration.WebContextInitParameter.WebAppContractsDirectory))) {
throw new TagAttributeException(tag, template, "Invalid path, contract resources cannot be accessed this way : " + path);
}
ctx.includeFacelet(parent, path);
} catch (IOException e) {
if (log.isLoggable(Level.FINE)) {
log.log(Level.FINE, e.toString(), e);
}
throw new TagAttributeException(tag, template, "Invalid path : " + path);
throw new TagAttributeException(tag, template, "Invalid path : " + path, e);
} finally {
ctx.popClient(this);
ctx.setVariableMapper(orig);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import java.util.logging.Level;
import java.util.logging.Logger;

import com.sun.faces.config.WebConfiguration;
import com.sun.faces.facelets.FaceletContextImplBase;
import com.sun.faces.facelets.TemplateClient;
import com.sun.faces.facelets.el.VariableMapperWrapper;
Expand Down Expand Up @@ -108,16 +107,12 @@ public void apply(FaceletContext ctxObj, UIComponent parent) throws IOException
if (path.trim().length() == 0) {
throw new TagAttributeException(tag, template, "Invalid path : " + path);
}
WebConfiguration webConfig = WebConfiguration.getInstance();
if (path.startsWith(webConfig.getOptionValue(WebConfiguration.WebContextInitParameter.WebAppContractsDirectory))) {
throw new TagAttributeException(tag, template, "Invalid path, contract resources cannot be accessed this way : " + path);
}
ctx.includeFacelet(parent, path);
} catch (IOException e) {
if (log.isLoggable(Level.FINE)) {
log.log(Level.FINE, e.toString(), e);
}
throw new TagAttributeException(tag, template, "Invalid path : " + path);
throw new TagAttributeException(tag, template, "Invalid path : " + path, e);
} finally {
ctx.setVariableMapper(orig);
ctx.popClient(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import java.util.logging.Level;
import java.util.logging.Logger;

import com.sun.faces.config.WebConfiguration;
import com.sun.faces.facelets.el.VariableMapperWrapper;
import com.sun.faces.facelets.tag.TagHandlerImpl;
import com.sun.faces.util.FacesLogger;
Expand Down Expand Up @@ -76,16 +75,12 @@ public void apply(FaceletContext ctx, UIComponent parent) throws IOException {
ctx.setVariableMapper(new VariableMapperWrapper(orig));
try {
nextHandler.apply(ctx, null);
WebConfiguration webConfig = WebConfiguration.getInstance();
if (path.startsWith(webConfig.getOptionValue(WebConfiguration.WebContextInitParameter.WebAppContractsDirectory))) {
throw new TagAttributeException(tag, src, "Invalid src, contract resources cannot be accessed this way : " + path);
}
ctx.includeFacelet(parent, path);
} catch (IOException e) {
if (log.isLoggable(Level.FINE)) {
log.log(Level.FINE, e.toString(), e);
}
throw new TagAttributeException(tag, src, "Invalid path : " + path);
throw new TagAttributeException(tag, src, "Invalid path : " + path, e);
} finally {
ctx.setVariableMapper(orig);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.util.logging.Logger;

import com.sun.faces.RIConstants;
import com.sun.faces.application.ApplicationAssociate;
import com.sun.faces.config.WebConfiguration;
import com.sun.faces.el.ELUtils;
import com.sun.faces.facelets.util.DevTools;
Expand Down Expand Up @@ -1219,9 +1220,8 @@ public static String getImageSource(FacesContext context, UIComponent component,
ResourceHandler handler = context.getApplication().getResourceHandler();
if (resName != null) {
String libName = (String) component.getAttributes().get("library");
WebConfiguration webConfig = WebConfiguration.getInstance();

if (libName == null && resName.startsWith(webConfig.getOptionValue(WebConfiguration.WebContextInitParameter.WebAppContractsDirectory))) {

if (libName == null && ApplicationAssociate.getInstance(context).getResourceManager().isContractsResource(resName)) {
if (context.isProjectStage(ProjectStage.Development)) {
String msg = "Illegal path, direct contract references are not allowed: " + resName;
context.addMessage(component.getClientId(context), new FacesMessage(FacesMessage.SEVERITY_ERROR, msg, msg));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import java.util.Map;
import java.util.logging.Logger;

import com.sun.faces.config.WebConfiguration;
import com.sun.faces.application.ApplicationAssociate;
import com.sun.faces.util.FacesLogger;

import jakarta.faces.application.FacesMessage;
Expand Down Expand Up @@ -194,9 +194,7 @@ public void encodeEnd(FacesContext context, UIComponent component) throws IOExce
ResponseWriter writer = context.getResponseWriter();
startExternalElement(context, writer, component);

WebConfiguration webConfig = WebConfiguration.getInstance();

if (library == null && name != null && name.startsWith(webConfig.getOptionValue(WebConfiguration.WebContextInitParameter.WebAppContractsDirectory))) {
if (library == null && name != null && ApplicationAssociate.getInstance(context).getResourceManager().isContractsResource(name)) {
if (context.isProjectStage(ProjectStage.Development)) {
String msg = "Illegal path, direct contract references are not allowed: " + name;
context.addMessage(component.getClientId(context), new FacesMessage(FacesMessage.SEVERITY_ERROR, msg, msg));
Expand Down

0 comments on commit c694635

Please sign in to comment.