You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by "volosied (via GitHub)" <gi...@apache.org> on 2026/08/13 03:14:02 UTC
[PR] Facelet path validation [myfaces]
volosied opened a new pull request, #1043:
URL: https://github.com/apache/myfaces/pull/1043
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [4.1.x] Facelet path validation [myfaces]
Posted by "pnicolucci (via GitHub)" <gi...@apache.org>.
pnicolucci commented on code in PR #1043:
URL: https://github.com/apache/myfaces/pull/1043#discussion_r3777490561
##########
impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java:
##########
@@ -248,34 +256,205 @@ public long getRefreshPeriod()
}
/**
- * Resolves a path based on the passed URL. If the path starts with '/', then resolve the path against
- * {@link jakarta.faces.context.ExternalContext#getResource(java.lang.String)
- * jakarta.faces.context.ExternalContext#getResource(java.lang.String)}. Otherwise create a new URL via
- * {@link URL#URL(java.net.URL, java.lang.String) URL(URL, String)}.
- *
- * @param source
- * base to resolve from
- * @param path
- * relative path to the source
+ * Resolves a path to a URL, validating scheme, traversal, and extension.
+ * Absolute paths (starting with '/') are resolved via ExternalContext;
+ * relative paths are resolved against the source URL.
+ * @param context FacesContext
+ * @param source base URL for relative resolution
+ * @param path path to resolve
* @return resolved URL
- * @throws IOException
+ * @throws IOException if path is invalid or not found
*/
public URL resolveURL(FacesContext context, URL source, String path) throws IOException
{
- if (path.startsWith("/"))
+ if (!isAllowedScheme(path))
+ {
+ throw new InvalidFileException(InvalidFileException.Reason.DISALLOWED_SCHEME,
+ "Remote or disallowed scheme in path: " + path);
+ }
+
+ URL resolved;
+ String normalizedPath;
+ boolean absoluteContextPath = path.startsWith("/");
+
+ if (absoluteContextPath)
{
+ // Absolute context-relative path via ExternalContext (scoped to WAR by container)
context.getAttributes().put(LAST_RESOURCE_RESOLVED, null);
- URL url = resolveURL(context, path);
- if (url == null)
+ resolved = resolveURL(context, path);
+ if (resolved == null)
{
throw new FileNotFoundException(path + " Not Found in ExternalContext as a Resource");
}
- return url;
+ normalizedPath = path;
}
else
{
- return new URL(source, path);
+ // Relative path resolved against source URL
+ if (source == null)
+ {
+ // Fall back to ExternalContext if no base URL available
+ resolved = resolveURL(context, path);
+ if (resolved == null)
+ {
+ throw new FileNotFoundException("Cannot resolve relative path '" + path);
+ }
+ normalizedPath = path;
+ }
+ else
+ {
+ resolved = new URL(source, path);
+ normalizedPath = resolved.getPath();
+ }
+ }
+
+ // Skip validation in UnitTest stage (uses synthetic paths)
+ if (context.isProjectStage(ProjectStage.UnitTest))
+ {
+ return resolved;
+ }
+
+ // Traversal guard: relative paths must stay within base (absolute paths already scoped by container)
+ if (!absoluteContextPath && source != null && !isWithinBase(resolved))
+ {
+ throw new InvalidFileException(InvalidFileException.Reason.PATH_TRAVERSAL,
+ "Path escapes application base: " + path);
+ }
+
+ // Extension must be a configured Facelet suffix
+ if (!mappingAllowed(context, normalizedPath))
+ {
+ throw new InvalidFileException(InvalidFileException.Reason.INVALID_EXTENSION,
+ "Invalid path provided: " + path);
+ }
+
+ return resolved;
+ }
+
+ // Path-validation helpers
+
+ private static final Set<String> BLOCKED_SCHEMES = new HashSet<>(
+ Arrays.asList("http", "https", "ftp", "ftps", "mailto", "tel",
Review Comment:
Would it be better to have an allowed scheme(s) rather than blocked?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [4.1.x] Facelet path validation [myfaces]
Posted by "tandraschko (via GitHub)" <gi...@apache.org>.
tandraschko commented on PR #1043:
URL: https://github.com/apache/myfaces/pull/1043#issuecomment-5309166948
Please provide some unittests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [4.1.x] Facelet path validation [myfaces]
Posted by "volosied (via GitHub)" <gi...@apache.org>.
volosied merged PR #1043:
URL: https://github.com/apache/myfaces/pull/1043
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [4.1.x] Facelet path validation [myfaces]
Posted by "volosied (via GitHub)" <gi...@apache.org>.
volosied commented on code in PR #1043:
URL: https://github.com/apache/myfaces/pull/1043#discussion_r3798022353
##########
impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java:
##########
@@ -248,34 +256,205 @@ public long getRefreshPeriod()
}
/**
- * Resolves a path based on the passed URL. If the path starts with '/', then resolve the path against
- * {@link jakarta.faces.context.ExternalContext#getResource(java.lang.String)
- * jakarta.faces.context.ExternalContext#getResource(java.lang.String)}. Otherwise create a new URL via
- * {@link URL#URL(java.net.URL, java.lang.String) URL(URL, String)}.
- *
- * @param source
- * base to resolve from
- * @param path
- * relative path to the source
+ * Resolves a path to a URL, validating scheme, traversal, and extension.
+ * Absolute paths (starting with '/') are resolved via ExternalContext;
+ * relative paths are resolved against the source URL.
+ * @param context FacesContext
+ * @param source base URL for relative resolution
+ * @param path path to resolve
* @return resolved URL
- * @throws IOException
+ * @throws IOException if path is invalid or not found
*/
public URL resolveURL(FacesContext context, URL source, String path) throws IOException
{
- if (path.startsWith("/"))
+ if (!isAllowedScheme(path))
+ {
+ throw new InvalidFileException(InvalidFileException.Reason.DISALLOWED_SCHEME,
+ "Remote or disallowed scheme in path: " + path);
+ }
+
+ URL resolved;
+ String normalizedPath;
+ boolean absoluteContextPath = path.startsWith("/");
+
+ if (absoluteContextPath)
{
+ // Absolute context-relative path via ExternalContext (scoped to WAR by container)
context.getAttributes().put(LAST_RESOURCE_RESOLVED, null);
- URL url = resolveURL(context, path);
- if (url == null)
+ resolved = resolveURL(context, path);
+ if (resolved == null)
{
throw new FileNotFoundException(path + " Not Found in ExternalContext as a Resource");
}
- return url;
+ normalizedPath = path;
}
else
{
- return new URL(source, path);
+ // Relative path resolved against source URL
+ if (source == null)
+ {
+ // Fall back to ExternalContext if no base URL available
+ resolved = resolveURL(context, path);
+ if (resolved == null)
+ {
+ throw new FileNotFoundException("Cannot resolve relative path '" + path);
+ }
+ normalizedPath = path;
+ }
+ else
+ {
+ resolved = new URL(source, path);
+ normalizedPath = resolved.getPath();
+ }
+ }
+
+ // Skip validation in UnitTest stage (uses synthetic paths)
+ if (context.isProjectStage(ProjectStage.UnitTest))
+ {
+ return resolved;
+ }
+
+ // Traversal guard: relative paths must stay within base (absolute paths already scoped by container)
+ if (!absoluteContextPath && source != null && !isWithinBase(resolved))
+ {
+ throw new InvalidFileException(InvalidFileException.Reason.PATH_TRAVERSAL,
+ "Path escapes application base: " + path);
+ }
+
+ // Extension must be a configured Facelet suffix
+ if (!mappingAllowed(context, normalizedPath))
+ {
+ throw new InvalidFileException(InvalidFileException.Reason.INVALID_EXTENSION,
+ "Invalid path provided: " + path);
+ }
+
+ return resolved;
+ }
+
+ // Path-validation helpers
+
+ private static final Set<String> BLOCKED_SCHEMES = new HashSet<>(
+ Arrays.asList("http", "https", "ftp", "ftps", "mailto", "tel",
Review Comment:
Fixed!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org