This is an automated email from the ASF dual-hosted git repository. dashorst pushed a commit to branch wicket-8.x in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 460f1bd919182cf3eb0bbb4b2ae151f37b462212 Author: Pedro Santos <[email protected]> AuthorDate: Sat Sep 19 19:47:27 2026 -0300 Escape the URL a link writes into its click handler Link and ExternalLink write their URL into JavaScript in four places: the window.location.href assignment each puts in the onclick attribute of a non-anchor element, and the quoted target each hands to PopupSettings#setTarget for the window.open call. None of them escaped it, so a URL carrying a single quote or a backslash did not hold as the string literal it was pasted into. All four now pass it through JavaScriptUtils#escapeQuotes, which escapes the backslash as well. The line this draws: a JavaScript string the framework builds itself is escaped by the code that builds it, while JavaScript the application hands over - the target of PopupSettings#setTarget, a TextTemplate variable - is written as given and stays the application's to encode. PopupSettings is therefore left alone; its target may be a quoted URL or a bare expression, and escaping it there would break the expression form. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --- .../wicket/markup/html/link/ExternalLink.java | 7 ++-- .../org/apache/wicket/markup/html/link/Link.java | 6 ++-- .../wicket/markup/html/link/PopupSettings.java | 10 ++++-- .../wicket/markup/html/link/ExternalLinkTest.java | 20 +++++++++-- .../apache/wicket/markup/html/link/LinkTest.java | 42 ++++++++++++++++------ 5 files changed, 65 insertions(+), 20 deletions(-) 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 e37799bab7..feac5c6748 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 @@ -24,6 +24,8 @@ import org.apache.wicket.request.UrlUtils; import org.apache.wicket.request.cycle.RequestCycle; import org.apache.wicket.request.flow.RedirectToUrlException; +import static org.apache.wicket.core.util.string.JavaScriptUtils.escapeQuotes; + /** * <p> * A simple anchor link (<a href="http://url">) pointing to any URL. Usually this is used for @@ -193,14 +195,15 @@ public class ExternalLink extends AbstractLink // generate a popup script by asking popup settings for one if (popupSettings != null) { - popupSettings.setTarget("'" + url + "'"); + popupSettings.setTarget("'" + escapeQuotes(url) + "'"); String popupScript = popupSettings.getPopupJavaScript(); tag.put("onclick", popupScript); } else { // or generate an onclick JS handler directly - tag.put("onclick", "window.location.href='" + url + "';return false;"); + tag.put("onclick", + "window.location.href='" + escapeQuotes(url) + "';return false;"); } } } 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 b7b303ccb7..783274dc62 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 @@ -25,6 +25,8 @@ import org.apache.wicket.markup.ComponentTag; import org.apache.wicket.model.IModel; import org.apache.wicket.request.mapper.parameter.PageParameters; +import static org.apache.wicket.core.util.string.JavaScriptUtils.escapeQuotes; + /** * Implementation of a hyperlink component. A link can be used with an anchor (<a href...) * element or any element that supports the onclick javascript event handler (such as buttons, td @@ -385,7 +387,7 @@ public abstract class Link<T> extends AbstractLink implements IRequestListener, // generate a popup script by asking popup settings for one if (popupSettings != null) { - popupSettings.setTarget("'" + url + "'"); + popupSettings.setTarget("'" + escapeQuotes(url) + "'"); String popupScript = popupSettings.getPopupJavaScript(); tag.put("onclick", popupScript); } @@ -398,7 +400,7 @@ public abstract class Link<T> extends AbstractLink implements IRequestListener, "onclick", "var win = this.ownerDocument.defaultView || this.ownerDocument.parentWindow; " + "if (win == window) { window.location.href='" + - url + + escapeQuotes(url) + "'; } ;return false"); } } 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 82dc08f82f..fe5280dc0d 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 @@ -221,9 +221,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 diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/link/ExternalLinkTest.java b/wicket-core/src/test/java/org/apache/wicket/markup/html/link/ExternalLinkTest.java index 035fbcd1bb..4ca40d6bec 100644 --- a/wicket-core/src/test/java/org/apache/wicket/markup/html/link/ExternalLinkTest.java +++ b/wicket-core/src/test/java/org/apache/wicket/markup/html/link/ExternalLinkTest.java @@ -66,13 +66,27 @@ public class ExternalLinkTest extends WicketTestCase @Test public void escapesJavascriptQuotes() throws Exception { - String unescaped = "javascript:alert('foo')"; + String uri = "files?path=d:\\it's \"home\""; MockPageWithOneComponent page = new MockPageWithOneComponent(); - page.add(new ExternalLink(COMPONENT_ID, unescaped)); + page.add(new ExternalLink(COMPONENT_ID, uri)); + + tester.startPage(page); + + assertThat(tester.getLastResponseAsString(), containsString( + "window.location.href='files?path=d:\\\\it\\'s \\"home\\"';")); + } + + @Test + public void escapesJavascriptQuotesInPopupTarget() throws Exception + { + String uri = "files?path=d:\\it's \"home\""; + MockPageWithOneComponent page = new MockPageWithOneComponent(); + page.add(new ExternalLink(COMPONENT_ID, uri).setPopupSettings(new PopupSettings())); tester.startPage(page); - assertThat(tester.getLastResponseAsString(), containsString("javascript:alert('foo')")); + assertThat(tester.getLastResponseAsString(), containsString( + "window.open('files?path=d:\\\\it\\'s \\"home\\"', ")); } /** diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/link/LinkTest.java b/wicket-core/src/test/java/org/apache/wicket/markup/html/link/LinkTest.java index 8aa0f9aab1..74289f2f51 100644 --- a/wicket-core/src/test/java/org/apache/wicket/markup/html/link/LinkTest.java +++ b/wicket-core/src/test/java/org/apache/wicket/markup/html/link/LinkTest.java @@ -44,13 +44,27 @@ public class LinkTest extends WicketTestCase @Test public void escapesJavascriptQuotesInPopupsTarget() { - String uri = "javascript:alert('foo');"; + String uri = "files?path=d:\\it's \"home\""; MockPageWithOneComponent page = new MockPageWithOneComponent(); page.add(new PopupLink(COMPONENT_ID, uri)); tester.startPage(page); - assertTrue(tester.getLastResponseAsString().contains("javascript:alert('foo');")); + assertTrue(tester.getLastResponseAsString().contains( + "window.open('files?path=d:\\\\it\\'s \\"home\\"', ")); + } + + @Test + public void escapesJavascriptQuotesInOnclickOfANonAnchor() + { + String uri = "files?path=d:\\it's \"home\""; + MockPageWithOneComponent page = new MockPageWithOneComponent(); + page.add(new UrlLink(COMPONENT_ID, uri)); + + tester.startPage(page); + + assertTrue(tester.getLastResponseAsString().contains( + "window.location.href='files?path=d:\\\\it\\'s \\"home\\"';")); } @Test(expected = MarkupException.class) @@ -72,15 +86,14 @@ public class LinkTest extends WicketTestCase tester.startPage(mockPageWithLink); } - static class PopupLink extends Link<Void> + static class UrlLink extends Link<Void> { private final String uri; - public PopupLink(String id, String uri) + public UrlLink(String id, String uri) { super(id); this.uri = uri; - setPopupSettings(new PopupSettings()); } @Override @@ -89,16 +102,25 @@ public class LinkTest extends WicketTestCase } @Override - protected void onComponentTag(ComponentTag tag) + protected CharSequence getURL() { - super.onComponentTag(tag); - tag.setName("a"); + return uri; + } + } + + static class PopupLink extends UrlLink + { + public PopupLink(String id, String uri) + { + super(id, uri); + setPopupSettings(new PopupSettings()); } @Override - protected CharSequence getURL() + protected void onComponentTag(ComponentTag tag) { - return uri; + super.onComponentTag(tag); + tag.setName("a"); } }
