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

Fixed jakarta.faces.WEBAPP_CONTRACTS_DIRECTORY spec/impl mismatch #5330

Merged
merged 2 commits into from
Oct 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path.isBlank()

Copy link
Contributor Author

@BalusC BalusC Oct 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not part of fix, but indeed candidate to be improved project-wide in a new pr

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just because you were editing those files ... ;)

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path.isBlank()

Copy link
Contributor Author

@BalusC BalusC Oct 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not part of fix, but indeed candidate to be improved project-wide in a new pr

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just because you were editing those files ... ;)

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
Loading