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 b6afceb7767dfea571849fee9ea6229a3985f219 Author: Pedro Santos <[email protected]> AuthorDate: Sat Sep 5 02:11:51 2026 -0300 Escape the backslash in JavaScriptUtils#escapeQuotes escapeQuotes replaced each quote with a backslash and that quote but left the backslash itself untouched, so a value carrying a backslash immediately before a quote came out with two backslashes in front of it: the first escapes the second, the quote closes the JavaScript string literal, and the rest of the value is script rather than text. The method now escapes the backslash, and escapes it first, which is what escapeQuotesAndBackslash used to do under a longer name. That method is kept for compatibility, deprecated, and delegates to escapeQuotes; the call sites in Wicket move to escapeQuotes. AjaxEditableLabel#onError writes the editor's first error message into a window.status='...' literal, and a validation message can carry the submitted value through ${input}, a default message variable every form component supplies, so a message that interpolates a submitted value reaches this escaper. AjaxListPanel#append escapes a tag from the panel's own markup, which the developer writes, but is moved off the old method as well. Applications that escaped the backslash themselves around a call to escapeQuotes now escape it twice and have to drop their own replacement - the javadoc says so. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --- .../core/util/string/JavaScriptUtilsTest.java | 18 ++++- .../wicket/core/util/string/JavaScriptUtils.java | 35 +++++----- .../wicket/markup/html/link/ExternalLink.java | 4 +- .../org/apache/wicket/markup/html/link/Link.java | 4 +- .../wicket/markup/html/link/PopupSettings.java | 4 +- .../AjaxEditableLabelErrorMessageEscapeTest.java | 78 ++++++++++++++++++++++ .../markup/html/repeater/AjaxListPanelTest.java | 30 +++++++++ 7 files changed, 150 insertions(+), 23 deletions(-) diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/core/util/string/JavaScriptUtilsTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/core/util/string/JavaScriptUtilsTest.java index 07b5c4ec10..1814e65942 100644 --- a/wicket-core-tests/src/test/java/org/apache/wicket/core/util/string/JavaScriptUtilsTest.java +++ b/wicket-core-tests/src/test/java/org/apache/wicket/core/util/string/JavaScriptUtilsTest.java @@ -101,7 +101,21 @@ class JavaScriptUtilsTest } @Test - void escapeQuotesAndBackslash(){ - assertThat(JavaScriptUtils.escapeQuotesAndBackslash("alert('foo\\tbar')")).isEqualTo("alert(\\'foo\\\\tbar\\')"); + void escapeQuotes(){ + assertThat(JavaScriptUtils.escapeQuotes("alert('foo\\tbar')")).isEqualTo("alert(\\'foo\\\\tbar\\')"); + } + + /** + * A backslash immediately before a quote must not be able to escape the backslash that this + * method puts in front of that quote, which would leave the quote free to close the literal. + */ + @Test + void escapeQuotesKeepsTheValueInsideItsStringLiteral() + { + String backslashBeforeQuote = "a\\';b"; + + CharSequence escaped = JavaScriptUtils.escapeQuotes(backslashBeforeQuote); + + assertThat(escaped).isEqualTo("a\\\\\\';b"); } } 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 f3871fb316..af01bc05a7 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 @@ -56,12 +56,21 @@ public class JavaScriptUtils public static final String ATTR_INTEGRITY = "integrity"; /** - * Escape single and double quotes so that they can be part of e.g. an alert call. - * + * Escape the backslash and both quote characters, so that the value can be placed inside a + * JavaScript string literal, e.g. the argument of an alert call. + * <p> + * The backslash has to be escaped, and has to be escaped first. A value that already carries a + * backslash immediately before a quote would otherwise end up with two backslashes in front of + * that quote: the first escapes the second, the quote itself is left unescaped and closes the + * literal, and whatever the value carries after that point is no longer text but script. + * <p> + * Note: this method used to leave the backslash untouched. A caller that escaped the backslash + * itself before calling this method now escapes it twice, and has to drop its own replacement. + * <p> * Note: JSON values need to escape only the double quote, so this method wont help. - * + * * @param input - * the JavaScript which needs to be escaped + * the value which needs to be escaped * @return Escaped version of the input */ public static CharSequence escapeQuotes(final CharSequence input) @@ -69,6 +78,7 @@ public class JavaScriptUtils CharSequence s = input; if (s != null) { + s = Strings.replaceAll(s, "\\", "\\\\"); s = Strings.replaceAll(s, "'", "\\'"); s = Strings.replaceAll(s, "\"", "\\\""); } @@ -76,23 +86,18 @@ public class JavaScriptUtils } /** - * Escape single and double quotes so that they can be part of e.g. an alert call. - * - * Note: JSON values need to escape only the double quote, so this method wont help. + * Escape the backslash and both quote characters, so that the value can be placed inside a + * JavaScript string literal, e.g. the argument of an alert call. * * @param input - * the JavaScript which needs to be escaped + * the value which needs to be escaped * @return Escaped version of the input + * @deprecated use {@link #escapeQuotes(CharSequence)}, which escapes the backslash as well */ + @Deprecated(forRemoval = true) public static CharSequence escapeQuotesAndBackslash(final CharSequence input) { - CharSequence s = input; - if (s != null) - { - s = Strings.replaceAll(s, "\\", "\\\\"); - s = escapeQuotes(s); - } - return s; + return escapeQuotes(input); } /** 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 229f77d8f9..ed28d2f0fb 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 @@ -27,7 +27,7 @@ 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.escapeQuotesAndBackslash; +import static org.apache.wicket.core.util.string.JavaScriptUtils.escapeQuotes; /** * <p> @@ -210,7 +210,7 @@ public class ExternalLink extends AbstractLink response.render(OnEventHeaderItem.forComponent(this, "click", "var win = this.ownerDocument.defaultView || this.ownerDocument.parentWindow; " // + "if (win == window) { window.location.href='" // - + escapeQuotesAndBackslash(url) // + + escapeQuotes(url) // + "'; } ;return false")); 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 cf0fb445c8..077c3d8089 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 @@ -28,7 +28,7 @@ import org.apache.wicket.markup.head.OnEventHeaderItem; import org.apache.wicket.model.IModel; import org.apache.wicket.request.mapper.parameter.PageParameters; -import static org.apache.wicket.core.util.string.JavaScriptUtils.escapeQuotesAndBackslash; +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...) @@ -433,7 +433,7 @@ public abstract class Link<T> extends AbstractLink implements IRequestListener, // generated during page load. This check ensures that the click is ignored response.render(OnEventHeaderItem.forComponent(this, "click", "var win = this.ownerDocument.defaultView || this.ownerDocument.parentWindow; " - + "if (win == window) { window.location.href='" + escapeQuotesAndBackslash(url) + + "if (win == window) { window.location.href='" + 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 bb20ed711a..7483388e51 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 @@ -21,7 +21,7 @@ import org.apache.wicket.util.io.IClusterable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static org.apache.wicket.core.util.string.JavaScriptUtils.escapeQuotesAndBackslash; +import static org.apache.wicket.core.util.string.JavaScriptUtils.escapeQuotes; import static org.apache.wicket.core.util.string.JavaScriptUtils.unquoteSingleQuoted; @@ -159,7 +159,7 @@ public class PopupSettings implements IClusterable windowTitle = windowTitle.replaceAll("\\W", "_"); } - var escapedTarget = escapeQuotesAndBackslash(unquoteSingleQuoted(target)); + var escapedTarget = escapeQuotes(unquoteSingleQuoted(target)); StringBuilder script = new StringBuilder(// "var w = window.open('" + escapedTarget + "', '" // ).append(windowTitle).append("', '"); diff --git a/wicket-extensions/src/test/java/org/apache/wicket/extensions/ajax/markup/html/AjaxEditableLabelErrorMessageEscapeTest.java b/wicket-extensions/src/test/java/org/apache/wicket/extensions/ajax/markup/html/AjaxEditableLabelErrorMessageEscapeTest.java new file mode 100644 index 0000000000..0e51a05de1 --- /dev/null +++ b/wicket-extensions/src/test/java/org/apache/wicket/extensions/ajax/markup/html/AjaxEditableLabelErrorMessageEscapeTest.java @@ -0,0 +1,78 @@ +/* + * 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.extensions.ajax.markup.html; + +import org.apache.wicket.behavior.AbstractAjaxBehavior; +import org.apache.wicket.markup.html.form.Form; +import org.apache.wicket.markup.html.form.FormComponent; +import org.apache.wicket.model.Model; +import org.apache.wicket.util.tester.WicketTestCase; +import org.apache.wicket.validation.IValidatable; +import org.apache.wicket.validation.IValidator; +import org.apache.wicket.validation.ValidationError; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * {@link AjaxEditableLabel#onError(org.apache.wicket.ajax.AjaxRequestTarget)} writes the editor's + * first error message into a single quoted JavaScript string literal, and a validation message may + * carry the value the user submitted. + * + * @author Pedro Santos + */ +class AjaxEditableLabelErrorMessageEscapeTest extends WicketTestCase +{ + /** + * The backslash in front of the quote used to escape the backslash the escaper put there, + * leaving the quote free to close the window.status literal. + */ + private static final String BACKSLASH_BEFORE_QUOTE = "a\\';b"; + + @Test + void errorMessageStaysInsideItsJavaScriptStringLiteral() + { + AjaxEditableLabel<String> editableLabel = new AjaxEditableLabel<>("editableLabel", + new Model<>("initial")); + editableLabel.add(new IValidator<String>() + { + private static final long serialVersionUID = 1L; + + @Override + public void validate(IValidatable<String> validatable) + { + validatable.error( + new ValidationError().setMessage("rejected: " + validatable.getValue())); + } + }); + + tester.startComponentInPage(editableLabel); + + // "click" the label to get into edit mode + tester.executeAjaxEvent(editableLabel.get("label"), "click"); + + // submit a value the validator will reject + FormComponent<?> editor = (FormComponent<?>)editableLabel.get("editor"); + tester.getRequest().setParameter(editor.getInputName(), BACKSLASH_BEFORE_QUOTE); + tester.getRequest().setParameter("save", "true"); + tester.getRequest().setMethod(Form.METHOD_GET); + tester.executeBehavior((AbstractAjaxBehavior)editor.getBehaviorById(0)); + + String response = tester.getLastResponseAsString(); + assertTrue(response.contains("a\\\\\\';b"), "no error message was reported"); + } +} diff --git a/wicket-extensions/src/test/java/org/apache/wicket/extensions/ajax/markup/html/repeater/AjaxListPanelTest.java b/wicket-extensions/src/test/java/org/apache/wicket/extensions/ajax/markup/html/repeater/AjaxListPanelTest.java index 9b4022e0c3..a8cbabf17a 100644 --- a/wicket-extensions/src/test/java/org/apache/wicket/extensions/ajax/markup/html/repeater/AjaxListPanelTest.java +++ b/wicket-extensions/src/test/java/org/apache/wicket/extensions/ajax/markup/html/repeater/AjaxListPanelTest.java @@ -62,4 +62,34 @@ class AjaxListPanelTest extends WicketTestCase tester.executeBehavior(behavior); assertTrue(tester.getLastResponseAsString().contains("Wicket.DOM.remove(Wicket.DOM.get('id13'));")); } + + /** + * The appended markup is written into a single quoted JavaScript string literal, so a backslash + * in it must not be able to escape the backslash that precedes the following quote. + */ + @Test + void appendedMarkupStaysInsideItsJavaScriptStringLiteral() + { + final String backslashBeforeQuote = "a\\';b"; + + final AjaxListPanel list = new AjaxListPanel("list"); + + AjaxEventBehavior behavior = new AjaxEventBehavior("click") { + @Override + protected void onEvent(AjaxRequestTarget target) + { + Label label = new Label(list.newChildId()); + label.setMarkupId(backslashBeforeQuote); + list.append(label, target); + } + }; + list.add(behavior); + + tester.startComponentInPage(list); + + tester.executeBehavior(behavior); + + String response = tester.getLastResponseAsString(); + assertTrue(response.contains("a\\\\';b"), "no markup was appended"); + } }
