From 37e91bcb26f18a4f736049701becb1ec6f2defbd Mon Sep 17 00:00:00 2001 From: Volodymyr Siedlecki Date: Tue, 18 Aug 2026 12:05:03 -0400 Subject: [PATCH 1/4] Fix path validation invalid file exception --- .../myfaces/context/InvalidFileException.java | 55 +++++ .../facelets/impl/DefaultFaceletFactory.java | 203 ++++++++++++++++-- 2 files changed, 243 insertions(+), 15 deletions(-) create mode 100644 impl/src/main/java/org/apache/myfaces/context/InvalidFileException.java diff --git a/impl/src/main/java/org/apache/myfaces/context/InvalidFileException.java b/impl/src/main/java/org/apache/myfaces/context/InvalidFileException.java new file mode 100644 index 0000000000..7a15ac53ab --- /dev/null +++ b/impl/src/main/java/org/apache/myfaces/context/InvalidFileException.java @@ -0,0 +1,55 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.myfaces.context; + +import java.io.IOException; + +/** Exception thrown when a Facelet resource path fails security or mapping validation. */ +public class InvalidFileException extends IOException +{ + private static final long serialVersionUID = 1L; + + /** Categorizes rejection reasons. */ + public enum Reason + { + DISALLOWED_SCHEME, + PATH_TRAVERSAL, + INVALID_EXTENSION + } + + private final Reason reason; + + public InvalidFileException(Reason reason, String message) + { + super(message); + this.reason = reason; + } + + public InvalidFileException(Reason reason, String message, Throwable cause) + { + super(message); + initCause(cause); + this.reason = reason; + } + + public Reason getReason() + { + return reason; + } +} diff --git a/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java b/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java index 89cb0aec79..4dbde74ae3 100644 --- a/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java +++ b/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java @@ -21,9 +21,13 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.net.URL; +import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; import java.util.regex.Pattern; @@ -31,8 +35,12 @@ import javax.el.ELException; import javax.faces.FacesException; import javax.faces.FactoryFinder; +import javax.faces.application.ProjectStage; +import javax.faces.application.ViewHandler; import javax.faces.application.ViewResource; +import javax.faces.context.ExternalContext; import javax.faces.context.FacesContext; +import org.apache.myfaces.context.InvalidFileException; import javax.faces.view.facelets.Facelet; import javax.faces.view.facelets.FaceletCache; import javax.faces.view.facelets.FaceletCacheFactory; @@ -69,6 +77,8 @@ public final class DefaultFaceletFactory extends FaceletFactory private ResourceResolver _resolver; private DefaultResourceResolver _defaultResolver; + + private volatile Set _allowedSuffixes; private FaceletCache _faceletCache; private AbstractFaceletCache _abstractFaceletCache; @@ -245,34 +255,197 @@ public long getRefreshPeriod() } /** - * Resolves a path based on the passed URL. If the path starts with '/', then resolve the path against - * {@link javax.faces.context.ExternalContext#getResource(java.lang.String) - * javax.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 ALLOWED_SCHEMES = Collections.unmodifiableSet( + new HashSet<>(Arrays.asList("file", "jar", "wsjar", "zip"))); + + /** Returns true for relative/container schemes; false for all others */ + private boolean isAllowedScheme(String path) + { + int colon = path.indexOf(':'); + + if (colon < 1) + { + return true; // relative path + } + + String scheme = path.substring(0, colon).toLowerCase(); + return ALLOWED_SCHEMES.contains(scheme); + } + + /** Verifies that resolved URL is contained within the application base. */ + private boolean isWithinBase(URL resolved) + { + URL base = getBaseUrl(); + if (base == null) + { + return true; + } + + // Compare path components (scheme-agnostic): extract path after "!" for jar URLs + String basePath = extractResourcePath(base.toExternalForm()); + String resolvedPath = extractResourcePath(resolved.toExternalForm()); + + if (!basePath.endsWith("/")) + { + basePath = basePath + "/"; + } + return resolvedPath.startsWith(basePath); + } + + /** Extract the in-archive path from jar/wsjar/file URLs (path after "!" for jar URLs). */ + private String extractResourcePath(String urlStr) + { + int jarSep = urlStr.indexOf('!'); + if (jarSep >= 0) + { + // jar: or wsjar: URL — extract path after "!" + return urlStr.substring(jarSep + 1); + } + // Regular file: URL — extract path component + int fileIdx = urlStr.indexOf("file:"); + if (fileIdx >= 0) + { + return urlStr.substring(fileIdx + 5); + } + return urlStr; + } + + /** Returns true if normalizedPath ends with a configured Facelet extension. */ + private boolean mappingAllowed(FacesContext context, String normalizedPath) + { + if (normalizedPath == null || normalizedPath.isEmpty()) + { + return false; + } + int dotIndex = normalizedPath.lastIndexOf('.'); + if (dotIndex < 0) + { + return false; + } + String ext = normalizedPath.substring(dotIndex); + + if (!getAllowedSuffixes(context).contains(ext)) + { + return false; + } + return true; + } + + /** Returns cached set of allowed Facelet suffixes built from init parameters. */ + private Set getAllowedSuffixes(FacesContext context) + { + if (_allowedSuffixes == null) + { + ExternalContext ec = context.getExternalContext(); + + String suffixParam = ec.getInitParameter(ViewHandler.FACELETS_SUFFIX_PARAM_NAME); + if (suffixParam == null) + { + suffixParam = ViewHandler.DEFAULT_FACELETS_SUFFIX; + } + Set allowed = new HashSet<>(Arrays.asList(suffixParam.trim().split("\\s+"))); + + String mappingsParam = ec.getInitParameter(ViewHandler.FACELETS_VIEW_MAPPINGS_PARAM_NAME); + if (mappingsParam == null) + { + mappingsParam = ec.getInitParameter("facelets.VIEW_MAPPINGS"); + } + if (mappingsParam != null) + { + for (String token : mappingsParam.split(";")) + { + token = token.trim(); + if (token.startsWith("*.")) + { + allowed.add(token.substring(1)); + } + } + } + + if (log.isLoggable(Level.FINE)) + { + log.fine("Allowed Facelet suffixes: " + allowed); + } + + _allowedSuffixes = allowed; } + return _allowedSuffixes; } /** From 8f8dca3a7740d8460f1ab20defc038af1bd817ea Mon Sep 17 00:00:00 2001 From: Volodymyr Siedlecki Date: Tue, 18 Aug 2026 12:39:50 -0400 Subject: [PATCH 2/4] unit test --- ...faultFaceletFactoryPathValidationTest.java | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 impl/src/test/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactoryPathValidationTest.java diff --git a/impl/src/test/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactoryPathValidationTest.java b/impl/src/test/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactoryPathValidationTest.java new file mode 100644 index 0000000000..894d8cfea3 --- /dev/null +++ b/impl/src/test/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactoryPathValidationTest.java @@ -0,0 +1,98 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.myfaces.view.facelets.impl; + +import javax.faces.application.ProjectStage; +import org.apache.myfaces.context.InvalidFileException; +import org.apache.myfaces.view.facelets.FaceletTestCase; +import org.junit.Assert; +import org.junit.Test; + +/** + * Verifies that {@link DefaultFaceletFactory#resolveURL(jakarta.faces.context.FacesContext, java.net.URL, String)} + * correctly rejects non-Facelet extensions and external URLs when running outside the + * {@code UnitTest} ProjectStage (which normally bypasses validation). + * + *

Tests switch the stage to {@code Development} before each assertion so the + * validation code-path inside {@code resolveURL} is active.

+ */ +public class DefaultFaceletFactoryPathValidationTest extends FaceletTestCase +{ + private DefaultFaceletFactory getFactory() + { + return (DefaultFaceletFactory) vdl.getFaceletFactory(); + } + + /** + * An external {@code http:} URL must be rejected with + * {@link InvalidFileException.Reason#DISALLOWED_SCHEME} even before the + * stage check, because scheme validation happens first. + */ + @Test + public void testExternalHttpUrlIsRejected() throws Exception + { + // Switch away from UnitTest so the validation block is not skipped + setProjectStage(ProjectStage.Development); + + DefaultFaceletFactory factory = getFactory(); + + try + { + factory.resolveURL(facesContext, null, "http://someverybadmaliciouswebsite.com/attack.xhtml"); + Assert.fail("Expected InvalidFileException for external http: URL"); + } + catch (InvalidFileException ex) + { + Assert.assertEquals("Rejection reason should be DISALLOWED_SCHEME", + InvalidFileException.Reason.DISALLOWED_SCHEME, ex.getReason()); + } + } + + /** + * A relative path whose extension is not a configured Facelet suffix must + * be rejected with {@link InvalidFileException.Reason#INVALID_EXTENSION}. + * + *

The default Facelet suffix is {@code .xhtml}; {@code .html} is not allowed. + * A non-null {@code source} URL is supplied so the code resolves via + * {@code new URL(source, path)} — pure string arithmetic, no I/O — and reaches + * the {@code mappingAllowed} check.

+ */ + @Test + public void testNonFaceletExtensionIsRejected() throws Exception + { + setProjectStage(ProjectStage.Development); + + DefaultFaceletFactory factory = getFactory(); + // Use the webapp context root as source so the resolved URL stays within + // the application base (bypassing PATH_TRAVERSAL) and the extension check runs. + java.net.URL base = getContext().toURL(); + java.net.URL source = new java.net.URL(base, "views/index.xhtml"); + + try + { + factory.resolveURL(facesContext, source, "template.html"); + Assert.fail("Expected InvalidFileException for a .html path"); + } + catch (InvalidFileException ex) + { + Assert.assertEquals("Rejection reason should be INVALID_EXTENSION", + InvalidFileException.Reason.INVALID_EXTENSION, ex.getReason()); + } + } +} From 6befc8c0d1ed71318d3c66c413ee55043c102d51 Mon Sep 17 00:00:00 2001 From: Volodymyr Siedlecki Date: Thu, 20 Aug 2026 13:16:34 -0400 Subject: [PATCH 3/4] Fix compilation error --- .../java/org/apache/myfaces/context/ExceptionHandlerUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impl/src/main/java/org/apache/myfaces/context/ExceptionHandlerUtils.java b/impl/src/main/java/org/apache/myfaces/context/ExceptionHandlerUtils.java index c06e16b45e..0d4df862c6 100644 --- a/impl/src/main/java/org/apache/myfaces/context/ExceptionHandlerUtils.java +++ b/impl/src/main/java/org/apache/myfaces/context/ExceptionHandlerUtils.java @@ -89,7 +89,7 @@ else if (ex instanceof LocationAware) if (component != null) { - if (!location.isBlank()) + if (!location.trim().isEmpty()) { location += ", "; } From e4b8b193ae3854488c805094e5bfc1894e7595ed Mon Sep 17 00:00:00 2001 From: Volodymyr Siedlecki Date: Thu, 20 Aug 2026 14:11:20 -0400 Subject: [PATCH 4/4] Bound the partial render/execute client-id list --- .../component/visit/PartialVisitContext.java | 14 +++- .../servlet/PartialViewContextImpl.java | 78 ++++++++++++------- .../context/ExecutePhaseClientIdsTest.java | 45 +++++++++++ .../context/RenderPhaseClientIdsTest.java | 61 +++++++++++++++ 4 files changed, 170 insertions(+), 28 deletions(-) diff --git a/impl/src/main/java/org/apache/myfaces/component/visit/PartialVisitContext.java b/impl/src/main/java/org/apache/myfaces/component/visit/PartialVisitContext.java index eba2810d99..fbbefd918b 100644 --- a/impl/src/main/java/org/apache/myfaces/component/visit/PartialVisitContext.java +++ b/impl/src/main/java/org/apache/myfaces/component/visit/PartialVisitContext.java @@ -49,6 +49,14 @@ public class PartialVisitContext extends VisitContext { + // Maximum NamingContainer nesting depth (number of separators) registered per client id. + // The number of separators in a client id equals its NamingContainer nesting depth; real views never + // nest more than a handful deep. Without a bound, a crafted client id made of many separators would make + // _addSubtreeClientId retain substring(0, i) for every separator, i.e. O(depth^2) characters and copies, + // which is an unauthenticated memory/CPU exhaustion vector (CWE-400). This keeps the work linear and acts + // as a backstop for any caller; the primary input caps live in PartialViewContextImpl. + private static final int MAX_NAMING_CONTAINER_DEPTH = 64; + // The client ids to visit private final Collection _clientIds; @@ -331,7 +339,9 @@ private void _addSubtreeClientId(String clientId) int length = clientId.length(); - for (int i = 0; i < length; i++) + // Bound the nesting depth we register to keep this method linear (see MAX_NAMING_CONTAINER_DEPTH). + int depth = 0; + for (int i = 0; i < length && depth < MAX_NAMING_CONTAINER_DEPTH; i++) { if (clientId.charAt(i) == separator) { @@ -352,6 +362,8 @@ private void _addSubtreeClientId(String clientId) // Stash away the client id c.add(clientId); + + depth++; } } } diff --git a/impl/src/main/java/org/apache/myfaces/context/servlet/PartialViewContextImpl.java b/impl/src/main/java/org/apache/myfaces/context/servlet/PartialViewContextImpl.java index 003912bcea..17a21708e3 100644 --- a/impl/src/main/java/org/apache/myfaces/context/servlet/PartialViewContextImpl.java +++ b/impl/src/main/java/org/apache/myfaces/context/servlet/PartialViewContextImpl.java @@ -23,6 +23,7 @@ import java.util.Collection; import java.util.Collections; import java.util.EnumSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -69,8 +70,16 @@ public class PartialViewContextImpl extends PartialViewContext private static final String PARTIAL_AJAX = "partial/ajax"; private static final String PARTIAL_AJAX_REQ = "javax.faces.partial.ajax"; private static final String PARTIAL_PROCESS = "partial/process"; - - private static final Set PARTIAL_EXECUTE_HINTS = Collections.unmodifiableSet( + + // Upper bounds for the attacker-controllable javax.faces.partial.render / .execute client id lists. + // A legitimate ajax request references only a handful of short client ids, so these caps never affect + // real traffic; they keep an unauthenticated caller from driving unbounded memory/CPU when the ids are + // expanded into a PartialVisitContext (CWE-400 quadratic resource exhaustion). See also the nesting-depth + // backstop in PartialVisitContext#_addSubtreeClientId. + private static final int MAX_CLIENT_IDS = 256; + private static final int MAX_CLIENT_ID_LENGTH = 256; + + private static final Set PARTIAL_EXECUTE_HINTS = Collections.unmodifiableSet( EnumSet.of(VisitHint.EXECUTE_LIFECYCLE, VisitHint.SKIP_UNRENDERED)); private static final VisitCallback RESET_VALUES_CALLBACK = new ResetValuesCallback(); @@ -206,19 +215,9 @@ public Collection getExecuteIds() if (executeMode != null && !executeMode.isEmpty() && !PartialViewContext.ALL_PARTIAL_PHASE_CLIENT_IDS.equals(executeMode)) { - - String[] clientIds - = StringUtils.splitShortString(_replaceTabOrEnterCharactersWithSpaces(executeMode), ' '); - //The collection must be mutable - List tempList = new ArrayList<>(clientIds.length); - for (String clientId : clientIds) - { - if (clientId.length() > 0) - { - tempList.add(clientId); - } - } + Collection tempList = parseClientIds(executeMode); + // The "javax.faces.source" parameter needs to be added to the list of // execute ids if missing (otherwise, we'd never execute an action associated // with, e.g., a button). @@ -230,7 +229,9 @@ public Collection getExecuteIds() { source = source.trim(); - if (!tempList.contains(source)) + // javax.faces.source is attacker-controlled as well; apply the same length bound so it + // cannot bypass the cap and be expanded into an oversized PartialVisitContext (CWE-400). + if (source.length() <= MAX_CLIENT_ID_LENGTH) { tempList.add(source); } @@ -277,6 +278,40 @@ private String _replaceTabOrEnterCharactersWithSpaces(String mode) return mode; } + /** + * Splits a space separated javax.faces.partial.render / .execute request parameter into its client ids. + *

+ * The result is a mutable, insertion-ordered, duplicate-free collection. Empty tokens are dropped, client + * ids longer than {@link #MAX_CLIENT_ID_LENGTH} are rejected and at most {@link #MAX_CLIENT_IDS} ids are + * returned. These bounds keep an unauthenticated caller from expanding this attacker-controlled parameter + * into an oversized PartialVisitContext (CWE-400); legitimate requests stay well below the limits. + */ + private Collection parseClientIds(String mode) + { + String[] clientIds = StringUtils.splitShortString(_replaceTabOrEnterCharactersWithSpaces(mode), ' '); + + // LinkedHashSet: collapse duplicate client ids once, here, instead of carrying them through the + // request, while preserving order. + Collection result = new LinkedHashSet<>(); + for (String clientId : clientIds) + { + int length = clientId.length(); + if (length == 0 || length > MAX_CLIENT_ID_LENGTH) + { + // skip empty tokens and reject implausibly long client ids + continue; + } + + result.add(clientId); + + if (result.size() >= MAX_CLIENT_IDS) + { + break; + } + } + return result; + } + @Override public Collection getRenderIds() { @@ -291,19 +326,8 @@ public Collection getRenderIds() if (renderMode != null && !renderMode.isEmpty() && !PartialViewContext.ALL_PARTIAL_PHASE_CLIENT_IDS.equals(renderMode)) { - String[] clientIds - = StringUtils.splitShortString(_replaceTabOrEnterCharactersWithSpaces(renderMode), ' '); - //The collection must be mutable - List tempList = new ArrayList<>(clientIds.length); - for (String clientId : clientIds) - { - if (clientId.length() > 0) - { - tempList.add(clientId); - } - } - _renderClientIds = tempList; + _renderClientIds = parseClientIds(renderMode); } else { diff --git a/impl/src/test/java/org/apache/myfaces/context/ExecutePhaseClientIdsTest.java b/impl/src/test/java/org/apache/myfaces/context/ExecutePhaseClientIdsTest.java index 8a7c90c8a4..520daeb4b4 100644 --- a/impl/src/test/java/org/apache/myfaces/context/ExecutePhaseClientIdsTest.java +++ b/impl/src/test/java/org/apache/myfaces/context/ExecutePhaseClientIdsTest.java @@ -26,6 +26,7 @@ import org.apache.myfaces.context.servlet.FacesContextImpl; import org.apache.myfaces.test.base.junit.AbstractJsfTestCase; import org.junit.Assert; +import org.junit.Test; /** * @@ -131,4 +132,48 @@ public void testRequestParams6() { // // Assert.assertTrue("Value match", pprContext.getExecuteIds().get(3).equals("component4")); } + + /** + * a single, implausibly long execute id must not be expanded. + */ + @Test + public void testOverlongClientIdIsRejected() { + StringBuilder colons = new StringBuilder(); + for (int i = 0; i < 100000; i++) { + colons.append(':'); + } + Map requestParamMap = new HashMap(); + requestParamMap.put(PartialViewContext.PARTIAL_EXECUTE_PARAM_NAME, colons.toString()); + ContextTestRequestWrapper wrapper = new ContextTestRequestWrapper(request, requestParamMap); + + FacesContext context = new FacesContextImpl(servletContext, wrapper, response); + + PartialViewContext pprContext = context.getPartialViewContext(); + + Assert.assertTrue(pprContext.getExecuteIds().isEmpty()); + } + + /** + * the attacker-controlled javax.faces.source parameter must be + * length-bounded too, otherwise it bypasses the execute-id cap. + */ + @Test + public void testOverlongSourceIsRejected() { + StringBuilder colons = new StringBuilder(); + for (int i = 0; i < 100000; i++) { + colons.append(':'); + } + Map requestParamMap = new HashMap(); + requestParamMap.put(PartialViewContext.PARTIAL_EXECUTE_PARAM_NAME, "form:input"); + requestParamMap.put("javax.faces.source", colons.toString()); + ContextTestRequestWrapper wrapper = new ContextTestRequestWrapper(request, requestParamMap); + + FacesContext context = new FacesContextImpl(servletContext, wrapper, response); + + PartialViewContext pprContext = context.getPartialViewContext(); + + // only the valid execute id survives; the oversized source is dropped + Assert.assertEquals(1, pprContext.getExecuteIds().size()); + Assert.assertTrue(pprContext.getExecuteIds().contains("form:input")); + } } diff --git a/impl/src/test/java/org/apache/myfaces/context/RenderPhaseClientIdsTest.java b/impl/src/test/java/org/apache/myfaces/context/RenderPhaseClientIdsTest.java index 7471b3396a..456849ba34 100644 --- a/impl/src/test/java/org/apache/myfaces/context/RenderPhaseClientIdsTest.java +++ b/impl/src/test/java/org/apache/myfaces/context/RenderPhaseClientIdsTest.java @@ -25,6 +25,7 @@ import org.apache.myfaces.context.servlet.FacesContextImpl; import org.apache.myfaces.test.base.junit.AbstractJsfTestCase; import org.junit.Assert; +import org.junit.Test; /** * Testcases for the request parameter handling @@ -133,4 +134,64 @@ public void testRequestParams6() { // // Assert.assertTrue("Value match",pprContext.getRenderIds().get(3).equals("component4")); } + + /** + * duplicate client ids must be collapsed so the parameter + * cannot be inflated with repeated ids. + */ + @Test + public void testDuplicateClientIdsAreCollapsed() { + String params = "form:input form:input form:input"; + Map requestParamMap = new HashMap(); + requestParamMap.put(PartialViewContext.PARTIAL_RENDER_PARAM_NAME, params); + ContextTestRequestWrapper wrapper = new ContextTestRequestWrapper(request, requestParamMap); + + FacesContext context = new FacesContextImpl(servletContext, wrapper, response); + + PartialViewContext pprContext = context.getPartialViewContext(); + + Assert.assertEquals(1, pprContext.getRenderIds().size()); + Assert.assertTrue(pprContext.getRenderIds().contains("form:input")); + } + + /** + * a single, implausibly long client id (e.g. a run of thousands + * of NamingContainer separators) must not be expanded into a PartialVisitContext. + */ + @Test + public void testOverlongClientIdIsRejected() { + StringBuilder colons = new StringBuilder(); + for (int i = 0; i < 100000; i++) { + colons.append(':'); + } + Map requestParamMap = new HashMap(); + requestParamMap.put(PartialViewContext.PARTIAL_RENDER_PARAM_NAME, colons.toString()); + ContextTestRequestWrapper wrapper = new ContextTestRequestWrapper(request, requestParamMap); + + FacesContext context = new FacesContextImpl(servletContext, wrapper, response); + + PartialViewContext pprContext = context.getPartialViewContext(); + + Assert.assertTrue(pprContext.getRenderIds().isEmpty()); + } + + /** + * the number of client ids read from the request is capped. + */ + @Test + public void testClientIdCountIsCapped() { + StringBuilder params = new StringBuilder(); + for (int i = 0; i < 5000; i++) { + params.append("id").append(i).append(' '); + } + Map requestParamMap = new HashMap(); + requestParamMap.put(PartialViewContext.PARTIAL_RENDER_PARAM_NAME, params.toString()); + ContextTestRequestWrapper wrapper = new ContextTestRequestWrapper(request, requestParamMap); + + FacesContext context = new FacesContextImpl(servletContext, wrapper, response); + + PartialViewContext pprContext = context.getPartialViewContext(); + + Assert.assertEquals(256, pprContext.getRenderIds().size()); + } }