This is an automated email from the ASF dual-hosted git repository. reiern70 pushed a commit to branch flow-exception-while-mapping in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 09c20efa0c63673687231eae4de011d532b08f44 Author: reiern70 <[email protected]> AuthorDate: Tue Oct 20 14:08:45 2026 -0500 Follow a flow exception thrown while the request is mapped RequestCycle followed a ReplaceHandlerException (AbortWithHttpErrorCodeException, RestartResponseException, RedirectToUrlException and the like) only while a request handler executed. One thrown while the request was mapped, before there was a handler, went to the onException listeners and the exception mapper like any other exception, and ended on the error page or as a 500. That is how the 400 for an ajax request without a base url (WICKET-4841) got lost. ServletWebRequest#getClientUrl() throws it, and since WICKET-5071 BookmarkableMapper and PageInstanceMapper call getClientUrl() while they map the request, so for an ajax request it is first called during mapping and the client got a 500. The test added with WICKET-4841 only checks the exception getClientUrl() throws, so it kept passing. GitHub pull request #1673, which answers a malformed base url the same way, runs into the same problem. A flow exception thrown while the request is mapped now replaces the handler the same way as one thrown while a handler executes, including dropping the scheduled handler when the exception asks for it. An ajax request without a base url gets its 400 again. An application that throws a flow exception from a request mapper, or from anything a mapper calls, now has it followed: a RestartResponseException renders its page, a RedirectToUrlException redirects, an AbortWithHttpErrorCodeException answers its status code. Its onException listeners and its exception mapper no longer see these exceptions, and onRequestHandlerResolved is called for the replacement handler. Any other exception thrown while mapping is handled as before. --- .../request/cycle/RequestCycleListenerTest.java | 87 ++++++++++++++++++++++ .../http/servlet/ServletWebRequestTest.java | 21 ++++++ .../apache/wicket/request/cycle/RequestCycle.java | 40 ++++++---- 3 files changed, 135 insertions(+), 13 deletions(-) diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/core/request/cycle/RequestCycleListenerTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/core/request/cycle/RequestCycleListenerTest.java index 27003952da..b0ddfe4d26 100644 --- a/wicket-core-tests/src/test/java/org/apache/wicket/core/request/cycle/RequestCycleListenerTest.java +++ b/wicket-core-tests/src/test/java/org/apache/wicket/core/request/cycle/RequestCycleListenerTest.java @@ -82,6 +82,12 @@ class RequestCycleListenerTest extends RequestHandlerExecutorTest } private RequestCycle newRequestCycle(final Consumer<IRequestCycle> cycleConsumer) + { + return newRequestCycle(() -> {}, cycleConsumer); + } + + private RequestCycle newRequestCycle(final Runnable mapping, + final Consumer<IRequestCycle> cycleConsumer) { final Response originalResponse = newResponse(); Request request = new MockWebRequest(Url.parse("http://wicket.apache.org")); @@ -106,6 +112,7 @@ class RequestCycleListenerTest extends RequestHandlerExecutorTest @Override public IRequestHandler mapRequest(Request request) { + mapping.run(); return handler; } @@ -242,6 +249,86 @@ class RequestCycleListenerTest extends RequestHandlerExecutorTest assertValues(0, 2, 3); } + /** */ + @Test + void replaceExceptionDuringMapping() + { + IncrementingListener incrementingListener = new IncrementingListener(); + Application.get().getRequestCycleListeners().add(incrementingListener); + + IRequestHandler replacement = new IRequestHandler() + { + @Override + public void respond(IRequestCycle requestCycle) + { + responses++; + } + + @Override + public void detach(IRequestCycle requestCycle) + { + detaches++; + } + }; + + RequestCycle cycle = newRequestCycle(() -> { + throw new ReplaceHandlerException(replacement, true); + }, rc -> {}); + cycle.scheduleRequestHandlerAfterCurrent(new IRequestHandler() + { + @Override + public void respond(IRequestCycle requestCycle) + { + throw new UnsupportedOperationException(); + } + + @Override + public void detach(IRequestCycle requestCycle) + { + throw new UnsupportedOperationException(); + } + }); + assertTrue(cycle.processRequestAndDetach()); + incrementingListener.assertValues(1, 1, 1, 1, 0, 1); + assertValues(0, 1, 1); + + cycle = newRequestCycle(() -> { + throw new ReplaceHandlerException(replacement, false); + }, rc -> {}); + cycle.scheduleRequestHandlerAfterCurrent(new IRequestHandler() + { + @Override + public void respond(IRequestCycle requestCycle) + { + responses++; + } + + @Override + public void detach(IRequestCycle requestCycle) + { + detaches++; + } + }); + assertTrue(cycle.processRequestAndDetach()); + incrementingListener.assertValues(1, 1, 2, 2, 0, 1); + assertValues(0, 2, 2); + } + + /** */ + @Test + void exceptionDuringMappingIsMapped() + { + IncrementingListener incrementingListener = new IncrementingListener(); + Application.get().getRequestCycleListeners().add(incrementingListener); + + RequestCycle cycle = newRequestCycle(() -> { + throw new RuntimeException("testing purposes only"); + }, rc -> {}); + assertFalse(cycle.processRequestAndDetach()); + incrementingListener.assertValues(1, 1, 0, 0, 1, 1); + assertValues(1, 0, 0); + } + /** */ @Test void exceptionIsHandledByRegisteredHandler() diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/protocol/http/servlet/ServletWebRequestTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/protocol/http/servlet/ServletWebRequestTest.java index 5aa035078d..b928dd8f21 100644 --- a/wicket-core-tests/src/test/java/org/apache/wicket/protocol/http/servlet/ServletWebRequestTest.java +++ b/wicket-core-tests/src/test/java/org/apache/wicket/protocol/http/servlet/ServletWebRequestTest.java @@ -25,6 +25,8 @@ import jakarta.servlet.http.HttpServletResponse; import org.apache.wicket.MarkupContainer; import org.apache.wicket.Page; +import org.apache.wicket.ajax.markup.html.AjaxLink; +import org.apache.wicket.ajax.markup.html.ajaxLink.AjaxLinkPage; import org.apache.wicket.markup.IMarkupResourceStreamProvider; import org.apache.wicket.markup.html.WebPage; import org.apache.wicket.mock.MockApplication; @@ -169,6 +171,25 @@ class ServletWebRequestTest } } + @Test + void ajaxRequestWithoutBaseUrlAnswersBadRequest() + { + WicketTester tester = new WicketTester(); + try + { + AjaxLinkPage page = tester.startPage(AjaxLinkPage.class); + tester.getRequest().setUrl(tester.urlFor((AjaxLink<?>)page.get("ajaxLink"))); + tester.getRequest().setHeader(WebRequest.HEADER_AJAX, "true"); + tester.processRequest(); + + assertEquals(HttpServletResponse.SC_BAD_REQUEST, tester.getLastResponse().getStatus()); + } + finally + { + tester.destroy(); + } + } + /** * Tests that {@link ServletWebRequest#getClientUrl()} returns the current url + the query * string when this is not forward dispatched request. When the request is error dispatched it diff --git a/wicket-core/src/main/java/org/apache/wicket/request/cycle/RequestCycle.java b/wicket-core/src/main/java/org/apache/wicket/request/cycle/RequestCycle.java index 8df80b8f4e..548ee5d017 100644 --- a/wicket-core/src/main/java/org/apache/wicket/request/cycle/RequestCycle.java +++ b/wicket-core/src/main/java/org/apache/wicket/request/cycle/RequestCycle.java @@ -186,7 +186,8 @@ public class RequestCycle implements IRequestCycle, IEventSink, IMetadataContext } /** - * Resolves current request to a {@link IRequestHandler}. + * Resolves current request to a {@link IRequestHandler}. A {@link ReplaceHandlerException} + * thrown while resolving replaces the handler, just like one thrown while a handler executes. * * @return RequestHandler instance */ @@ -242,7 +243,15 @@ public class RequestCycle implements IRequestCycle, IEventSink, IMetadataContext set(this); listeners.onBeginRequest(this); onBeginRequest(); - IRequestHandler handler = resolveRequestHandler(); + IRequestHandler handler; + try + { + handler = resolveRequestHandler(); + } + catch (RuntimeException e) + { + handler = replacementOrRethrow(e); + } if (handler == null) { // Did not find any suitable handler, thus not executing the request @@ -298,21 +307,26 @@ public class RequestCycle implements IRequestCycle, IEventSink, IMetadataContext } catch (RuntimeException e) { - ReplaceHandlerException replacer = Exceptions.findCause(e, ReplaceHandlerException.class); + handler = replacementOrRethrow(e); + } + } + } - if (replacer == null) - { - throw e; - } + private IRequestHandler replacementOrRethrow(RuntimeException e) + { + ReplaceHandlerException replacer = Exceptions.findCause(e, ReplaceHandlerException.class); - if (replacer.getRemoveScheduled()) - { - requestHandlerExecutor.schedule(null); - } + if (replacer == null) + { + throw e; + } - handler = replacer.getReplacementRequestHandler(); - } + if (replacer.getRemoveScheduled()) + { + requestHandlerExecutor.schedule(null); } + + return replacer.getReplacementRequestHandler(); } /**
