This is an automated email from the ASF dual-hosted git repository. dashorst pushed a commit to branch wicket-10.x in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 63a66804e49cfb014b4ba8c05a8649b88939d2ab Author: Emond Papegaaij <[email protected]> AuthorDate: Thu Sep 24 17:30:18 2026 +0200 Re-evaluate the RENDER authorization on a freshly created page ListenerRequestHandler gates an IRequestListener on Component#canCallListener(), which is isEnabledInHierarchy() && isVisibleInHierarchy(). The visible half reads FLAG_IS_RENDER_ALLOWED, and Component#configure() is the only place that writes it, so on a page instance the request itself created that flag still holds its constructor default: neither the Component.RENDER decision of the IAuthorizationStrategy nor onConfigure() has been applied by the time the gate is consulted. The enabled half is no different: isEnabled() reads FLAG_ENABLED, which is true by default, so an enabled flag an application clears in onConfigure() is not applied either. A listener URL reaches such an instance whenever it carries no page identifier - which can be written by hand for a stateful page, and which is what Wicket itself writes for a page it considers stateless, since AbstractBookmarkableMapper#getPageInfo leaves the page id null for those. Configure the component and its parents in respond() before anything reads that state, from the page down because a parent's onConfigure() decides what its children inherit and because both halves of canCallListener() walk exactly that chain. Configuring is idempotent within a request, so a page that PageAndComponentProvider#getComponent() already configured while resolving a component added in a lifecycle callback is left alone. A full beforeRender() pass would serve as well, but calls onBeforeRender() a second time in the same request. Guard on doesProvideNewPage() rather than wasExpired(). Until 8b3e7d4220 the condition read "freshPage && (isStateless == false || component == null)" and so refused a listener on a fresh instance of any page reporting itself stateful; narrowing it to "a page id was supplied and could not be resolved" left the no-page-id case out of it. respond() already computes freshPage two lines above and used it only to decide whether a missing component is fatal. Because the component is configured first, isStateless() is accurate when canCallListenerAfterExpiry() reads it: a component that may not render pretends to be stateless, passes this guard and is refused by canCallListener() instead - the same ListenerInvocationNotAllowedException that a URL carrying the real page id produces. Stateless pages are unaffected. They serve their listeners from an instance created for the request by design, and their components report isStateless() == true, so canCallListenerAfterExpiry() keeps letting them through. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --- .../MetaDataRoleFreshPageListenerTest.java | 192 +++++++++ .../request/handler/ListenerOnFreshPageTest.java | 463 +++++++++++++++++++++ .../src/main/java/org/apache/wicket/Component.java | 8 + .../request/handler/ListenerRequestHandler.java | 64 ++- .../request/component/IRequestableComponent.java | 14 +- .../org/apache/wicket/settings/PageSettings.java | 9 +- 6 files changed, 741 insertions(+), 9 deletions(-) diff --git a/wicket-auth-roles/src/test/java/org/apache/wicket/authroles/authorization/strategies/role/metadata/MetaDataRoleFreshPageListenerTest.java b/wicket-auth-roles/src/test/java/org/apache/wicket/authroles/authorization/strategies/role/metadata/MetaDataRoleFreshPageListenerTest.java new file mode 100644 index 0000000000..0431871364 --- /dev/null +++ b/wicket-auth-roles/src/test/java/org/apache/wicket/authroles/authorization/strategies/role/metadata/MetaDataRoleFreshPageListenerTest.java @@ -0,0 +1,192 @@ +/* + * 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.wicket.authroles.authorization.strategies.role.metadata; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.Serializable; + +import org.apache.wicket.Component; +import org.apache.wicket.MarkupContainer; +import org.apache.wicket.authroles.authorization.strategies.role.IRoleCheckingStrategy; +import org.apache.wicket.authroles.authorization.strategies.role.RoleAuthorizationStrategy; +import org.apache.wicket.authroles.authorization.strategies.role.Roles; +import org.apache.wicket.core.request.handler.ListenerInvocationNotAllowedException; +import org.apache.wicket.markup.IMarkupResourceStreamProvider; +import org.apache.wicket.markup.html.WebPage; +import org.apache.wicket.markup.html.link.Link; +import org.apache.wicket.request.Url; +import org.apache.wicket.util.resource.IResourceStream; +import org.apache.wicket.util.resource.StringResourceStream; +import org.apache.wicket.util.tester.WicketTester; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * A link reserved for a role with + * {@link MetaDataRoleAuthorizationStrategy#authorize(Component, org.apache.wicket.authorization.Action, String)} + * must not be invoked for a user who does not hold that role, not even when that user asks for a + * listener URL that leaves out the page identifier and so reaches a page instance Wicket creates + * for that very request. + */ +class MetaDataRoleFreshPageListenerTest +{ + private WicketTester tester; + + @BeforeEach + void setUp() + { + tester = new WicketTester(); + TestPage.adminOnlyClicked = false; + TestPage.ordinaryClicked = false; + } + + @AfterEach + void tearDown() + { + tester.destroy(); + } + + private void signInAs(String role) + { + tester.getApplication() + .getSecuritySettings() + .setAuthorizationStrategy(new RoleAuthorizationStrategy(new UserRolesAuthorizer(role))); + } + + /** + * The listener URL of a component with the page identifier left out. + */ + private void requestWithoutPageId(String componentPath) + { + tester.getRequest() + .setUrl(Url.parse( + "wicket/bookmarkable/" + TestPage.class.getName() + "?-1.-" + componentPath)); + tester.processRequest(); + } + + @Test + void adminMayInvokeTheAdminLink() + { + signInAs("ADMIN"); + tester.startPage(TestPage.class); + tester.assertRenderedPage(TestPage.class); + assertTrue(tester.getLastResponseAsString().contains("wicket:id=\"adminOnly\"")); + + tester.clickLink("adminOnly"); + + assertTrue(TestPage.adminOnlyClicked); + } + + @Test + void userMayNotInvokeTheAdminLinkOnAFreshPage() + { + signInAs("USER"); + tester.startPage(TestPage.class); + tester.assertRenderedPage(TestPage.class); + + // the link is reserved for ADMIN, so this user never received it + assertFalse(tester.getLastResponseAsString().contains("wicket:id=\"adminOnly\"")); + + assertThrows(ListenerInvocationNotAllowedException.class, + () -> requestWithoutPageId("adminOnly")); + + assertFalse(TestPage.adminOnlyClicked, + "Link#onClick() was invoked although RENDER is reserved for the ADMIN role"); + } + + /** Control: reserving one link leaves the rest of the page alone. */ + @Test + void userMayStillInvokeTheOrdinaryLink() + { + signInAs("USER"); + tester.startPage(TestPage.class); + + tester.clickLink("ordinary"); + + assertTrue(TestPage.ordinaryClicked); + assertFalse(TestPage.adminOnlyClicked); + } + + /** A page everyone may open, carrying one link reserved for ADMIN. */ + public static class TestPage extends WebPage implements IMarkupResourceStreamProvider + { + private static final long serialVersionUID = 1L; + + // a fresh page instance is created per request, so the invocations are recorded statically + static boolean adminOnlyClicked; + static boolean ordinaryClicked; + + public TestPage() + { + Link<Void> adminOnly = new Link<Void>("adminOnly") + { + private static final long serialVersionUID = 1L; + + @Override + public void onClick() + { + adminOnlyClicked = true; + } + }; + add(adminOnly); + MetaDataRoleAuthorizationStrategy.authorize(adminOnly, Component.RENDER, "ADMIN"); + + add(new Link<Void>("ordinary") + { + private static final long serialVersionUID = 1L; + + @Override + public void onClick() + { + ordinaryClicked = true; + } + }); + } + + @Override + public IResourceStream getMarkupResourceStream(MarkupContainer container, + Class<?> containerClass) + { + return new StringResourceStream("<html><body>" // + + "<a wicket:id=\"adminOnly\">adminOnly</a>" // + + "<a wicket:id=\"ordinary\">ordinary</a>" // + + "</body></html>"); + } + } + + private static final class UserRolesAuthorizer implements IRoleCheckingStrategy, Serializable + { + private static final long serialVersionUID = 1L; + + private final Roles roles; + + UserRolesAuthorizer(String roles) + { + this.roles = new Roles(roles); + } + + @Override + public boolean hasAnyRole(Roles roles) + { + return this.roles.hasAnyRole(roles); + } + } +} diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/core/request/handler/ListenerOnFreshPageTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/core/request/handler/ListenerOnFreshPageTest.java new file mode 100644 index 0000000000..0bbc6630b9 --- /dev/null +++ b/wicket-core-tests/src/test/java/org/apache/wicket/core/request/handler/ListenerOnFreshPageTest.java @@ -0,0 +1,463 @@ +/* + * 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.wicket.core.request.handler; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.apache.wicket.Component; +import org.apache.wicket.MarkupContainer; +import org.apache.wicket.Page; +import org.apache.wicket.ajax.AjaxRequestTarget; +import org.apache.wicket.ajax.markup.html.AjaxLink; +import org.apache.wicket.authorization.Action; +import org.apache.wicket.authorization.IAuthorizationStrategy; +import org.apache.wicket.markup.IMarkupResourceStreamProvider; +import org.apache.wicket.markup.html.WebMarkupContainer; +import org.apache.wicket.markup.html.WebPage; +import org.apache.wicket.markup.html.link.Link; +import org.apache.wicket.markup.html.link.StatelessLink; +import org.apache.wicket.request.Url; +import org.apache.wicket.util.resource.IResourceStream; +import org.apache.wicket.util.resource.StringResourceStream; +import org.apache.wicket.util.tester.WicketTestCase; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * An {@link org.apache.wicket.IRequestListener} must not be invoked on a page instance that Wicket + * created for the very request that carries the listener URL. + * <p> + * Such an instance has never been through {@link Component#configure()}, so the + * {@link Component#RENDER} decision of the {@link IAuthorizationStrategy} has not been made and + * {@link Component#onConfigure()} has not run - while + * {@link org.apache.wicket.core.request.handler.ListenerRequestHandler} gates the invocation on + * {@link Component#canCallListener()}, which reads exactly those two. + * <p> + * A listener URL reaches such an instance in two ways, and this test covers both: the page + * identifier is simply left out of a URL that would otherwise carry it, or the page is one Wicket + * considers stateless, for which Wicket itself writes listener URLs without a page identifier. + */ +class ListenerOnFreshPageTest extends WicketTestCase +{ + @BeforeEach + void resetInvocations() + { + // a fresh page instance is created per request, so the pages record their invocations + // in static fields + StatefulTestPage.restrictedClicked = false; + StatefulTestPage.ordinaryClicked = false; + StatefulTestPage.hiddenClicked = false; + StatefulTestPage.disabledClicked = false; + StatefulTestPage.nestedClicked = false; + StatefulTestPage.ajaxClicked = false; + StatelessTestPage.restrictedClicked = false; + StatelessTestPage.ordinaryClicked = false; + } + + /** + * Denies {@code action} for the component with the given id, and allows everything else. + */ + private void deny(final Action action, final String componentId) + { + tester.getApplication() + .getSecuritySettings() + .setAuthorizationStrategy(new IAuthorizationStrategy.AllowAllAuthorizationStrategy() + { + @Override + public boolean isActionAuthorized(Component component, Action a) + { + return !(action.equals(a) && componentId.equals(component.getId())); + } + }); + } + + /** + * The listener URL of a component, with the page identifier left out - the shape Wicket writes + * for a stateless page, and the shape a caller can hand-craft for any page. + */ + private static String freshPageUrl(Class<? extends Page> pageClass, String componentPath) + { + return "wicket/bookmarkable/" + pageClass.getName() + "?-1.-" + componentPath; + } + + /** + * The listener URL of a component on the page instance that was last rendered. + */ + private String renderedPageUrl(String componentPath) + { + Page page = tester.getLastRenderedPage(); + return "wicket/bookmarkable/" + page.getClass().getName() + "?" + page.getPageId() + "-" + + page.getRenderCount() + ".-" + componentPath; + } + + private void request(String url) + { + tester.getRequest().setUrl(Url.parse(url)); + tester.processRequest(); + } + + /** + * A stateful page, a link the strategy denies {@link Component#RENDER} for, and a listener URL + * with the page identifier left out. + */ + @Test + void renderDeniedListenerIsNotInvokedOnAFreshPage() + { + deny(Component.RENDER, "restricted"); + tester.startPage(StatefulTestPage.class); + tester.assertRenderedPage(StatefulTestPage.class); + + // the fixture is stateful, so Wicket itself never writes a page-id-less URL for it + assertFalse(tester.getLastRenderedPage().isPageStateless()); + // and the denied link really was kept out of the markup + assertFalse(tester.getLastResponseAsString().contains("wicket:id=\"restricted\"")); + assertTrue(tester.getLastResponseAsString().contains("wicket:id=\"ordinary\"")); + + assertThrows(ListenerInvocationNotAllowedException.class, + () -> request(freshPageUrl(StatefulTestPage.class, "restricted"))); + + assertFalse(StatefulTestPage.restrictedClicked, + "Link#onClick() was invoked although IAuthorizationStrategy denied RENDER for the link"); + } + + /** + * Control: the {@link Component#ENABLE} decision is consulted on every invocation, so it holds + * on a fresh page instance as well. + */ + @Test + void enableDeniedListenerIsNotInvokedOnAFreshPage() + { + deny(Component.ENABLE, "restricted"); + tester.startPage(StatefulTestPage.class); + + assertThrows(ListenerInvocationNotAllowedException.class, + () -> request(freshPageUrl(StatefulTestPage.class, "restricted"))); + + assertFalse(StatefulTestPage.restrictedClicked); + } + + /** + * A page that reports itself stateful cannot serve a listener on an instance created for this + * request: the caller, not the application, chose the page parameters that instance built its + * state from. Wicket re-renders the new instance instead. + */ + @Test + void listenerIsNotInvokedOnAFreshInstanceOfAStatefulPage() + { + tester.startPage(StatefulTestPage.class); + assertFalse(tester.getLastRenderedPage().isPageStateless()); + + request(freshPageUrl(StatefulTestPage.class, "ordinary")); + + assertFalse(StatefulTestPage.ordinaryClicked, + "Link#onClick() was invoked on a page instance created for this very request"); + tester.assertRenderedPage(StatefulTestPage.class); + } + + /** + * Control: the same two requests against the instance that was actually rendered. The ordinary + * link works and the denied link is refused, which is the behaviour the page-id-less URL has to + * match. + */ + @Test + void listenersOnTheRenderedPageInstanceAreUnaffected() + { + deny(Component.RENDER, "restricted"); + tester.startPage(StatefulTestPage.class); + + assertThrows(ListenerInvocationNotAllowedException.class, + () -> request(renderedPageUrl("restricted"))); + assertFalse(StatefulTestPage.restrictedClicked); + + tester.startPage(StatefulTestPage.class); + request(renderedPageUrl("ordinary")); + assertTrue(StatefulTestPage.ordinaryClicked); + } + + /** + * Visibility an application sets in {@link Component#onConfigure()} is applied on a fresh page + * instance too, because onConfigure() is now run before the listener gate. + */ + @Test + void listenerHiddenInOnConfigureIsNotInvokedOnAFreshPage() + { + tester.startPage(StatefulTestPage.class); + assertFalse(tester.getLastResponseAsString().contains("wicket:id=\"hidden\"")); + + assertThrows(ListenerInvocationNotAllowedException.class, + () -> request(freshPageUrl(StatefulTestPage.class, "hidden"))); + + assertFalse(StatefulTestPage.hiddenClicked, + "Link#onClick() was invoked although onConfigure() had made the link invisible"); + } + + /** + * The enabled half of {@link Component#canCallListener()} needs the same treatment. Only + * {@link Component#isEnableAllowed()} consults the {@link IAuthorizationStrategy} on every + * call; {@link Component#isEnabled()} reads a flag that is true by default, so an enabled + * flag an application clears in {@link Component#onConfigure()} is no better protected than + * visibility is. + */ + @Test + void listenerDisabledInOnConfigureIsNotInvokedOnAFreshPage() + { + tester.startPage(StatefulTestPage.class); + // unlike an invisible link, a disabled one is rendered, so the caller has its path + assertTrue(tester.getLastResponseAsString().contains("wicket:id=\"disabled\"")); + + assertThrows(ListenerInvocationNotAllowedException.class, + () -> request(freshPageUrl(StatefulTestPage.class, "disabled"))); + + assertFalse(StatefulTestPage.disabledClicked, + "Link#onClick() was invoked although onConfigure() had disabled the link"); + } + + /** + * Both halves of {@link Component#canCallListener()} walk the parent chain, and a parent's + * {@link Component#onConfigure()} decides what its children inherit, so configuring the + * component alone is not enough: here the link itself is never restricted and it is the + * container above it that hides itself. + */ + @Test + void listenerUnderAContainerHiddenInOnConfigureIsNotInvokedOnAFreshPage() + { + tester.startPage(StatefulTestPage.class); + assertFalse(tester.getLastResponseAsString().contains("wicket:id=\"nested\"")); + + assertThrows(ListenerInvocationNotAllowedException.class, + () -> request(freshPageUrl(StatefulTestPage.class, "container:nested"))); + + assertFalse(StatefulTestPage.nestedClicked, + "Link#onClick() was invoked although onConfigure() had made its container invisible"); + } + + /** + * A behaviour is gated on {@link Component#canCallListener()} of the component it is attached + * to, so it is refused for the same reason. + */ + @Test + void behaviourOnARenderDeniedComponentIsNotInvokedOnAFreshPage() + { + deny(Component.RENDER, "ajax"); + tester.startPage(StatefulTestPage.class); + assertFalse(tester.getLastResponseAsString().contains("wicket:id=\"ajax\"")); + + assertThrows(ListenerInvocationNotAllowedException.class, () -> request( + "wicket/bookmarkable/" + StatefulTestPage.class.getName() + "?-1.0-ajax")); + + assertFalse(StatefulTestPage.ajaxClicked, + "An AJAX behaviour was invoked although IAuthorizationStrategy denied RENDER for its component"); + } + + /** + * On a stateless page Wicket writes the page-id-less listener URL itself, so this route needs + * no manipulation at all. The RENDER decision has to be made for it just the same. + */ + @Test + void renderDeniedListenerIsNotInvokedOnAStatelessPage() + { + deny(Component.RENDER, "restricted"); + tester.startPage(StatelessTestPage.class); + assertTrue(tester.getLastRenderedPage().isPageStateless()); + + assertThrows(ListenerInvocationNotAllowedException.class, + () -> request(freshPageUrl(StatelessTestPage.class, "restricted"))); + + assertFalse(StatelessTestPage.restrictedClicked, + "StatelessLink#onClick() was invoked although IAuthorizationStrategy denied RENDER for the link"); + } + + /** + * Control, and the regression this fix must not cause: a stateless page serves its listeners + * from a page instance created for the request by design, and that has to keep working. + */ + @Test + void ordinaryListenerIsStillInvokedOnAStatelessPage() + { + tester.startPage(StatelessTestPage.class); + assertTrue(tester.getLastRenderedPage().isPageStateless()); + // the URL Wicket itself wrote carries no page identifier + assertTrue(tester.getTagByWicketId("ordinary").getAttribute("href").contains("?-1.-ordinary"), + tester.getTagByWicketId("ordinary").getAttribute("href")); + + request(freshPageUrl(StatelessTestPage.class, "ordinary")); + + assertTrue(StatelessTestPage.ordinaryClicked, "StatelessLink#onClick() should be invoked"); + } + + /** A stateful page with one link the application may restrict. */ + public static class StatefulTestPage extends WebPage implements IMarkupResourceStreamProvider + { + private static final long serialVersionUID = 1L; + + static boolean restrictedClicked; + static boolean ordinaryClicked; + static boolean hiddenClicked; + static boolean disabledClicked; + static boolean nestedClicked; + static boolean ajaxClicked; + + public StatefulTestPage() + { + add(new Link<Void>("restricted") + { + private static final long serialVersionUID = 1L; + + @Override + public void onClick() + { + restrictedClicked = true; + } + }); + add(new Link<Void>("ordinary") + { + private static final long serialVersionUID = 1L; + + @Override + public void onClick() + { + ordinaryClicked = true; + } + }); + add(new Link<Void>("hidden") + { + private static final long serialVersionUID = 1L; + + @Override + protected void onConfigure() + { + super.onConfigure(); + setVisible(false); + } + + @Override + public void onClick() + { + hiddenClicked = true; + } + }); + add(new Link<Void>("disabled") + { + private static final long serialVersionUID = 1L; + + @Override + protected void onConfigure() + { + super.onConfigure(); + setEnabled(false); + } + + @Override + public void onClick() + { + disabledClicked = true; + } + }); + WebMarkupContainer container = new WebMarkupContainer("container") + { + private static final long serialVersionUID = 1L; + + @Override + protected void onConfigure() + { + super.onConfigure(); + setVisible(false); + } + }; + add(container); + container.add(new Link<Void>("nested") + { + private static final long serialVersionUID = 1L; + + @Override + public void onClick() + { + nestedClicked = true; + } + }); + add(new AjaxLink<Void>("ajax") + { + private static final long serialVersionUID = 1L; + + @Override + public void onClick(AjaxRequestTarget target) + { + ajaxClicked = true; + } + }); + } + + @Override + public IResourceStream getMarkupResourceStream(MarkupContainer container, + Class<?> containerClass) + { + return new StringResourceStream("<html><body>" // + + "<a wicket:id=\"restricted\">restricted</a>" // + + "<a wicket:id=\"ordinary\">ordinary</a>" // + + "<a wicket:id=\"hidden\">hidden</a>" // + + "<a wicket:id=\"disabled\">disabled</a>" // + + "<div wicket:id=\"container\"><a wicket:id=\"nested\">nested</a></div>" // + + "<a wicket:id=\"ajax\">ajax</a>" // + + "</body></html>"); + } + } + + /** A page Wicket considers stateless, so it writes page-id-less listener URLs for it. */ + public static class StatelessTestPage extends WebPage implements IMarkupResourceStreamProvider + { + private static final long serialVersionUID = 1L; + + static boolean restrictedClicked; + static boolean ordinaryClicked; + + public StatelessTestPage() + { + add(new StatelessLink<Void>("restricted") + { + private static final long serialVersionUID = 1L; + + @Override + public void onClick() + { + restrictedClicked = true; + } + }); + add(new StatelessLink<Void>("ordinary") + { + private static final long serialVersionUID = 1L; + + @Override + public void onClick() + { + ordinaryClicked = true; + } + }); + } + + @Override + public IResourceStream getMarkupResourceStream(MarkupContainer container, + Class<?> containerClass) + { + return new StringResourceStream("<html><body>" // + + "<a wicket:id=\"restricted\">restricted</a>" // + + "<a wicket:id=\"ordinary\">ordinary</a>" // + + "</body></html>"); + } + } +} diff --git a/wicket-core/src/main/java/org/apache/wicket/Component.java b/wicket-core/src/main/java/org/apache/wicket/Component.java index 371fbc9a70..75736dd2d1 100644 --- a/wicket-core/src/main/java/org/apache/wicket/Component.java +++ b/wicket-core/src/main/java/org/apache/wicket/Component.java @@ -4418,6 +4418,14 @@ public abstract class Component * method so that image data can be streamed. Such a component would override this method and * return {@literal true}. * </p> + * <p> + * NOTE: the visibility half of this check reads {@link #isRenderAllowed()}, which holds the + * outcome of the {@link #RENDER} authorization and is computed in {@link #configure()} alone. A + * caller that consults this method outside the render phase - on a page instance that was + * created for the current request, say - has to {@link #configure()} the component and its + * parents first, or it reads the constructor default rather than the decision the + * {@link org.apache.wicket.authorization.IAuthorizationStrategy} would make. + * </p> * * @return {@literal true} iff the listener method can be invoked on this component */ diff --git a/wicket-core/src/main/java/org/apache/wicket/core/request/handler/ListenerRequestHandler.java b/wicket-core/src/main/java/org/apache/wicket/core/request/handler/ListenerRequestHandler.java index 5279cadb56..7f0d4c74bb 100644 --- a/wicket-core/src/main/java/org/apache/wicket/core/request/handler/ListenerRequestHandler.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/request/handler/ListenerRequestHandler.java @@ -16,6 +16,9 @@ */ package org.apache.wicket.core.request.handler; +import java.util.ArrayDeque; +import java.util.Deque; + import org.apache.wicket.Component; import org.apache.wicket.IRequestListener; import org.apache.wicket.Page; @@ -167,24 +170,32 @@ public class ListenerRequestHandler + "' has been removed from page."); } + if (freshPage && component != null) + { + configureForListener((Component)component); + } + RedirectPolicy policy = page.isPageStateless() ? RedirectPolicy.NEVER_REDIRECT : RedirectPolicy.AUTO_REDIRECT; - boolean blockIfExpired = component != null && !component.canCallListenerAfterExpiry(); + boolean blockOnFreshPage = component != null && !component.canCallListenerAfterExpiry(); boolean lateComponent = component == null && freshPage; - if ((pageComponentProvider.wasExpired() && blockIfExpired) || lateComponent) + if ((freshPage && blockOnFreshPage) || lateComponent) { - // A request listener is invoked on an expired page or the component couldn't be - // determined. The best we can do is to re-paint the newly constructed page. + // A request listener is invoked on a page instance that was created for this very + // request - because the stored page had expired, or because the URL carried no page + // identifier at all - and the component does not allow that, or the component couldn't + // be determined. The best we can do is to re-paint the newly constructed page. // Reference: WICKET-4454, WICKET-6288 if (LOG.isDebugEnabled()) { LOG.debug( - "An IRequestListener was called but its page/component({}) couldn't be resolved. " + "An IRequestListener was called on a freshly created page, or its page/component({}) " + + "couldn't be resolved. " + "Scheduling re-create of the page and ignoring the listener interface...", getComponentPath()); } @@ -202,6 +213,49 @@ public class ListenerRequestHandler invokeListener(requestCycle, policy, isAjax); } + /** + * Configures a component of a page instance that was created for this very request, along with + * all of its parents. + * <p> + * Such an instance has never been rendered, and {@link Component#configure()} is the only place + * where the {@link Component#RENDER} decision of the + * {@link org.apache.wicket.authorization.IAuthorizationStrategy} is made and where + * {@link Component#onConfigure()} is called. Both are read by + * {@link Component#canCallListener()}, which decides below whether the listener may be invoked + * at all, and by {@link Component#isStateless()}, on which + * {@link Component#canCallListenerAfterExpiry()} depends - so without this a listener would be + * gated on the constructor defaults of a component the application may well have denied. + * <p> + * The component is configured from the page down, because a parent's {@code onConfigure()} + * decides what its children inherit, and because {@link Component#isVisibleInHierarchy()} and + * {@link Component#isEnabledInHierarchy()} walk exactly this chain. Configuring is idempotent + * within a request, so a page that was already configured - by + * {@link PageAndComponentProvider#getComponent()} resolving a component that is added in a + * lifecycle callback, for instance - is left alone. + * + * @param component + * the component the listener is about to be invoked on + */ + private void configureForListener(Component component) + { + Page page = component.getPage(); + if (!page.isInitialized()) + { + page.internalInitialize(); + } + + Deque<Component> hierarchy = new ArrayDeque<>(); + for (Component current = component; current != null; current = current.getParent()) + { + hierarchy.addFirst(current); + } + + for (Component current : hierarchy) + { + current.configure(); + } + } + private void invokeListener(IRequestCycle requestCycle, RedirectPolicy policy, boolean ajax) { if (getBehaviorIndex() == null) diff --git a/wicket-core/src/main/java/org/apache/wicket/request/component/IRequestableComponent.java b/wicket-core/src/main/java/org/apache/wicket/request/component/IRequestableComponent.java index 57bd59757e..c7d53e3c0d 100644 --- a/wicket-core/src/main/java/org/apache/wicket/request/component/IRequestableComponent.java +++ b/wicket-core/src/main/java/org/apache/wicket/request/component/IRequestableComponent.java @@ -114,8 +114,18 @@ public interface IRequestableComponent void detach(); /** - * @return {@code true} if it is save to call an {@link org.apache.wicket.IRequestListener} on - * this component when the owner page is freshly created after expiration + * Whether an {@link org.apache.wicket.IRequestListener} may be called on this component when + * the owner page is not the instance the client was served, but one created for the very + * request that carries the listener URL. That happens when the stored page has expired, and + * also when the URL carries no page identifier at all - which is the shape Wicket writes for a + * stateless page. + * <p> + * Such an instance has none of the state the client acted on, and the page parameters it was + * built from come from the caller, so the default is to refuse. A component that does not carry + * state of its own is safe either way. + * + * @return {@code true} if it is safe to call an {@link org.apache.wicket.IRequestListener} on + * this component when the owner page is freshly created */ boolean canCallListenerAfterExpiry(); } diff --git a/wicket-core/src/main/java/org/apache/wicket/settings/PageSettings.java b/wicket-core/src/main/java/org/apache/wicket/settings/PageSettings.java index 2bbe6d1b1c..4425f4994f 100644 --- a/wicket-core/src/main/java/org/apache/wicket/settings/PageSettings.java +++ b/wicket-core/src/main/java/org/apache/wicket/settings/PageSettings.java @@ -48,6 +48,9 @@ public class PageSettings /** * determines whether an {@link IRequestListener} can be executed * when its owning page is freshly created after expiration + * <p> + * A page is freshly created for a listener URL that carries no page identifier as well, so this + * setting governs that case too. */ private boolean callListenerAfterExpiry = false; @@ -131,7 +134,8 @@ public class PageSettings /** * @return {@code true} if Wicket should execute an {@link IRequestListener} on a component - * which owning page is freshly created after expiration of the old one + * which owning page is freshly created - after expiration of the old one, or because the + * listener URL carried no page identifier * @see #getRecreateBookmarkablePagesAfterExpiry() * @see org.apache.wicket.request.component.IRequestableComponent#canCallListenerAfterExpiry() */ @@ -142,7 +146,8 @@ public class PageSettings /** * Sets a setting that determines whether Wicket should execute the {@link IRequestListener} on a component - * which owner page is freshly created after expiration of the old one + * which owner page is freshly created after expiration of the old one, or because the listener + * URL carried no page identifier * * @param callAfterExpiry * {@code true} if Wicket should execute the listener
