This is an automated email from the ASF dual-hosted git repository. benweidig pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tapestry-5.git
commit 7716142ca1b13b9eaa63ec7fff0f66c4a1117a80 Author: Ben Weidig <[email protected]> AuthorDate: Wed Oct 7 08:25:15 2026 +0200 Improving the handling of the container page query --- .../services/AjaxComponentEventRequestHandler.java | 15 +- .../services/ComponentEventLinkEncoderImpl.java | 44 ++++-- .../services/ComponentEventRequestHandlerImpl.java | 13 ++ .../services/RequestSecurityManagerImpl.java | 8 +- .../internal/services/WhitelistOnlyPageFilter.java | 95 +++++++++++ .../apache/tapestry5/modules/TapestryModule.java | 7 + .../services/WhitelistOnlyPageFilterTest.java | 174 +++++++++++++++++++++ 7 files changed, 343 insertions(+), 13 deletions(-) diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/AjaxComponentEventRequestHandler.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/AjaxComponentEventRequestHandler.java index 96ee1da8b..4c6d94bab 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/AjaxComponentEventRequestHandler.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/AjaxComponentEventRequestHandler.java @@ -1,4 +1,4 @@ -// Copyright 2007-2013 The Apache Software Foundation +// Copyright 2007-2013, 2026 The Apache Software Foundation // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -19,6 +19,7 @@ import java.io.IOException; import org.apache.tapestry5.TrackableComponentEventCallback; import org.apache.tapestry5.commons.internal.util.TapestryException; import org.apache.tapestry5.http.services.Request; +import org.apache.tapestry5.internal.EmptyEventContext; import org.apache.tapestry5.internal.InternalConstants; import org.apache.tapestry5.internal.structure.ComponentPageElement; import org.apache.tapestry5.internal.structure.Page; @@ -95,6 +96,18 @@ public class AjaxComponentEventRequestHandler implements ComponentEventRequestHa Page containerPage = cache.get(parameters.getContainingPageName()); + // When the event targets a component from a different page than the active page, the + // containing page also has code executing for this request, so its activation event + // should also be triggered. + // An empty context is used because the activation context in the request belongs to + // the active page. + if (containerPage != activePage + && pageActivator.activatePage(containerPage.getRootElement().getComponentResources(), + new EmptyEventContext(), interceptor)) + { + return; + } + ComponentPageElement element = containerPage.getComponentElementByNestedId(parameters.getNestedComponentId()); // In many cases, the triggered element is a Form that needs to be able to diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentEventLinkEncoderImpl.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentEventLinkEncoderImpl.java index 0a20ca1e9..16efb4d1b 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentEventLinkEncoderImpl.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentEventLinkEncoderImpl.java @@ -1,4 +1,4 @@ -// Copyright 2009, 2010, 2011, 2012 The Apache Software Foundation +// Copyright 2009-2012, 2026 The Apache Software Foundation // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -15,7 +15,6 @@ package org.apache.tapestry5.internal.services; import org.apache.tapestry5.*; -import org.apache.tapestry5.beanmodel.services.*; import org.apache.tapestry5.commons.util.CollectionFactory; import org.apache.tapestry5.http.Link; import org.apache.tapestry5.http.TapestryHttpSymbolConstants; @@ -376,15 +375,42 @@ public class ComponentEventLinkEncoderImpl implements ComponentEventLinkEncoder String value = request.getParameter(InternalConstants.CONTAINER_PAGE_NAME); - String containingPageName = value == null - ? activePageName - : componentClassResolver.canonicalizePageName(value); + String containingPageName; - EventContext eventContext = contextPathEncoder.decodePath(joinPath(remainingPath)); - EventContext activationContext = contextPathEncoder.decodePath(request.getParameter(InternalConstants.PAGE_CONTEXT_NAME)); + if (value == null) + { + containingPageName = activePageName; + } else + { + // The event handler method executes on a component of the containing page, so the + // containing page's code runs for this request. + // It must pass the same gates as the active page. + if (!componentClassResolver.isPageName(value)) + { + return null; + } + + containingPageName = componentClassResolver.canonicalizePageName(value); + + if (isWhitelistOnlyAndNotValid(containingPageName)) + { + return null; + } + } + + try + { + EventContext eventContext = contextPathEncoder.decodePath(joinPath(remainingPath)); + EventContext activationContext = contextPathEncoder.decodePath(request.getParameter(InternalConstants.PAGE_CONTEXT_NAME)); - return new ComponentEventRequestParameters(activePageName, containingPageName, nestedComponentId, eventType, - activationContext, eventContext); + return new ComponentEventRequestParameters(activePageName, containingPageName, nestedComponentId, eventType, + activationContext, eventContext); + } catch (IllegalArgumentException e) + { + // TAP5-2436 (parity with checkIfPage()): a malformed context is a request for a + // resource that doesn't exist (404), not an exception report. + return null; + } } public PageRenderRequestParameters decodePageRenderRequest(Request request) diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentEventRequestHandlerImpl.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentEventRequestHandlerImpl.java index 61076c64a..691c4dfd5 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentEventRequestHandlerImpl.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentEventRequestHandlerImpl.java @@ -16,6 +16,7 @@ import org.apache.tapestry5.TrackableComponentEventCallback; import org.apache.tapestry5.beanmodel.services.*; import org.apache.tapestry5.commons.internal.util.TapestryException; import org.apache.tapestry5.http.services.Response; +import org.apache.tapestry5.internal.EmptyEventContext; import org.apache.tapestry5.internal.structure.ComponentPageElement; import org.apache.tapestry5.internal.structure.Page; import org.apache.tapestry5.ioc.annotations.Primary; @@ -69,6 +70,18 @@ public class ComponentEventRequestHandlerImpl implements ComponentEventRequestHa Page containerPage = cache.get(parameters.getContainingPageName()); + // When the event targets a component from a different page than the active page, the + // containing page also has code executing for this request, and must also trigger its + // activation event. + // An empty context is used because the activation context in the request belongs to + // the active page. + if (containerPage != activePage + && pageActivator.activatePage(containerPage.getRootElement().getComponentResources(), + new EmptyEventContext(), resultProcessor)) + { + return; + } + TrackableComponentEventCallback callback = new ComponentResultProcessorWrapper(resultProcessor); environment.push(ComponentEventResultProcessor.class, resultProcessor); diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/RequestSecurityManagerImpl.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/RequestSecurityManagerImpl.java index db56e7b88..a5be05847 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/RequestSecurityManagerImpl.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/RequestSecurityManagerImpl.java @@ -1,4 +1,4 @@ -// Copyright 2008-2013 The Apache Software Foundation +// Copyright 2008-2013, 2026 The Apache Software Foundation // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -16,7 +16,6 @@ package org.apache.tapestry5.internal.services; import org.apache.tapestry5.MetaDataConstants; import org.apache.tapestry5.SymbolConstants; -import org.apache.tapestry5.beanmodel.services.*; import org.apache.tapestry5.http.Link; import org.apache.tapestry5.http.LinkSecurity; import org.apache.tapestry5.http.services.Request; @@ -54,7 +53,10 @@ public class RequestSecurityManagerImpl implements RequestSecurityManager public boolean checkForInsecureComponentEventRequest(ComponentEventRequestParameters parameters) throws IOException { - if (!needsRedirect(parameters.getActivePageName())) + // Both the active page and the containing page have code executing for this request, + // so @Secure on either must be honored. + if (!needsRedirect(parameters.getActivePageName()) + && !needsRedirect(parameters.getContainingPageName())) { return false; } diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/WhitelistOnlyPageFilter.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/WhitelistOnlyPageFilter.java new file mode 100644 index 000000000..f4fbd79d9 --- /dev/null +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/WhitelistOnlyPageFilter.java @@ -0,0 +1,95 @@ +// Licensed 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.tapestry5.internal.services; + +import org.apache.tapestry5.MetaDataConstants; +import org.apache.tapestry5.http.services.Request; +import org.apache.tapestry5.internal.InternalConstants; +import org.apache.tapestry5.services.ComponentEventRequestParameters; +import org.apache.tapestry5.services.ComponentRequestFilter; +import org.apache.tapestry5.services.ComponentRequestHandler; +import org.apache.tapestry5.services.MetaDataLocator; +import org.apache.tapestry5.services.PageRenderRequestParameters; +import org.apache.tapestry5.services.security.ClientWhitelist; + +import java.io.IOException; + +/** + * Ensures that a component event request does not reach a page annotated with + * {@link org.apache.tapestry5.annotations.WhitelistAccessOnly} from a client that is not on the + * {@linkplain ClientWhitelist whitelist}. + * + * A component event request identifies two pages: the active page (from the URL path) and the + * containing page (from the {@link InternalConstants#CONTAINER_PAGE_NAME} request parameter), whose + * component actually handles the event. Code from both pages runs for the request, so both are + * checked here. + * + * {@link ComponentEventLinkEncoderImpl} performs the same check while decoding the request, and is + * the gate that normally rejects these requests. This filter repeats it independently, so that any + * other route to the component event pipeline is covered as well. + * + * Page render requests are passed through: they are checked while decoding, and the page render + * dispatcher does not honour the "component not found" attribute used here to force a 404. + * + * @see org.apache.tapestry5.annotations.WhitelistAccessOnly + * @see MetaDataConstants#WHITELIST_ONLY_PAGE + * @since 5.9.2 + */ +public class WhitelistOnlyPageFilter implements ComponentRequestFilter +{ + private final Request request; + + private final MetaDataLocator metaDataLocator; + + private final ClientWhitelist clientWhitelist; + + public WhitelistOnlyPageFilter(Request request, MetaDataLocator metaDataLocator, ClientWhitelist clientWhitelist) + { + this.request = request; + this.metaDataLocator = metaDataLocator; + this.clientWhitelist = clientWhitelist; + } + + @Override + public void handleComponentEvent(ComponentEventRequestParameters parameters, ComponentRequestHandler handler) + throws IOException + { + if (isWhitelistOnlyAndNotValid(parameters.getActivePageName()) + || isWhitelistOnlyAndNotValid(parameters.getContainingPageName())) + { + // Same treatment as a request naming a component that does not exist: handling is + // aborted and the dispatcher turns the request into a 404, rather than reporting that + // the page exists but is protected. + + request.setAttribute(InternalConstants.REFERENCED_COMPONENT_NOT_FOUND, true); + + return; + } + + handler.handleComponentEvent(parameters); + } + + @Override + public void handlePageRender(PageRenderRequestParameters parameters, ComponentRequestHandler handler) + throws IOException + { + // Pass these through to the default handler. + handler.handlePageRender(parameters); + } + + private boolean isWhitelistOnlyAndNotValid(String canonicalizedPageName) + { + return metaDataLocator.findMeta(MetaDataConstants.WHITELIST_ONLY_PAGE, canonicalizedPageName, boolean.class) + && !clientWhitelist.isClientRequestOnWhitelist(); + } +} diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java b/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java index feaa73457..928d75ace 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java @@ -2479,6 +2479,10 @@ public final class TapestryModule /** * Contributes: * <dl> + * <dt>WhitelistOnlyPage</dt> + * <dd>{@link org.apache.tapestry5.internal.services.WhitelistOnlyPageFilter} - Aborts handling of a component + * event that targets a {@link org.apache.tapestry5.annotations.WhitelistAccessOnly} page from a client that is + * not on the whitelist</dd> * <dt>OperationTracker</dt> * <dd>Tracks general information about the request using {@link OperationTracker}</dd> * <dt>UnknownComponentFilter (production mode only)</dt> @@ -2493,6 +2497,9 @@ public final class TapestryModule */ public void contributeComponentRequestHandler(OrderedConfiguration<ComponentRequestFilter> configuration, @Symbol(TapestryHttpSymbolConstants.PRODUCTION_MODE) boolean productionMode) { + // A security gate, so it runs ahead of everything else and in development mode as well + configuration.addInstance("WhitelistOnlyPage", WhitelistOnlyPageFilter.class, "before:*"); + configuration.addInstance("OperationTracker", RequestOperationTracker.class); if (productionMode) diff --git a/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/WhitelistOnlyPageFilterTest.java b/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/WhitelistOnlyPageFilterTest.java new file mode 100644 index 000000000..53771ce06 --- /dev/null +++ b/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/WhitelistOnlyPageFilterTest.java @@ -0,0 +1,174 @@ +// Licensed 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.tapestry5.internal.services; + +import org.apache.tapestry5.EventConstants; +import org.apache.tapestry5.MetaDataConstants; +import org.apache.tapestry5.http.services.Request; +import org.apache.tapestry5.internal.EmptyEventContext; +import org.apache.tapestry5.internal.InternalConstants; +import org.apache.tapestry5.internal.test.InternalBaseTestCase; +import org.apache.tapestry5.services.ComponentEventRequestParameters; +import org.apache.tapestry5.services.ComponentRequestHandler; +import org.apache.tapestry5.services.MetaDataLocator; +import org.apache.tapestry5.services.PageRenderRequestParameters; +import org.apache.tapestry5.services.security.ClientWhitelist; +import org.testng.annotations.Test; + +/** + * The whitelist gate that normally rejects these requests lives in + * {@link ComponentEventLinkEncoderImpl}; this filter repeats it independently, so that any other + * route into the component event pipeline is covered too. + */ +public class WhitelistOnlyPageFilterTest extends InternalBaseTestCase +{ + private static final String PUBLIC_PAGE = "PublicPage"; + + private static final String PROTECTED_PAGE = "core/PageCatalog"; + + private static ComponentEventRequestParameters parameters(String activePageName, String containingPageName) + { + return new ComponentEventRequestParameters(activePageName, containingPageName, "", EventConstants.ACTION, + new EmptyEventContext(), new EmptyEventContext()); + } + + private void train_isWhitelistOnly(MetaDataLocator locator, String pageName, boolean whitelistOnly) + { + expect(locator.findMeta(MetaDataConstants.WHITELIST_ONLY_PAGE, pageName, boolean.class)) + .andReturn(whitelistOnly); + } + + @Test + public void whitelist_only_active_page_is_not_handled() throws Exception + { + Request request = mockRequest(); + MetaDataLocator locator = mockMetaDataLocator(); + ClientWhitelist whitelist = newMock(ClientWhitelist.class); + ComponentRequestHandler handler = mockComponentRequestHandler(); + + train_isWhitelistOnly(locator, PROTECTED_PAGE, true); + + expect(whitelist.isClientRequestOnWhitelist()).andReturn(false); + + // Handling is aborted; the dispatcher turns this into a 404. + + request.setAttribute(InternalConstants.REFERENCED_COMPONENT_NOT_FOUND, true); + + replay(); + + new WhitelistOnlyPageFilter(request, locator, whitelist) + .handleComponentEvent(parameters(PROTECTED_PAGE, PROTECTED_PAGE), handler); + + verify(); + } + + /** + * The exploit the encoder's check closes: a public active page in the URL path fronting a + * whitelist-only containing page named by the t:cp request parameter. + */ + @Test + public void whitelist_only_containing_page_is_not_handled() throws Exception + { + Request request = mockRequest(); + MetaDataLocator locator = mockMetaDataLocator(); + ClientWhitelist whitelist = newMock(ClientWhitelist.class); + ComponentRequestHandler handler = mockComponentRequestHandler(); + + train_isWhitelistOnly(locator, PUBLIC_PAGE, false); + + train_isWhitelistOnly(locator, PROTECTED_PAGE, true); + + expect(whitelist.isClientRequestOnWhitelist()).andReturn(false); + + request.setAttribute(InternalConstants.REFERENCED_COMPONENT_NOT_FOUND, true); + + replay(); + + new WhitelistOnlyPageFilter(request, locator, whitelist) + .handleComponentEvent(parameters(PUBLIC_PAGE, PROTECTED_PAGE), handler); + + verify(); + } + + @Test + public void whitelist_only_page_is_handled_for_a_client_on_the_whitelist() throws Exception + { + Request request = mockRequest(); + MetaDataLocator locator = mockMetaDataLocator(); + ClientWhitelist whitelist = newMock(ClientWhitelist.class); + ComponentRequestHandler handler = mockComponentRequestHandler(); + + ComponentEventRequestParameters parameters = parameters(PUBLIC_PAGE, PROTECTED_PAGE); + + train_isWhitelistOnly(locator, PUBLIC_PAGE, false); + + train_isWhitelistOnly(locator, PROTECTED_PAGE, true); + + expect(whitelist.isClientRequestOnWhitelist()).andReturn(true); + + handler.handleComponentEvent(parameters); + + replay(); + + new WhitelistOnlyPageFilter(request, locator, whitelist).handleComponentEvent(parameters, handler); + + verify(); + } + + @Test + public void unprotected_pages_are_handled() throws Exception + { + Request request = mockRequest(); + MetaDataLocator locator = mockMetaDataLocator(); + ClientWhitelist whitelist = newMock(ClientWhitelist.class); + ComponentRequestHandler handler = mockComponentRequestHandler(); + + ComponentEventRequestParameters parameters = parameters(PUBLIC_PAGE, PUBLIC_PAGE); + + train_isWhitelistOnly(locator, PUBLIC_PAGE, false); + + train_isWhitelistOnly(locator, PUBLIC_PAGE, false); + + handler.handleComponentEvent(parameters); + + replay(); + + new WhitelistOnlyPageFilter(request, locator, whitelist).handleComponentEvent(parameters, handler); + + verify(); + } + + /** + * Page render requests are gated while decoding; the page render dispatcher does not honour the + * attribute this filter sets, so they are passed straight through. + */ + @Test + public void page_render_requests_are_passed_through() throws Exception + { + Request request = mockRequest(); + MetaDataLocator locator = mockMetaDataLocator(); + ClientWhitelist whitelist = newMock(ClientWhitelist.class); + ComponentRequestHandler handler = mockComponentRequestHandler(); + + PageRenderRequestParameters parameters = new PageRenderRequestParameters(PROTECTED_PAGE, + new EmptyEventContext(), false); + + handler.handlePageRender(parameters); + + replay(); + + new WhitelistOnlyPageFilter(request, locator, whitelist).handlePageRender(parameters, handler); + + verify(); + } +}
