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 ed3f3e6fb00a00bedc0bde7feecf659c76e031b3 Author: Pedro Santos <[email protected]> AuthorDate: Sat Oct 3 11:29:51 2026 -0300 Move the popup target escaping from PopupSettings to the links Link and ExternalLink now escape the URL they quote into PopupSettings#setTarget, and getPopupJavaScript writes the configured target into the window.open call as given, instead of unquoting and escaping it there. JavaScriptUtils#unquoteSingleQuoted, which nothing uses any more, is deprecated. A JavaScript string the framework builds itself is escaped by the code that builds it, while JavaScript the application hands over - the target of setTarget, which may be a quoted URL or a bare expression - is written as given and stays the application's to encode. Escaping inside PopupSettings broke the expression form. Co-Authored-By: Claude Fable 5 <[email protected]> --- .../wicket/markup/html/link/PopupSettingsTest.java | 47 ++++++++++++++++++++++ .../wicket/core/util/string/JavaScriptUtils.java | 4 ++ .../wicket/markup/html/link/ExternalLink.java | 2 +- .../org/apache/wicket/markup/html/link/Link.java | 2 +- .../wicket/markup/html/link/PopupSettings.java | 17 ++++---- 5 files changed, 61 insertions(+), 11 deletions(-) diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/link/PopupSettingsTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/link/PopupSettingsTest.java new file mode 100644 index 0000000000..a17104bc20 --- /dev/null +++ b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/link/PopupSettingsTest.java @@ -0,0 +1,47 @@ +/* + * 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.markup.html.link; + +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests that {@link PopupSettings} writes the configured target into the generated JavaScript as + * given, for both of the target forms its javadoc documents. + */ +class PopupSettingsTest +{ + @Test + void writesAQuotedUrlTargetAsGiven() + { + PopupSettings settings = new PopupSettings(); + settings.setTarget("'some/url'"); + + assertThat(settings.getPopupJavaScript()).contains("window.open('some/url', "); + } + + @Test + void writesAJavaScriptExpressionTargetAsGiven() + { + PopupSettings settings = new PopupSettings(); + settings.setTarget("this.dataset['popup-url']"); + + assertThat(settings.getPopupJavaScript()) + .contains("window.open(this.dataset['popup-url'], "); + } +} diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/string/JavaScriptUtils.java b/wicket-core/src/main/java/org/apache/wicket/core/util/string/JavaScriptUtils.java index af01bc05a7..b52c489ad0 100644 --- a/wicket-core/src/main/java/org/apache/wicket/core/util/string/JavaScriptUtils.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/util/string/JavaScriptUtils.java @@ -113,7 +113,11 @@ public class JavaScriptUtils * * @param s the single-quoted string * @return the contents between the outer single quotes + * @deprecated no longer used: + * {@link org.apache.wicket.markup.html.link.PopupSettings#getPopupJavaScript()} + * writes its target as given */ + @Deprecated(forRemoval = true) public static String unquoteSingleQuoted(String s) { if (s != null) diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/link/ExternalLink.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/link/ExternalLink.java index ed28d2f0fb..e51e382e18 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/link/ExternalLink.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/link/ExternalLink.java @@ -191,7 +191,7 @@ public class ExternalLink extends AbstractLink { if (popupSettings != null) { - popupSettings.setTarget("'" + url + "'"); + popupSettings.setTarget("'" + escapeQuotes(url) + "'"); response.render(OnEventHeaderItem.forComponent(this, "click", popupSettings.getPopupJavaScript())); return; diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/link/Link.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/link/Link.java index 077c3d8089..9d8462a998 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/link/Link.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/link/Link.java @@ -416,7 +416,7 @@ public abstract class Link<T> extends AbstractLink implements IRequestListener, // next check for popup settings if (popupSettings != null) { - popupSettings.setTarget("'" + url.toString() + "'"); + popupSettings.setTarget("'" + escapeQuotes(url) + "'"); response.render(OnEventHeaderItem.forComponent(this, "click", popupSettings.getPopupJavaScript())); return; diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/link/PopupSettings.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/link/PopupSettings.java index 7483388e51..a04dac4cfb 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/link/PopupSettings.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/link/PopupSettings.java @@ -16,14 +16,10 @@ */ package org.apache.wicket.markup.html.link; -import org.apache.wicket.core.util.string.JavaScriptUtils; import org.apache.wicket.util.io.IClusterable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static org.apache.wicket.core.util.string.JavaScriptUtils.escapeQuotes; -import static org.apache.wicket.core.util.string.JavaScriptUtils.unquoteSingleQuoted; - /** * A popup specification can be used as a property of the {@link Link}classes to specify that the @@ -159,9 +155,8 @@ public class PopupSettings implements IClusterable windowTitle = windowTitle.replaceAll("\\W", "_"); } - var escapedTarget = escapeQuotes(unquoteSingleQuoted(target)); StringBuilder script = new StringBuilder(// - "var w = window.open('" + escapedTarget + "', '" // + "var w = window.open(" + target + ", '" // ).append(windowTitle).append("', '"); script.append("scrollbars=").append(flagToString(SCROLLBARS)); @@ -227,9 +222,13 @@ public class PopupSettings implements IClusterable * the anchor element, but clients may want to override this (e.g. when the HTML element is not an anchor) by * setting the target explicitly. * - * <strong>Note</strong>: if the target is an url (relative or absolute) then it should be wrapped in - * quotes, for example: <code>setTarget("'some/url'")</code>. If the url is delivered with an HTML attribute then - * it should be without quotes, for example: <code>setTarget("this.dataset['popup-url']")</code> with markup like: + * <strong>Note</strong>: the target is written into the onclick handler as JavaScript, as given. + * If the target is an url (relative or absolute) then it should be wrapped in quotes, for example: + * <code>setTarget("'some/url'")</code>, and an url that is not a constant has to be escaped for + * a JavaScript string first, as {@link Link} does: + * <code>setTarget("'" + JavaScriptUtils.escapeQuotes(url) + "'")</code>. If the url is delivered + * with an HTML attribute then it should be without quotes, for example: + * <code>setTarget("this.dataset['popup-url']")</code> with markup like: * <pre><code><a data-popup-url="some/url">Link</a></code></pre>. * * @param target
