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 ec5f1d19b6a1717fe87e4fd13ac05ad33995fb53 Author: Emond Papegaaij <[email protected]> AuthorDate: Tue Oct 6 16:43:15 2026 +0200 Allow brackets and parentheses in a component id A CompoundPropertyModel uses the component id as its property expression, and PropertyResolver supports map[key], list[0] and method() expressions. Since a component id may no longer contain brackets, components bound like that threw in their constructor. A component id is only written into the JavaScript inside a string literal, as part of an input name, where '(', ')', '[' and ']' cannot end it. A component id may now contain them again; a markup id still may not, because it is also written outside a string literal, as in Wicket.Timer.<markupId>. DefaultMarkupIdGenerator, which builds the markup id from the component id in development mode, replaces them with '_', and the IMarkupIdGenerator javadoc asks the same of a custom generator. Co-Authored-By: Claude Opus 5.5 <[email protected]> --- .../apache/wicket/ComponentIdCharactersTest.java | 100 ++++++++++++++++++++- .../src/main/java/org/apache/wicket/Component.java | 47 +++++++--- .../apache/wicket/DefaultMarkupIdGenerator.java | 5 ++ .../java/org/apache/wicket/IMarkupIdGenerator.java | 3 +- 4 files changed, 137 insertions(+), 18 deletions(-) diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/ComponentIdCharactersTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/ComponentIdCharactersTest.java index 611800ae8f..9b45043c3b 100644 --- a/wicket-core-tests/src/test/java/org/apache/wicket/ComponentIdCharactersTest.java +++ b/wicket-core-tests/src/test/java/org/apache/wicket/ComponentIdCharactersTest.java @@ -17,15 +17,25 @@ package org.apache.wicket; import static org.junit.jupiter.api.Assertions.assertEquals; +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 java.util.HashMap; +import java.util.Map; + import org.apache.wicket.markup.IMarkupCacheKeyProvider; import org.apache.wicket.markup.IMarkupResourceStreamProvider; import org.apache.wicket.markup.html.WebComponent; import org.apache.wicket.markup.html.WebPage; +import org.apache.wicket.markup.html.basic.Label; +import org.apache.wicket.markup.html.form.Form; +import org.apache.wicket.markup.html.form.TextField; +import org.apache.wicket.model.CompoundPropertyModel; import org.apache.wicket.util.resource.IResourceStream; import org.apache.wicket.util.resource.StringResourceStream; +import org.apache.wicket.util.tester.FormTester; import org.apache.wicket.util.tester.WicketTestCase; import org.junit.jupiter.api.Test; @@ -34,9 +44,11 @@ import org.junit.jupiter.api.Test; */ class ComponentIdCharactersTest extends WicketTestCase { - private static final String[] INVALID_IDS = { "a'b", "a\"b", "a`b", "a\\b", "a;b", "a(b", "a)b", - "a[b", "a]b", "a{b", "a}b", "a<b", "a>b", "a&b", "a\nb", "a\rb", "a\tb", "a\u0000b", - "a\u2028b", "a\u2029b" }; + private static final String[] INVALID_IDS = { "a'b", "a\"b", "a`b", "a\\b", "a;b", "a{b", "a}b", + "a<b", "a>b", "a&b", "a\nb", "a\rb", "a\tb", "a\u0000b", "a\u2028b", "a\u2029b" }; + + /** A component id may contain brackets and parentheses, a markup id may not */ + private static final String[] INVALID_MARKUP_IDS = { "a(b", "a)b", "a[b", "a]b" }; /** Characters that cannot end a JavaScript string literal are still allowed */ private static final String VALID_ID = "a b#c=d/e+f,g.h-i_j*k!l\u00fcber1"; @@ -45,6 +57,10 @@ class ComponentIdCharactersTest extends WicketTestCase void componentIdMayContainOtherCharacters() { assertEquals(VALID_ID, new WebComponent(VALID_ID).getId()); + for (String id : INVALID_MARKUP_IDS) + { + assertEquals(id, new WebComponent(id).getId()); + } } @Test @@ -58,6 +74,32 @@ class ComponentIdCharactersTest extends WicketTestCase assertThrows(WicketRuntimeException.class, () -> new WebComponent("a~b")); } + /** + * A {@link CompoundPropertyModel} uses the component id as its property expression, and + * {@code PropertyResolver} supports {@code map[key]} and {@code method()} expressions. + */ + @Test + void componentIdMayBeAPropertyExpression() + { + Bean bean = new Bean(); + bean.map.put("key", "value"); + + tester.startPage(new PropertyExpressionPage(bean)); + tester.assertLabel("form:name()", "Alice"); + tester.assertModelValue("form:map[key]", "value"); + + Component field = tester.getComponentFromLastRenderedPage("form:map[key]"); + String markupId = field.getMarkupId(); + assertTrue(markupId.startsWith("map_key_"), markupId); + assertFalse(markupId.contains("["), markupId); + + FormTester formTester = tester.newFormTester("form"); + formTester.setValue("map[key]", "changed"); + formTester.submit(); + + assertEquals("changed", bean.map.get("key")); + } + @Test void markupIdMayContainOtherCharacters() { @@ -76,6 +118,11 @@ class ComponentIdCharactersTest extends WicketTestCase assertThrows(IllegalArgumentException.class, () -> component.setMarkupId(id), id); assertThrows(IllegalArgumentException.class, () -> component.setMarkupIdImpl(id), id); } + for (String id : INVALID_MARKUP_IDS) + { + assertThrows(IllegalArgumentException.class, () -> component.setMarkupId(id), id); + assertThrows(IllegalArgumentException.class, () -> component.setMarkupIdImpl(id), id); + } } @Test @@ -101,6 +148,53 @@ class ComponentIdCharactersTest extends WicketTestCase assertTrue(false, "no cause mentions " + text + ": " + e); } + /** + * A bean bound with property expressions that are not plain property names. + */ + public static class Bean implements Serializable + { + private static final long serialVersionUID = 1L; + + private final Map<String, String> map = new HashMap<>(); + + public Map<String, String> getMap() + { + return map; + } + + public String name() + { + return "Alice"; + } + } + + /** + * A page that binds its components with a {@link CompoundPropertyModel}. + */ + public static class PropertyExpressionPage extends WebPage + implements + IMarkupResourceStreamProvider + { + private static final long serialVersionUID = 1L; + + PropertyExpressionPage(Bean bean) + { + Form<Bean> form = new Form<>("form", new CompoundPropertyModel<>(bean)); + add(form); + form.add(new Label("name()")); + form.add(new TextField<String>("map[key]").setOutputMarkupId(true)); + } + + @Override + public IResourceStream getMarkupResourceStream(MarkupContainer container, + Class<?> containerClass) + { + return new StringResourceStream("<html><body><form wicket:id=\"form\">" + + "<span wicket:id=\"name()\"></span>" + + "<input type=\"text\" wicket:id=\"map[key]\"/></form></body></html>"); + } + } + /** * A page whose markup gives its component an id attribute. */ 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 1a344e392a..3468c2eddb 100644 --- a/wicket-core/src/main/java/org/apache/wicket/Component.java +++ b/wicket-core/src/main/java/org/apache/wicket/Component.java @@ -400,12 +400,23 @@ public abstract class Component */ private static final String MARKUP_ID_ATTR_NAME = "id"; - /** Characters a component id or a markup id must not contain, see {@link #isValidId(String)} */ + /** Characters a markup id must not contain, see {@link #isValidId(String, String)} */ private static final String INVALID_ID_CHARS = "'\"`\\()[]{}<>;&"; private static final String INVALID_ID_MESSAGE = "must not contain quotes, backslashes, brackets, ';', '&' or control characters."; + /** + * Characters a component id must not contain, see {@link #isValidId(String, String)}. Unlike a + * markup id, a component id may contain {@code '('}, {@code ')'}, {@code '['} and {@code ']'}, + * so it can be a {@code map[key]} or {@code method()} expression for a + * {@link org.apache.wicket.model.CompoundPropertyModel}. + */ + private static final String INVALID_COMPONENT_ID_CHARS = "'\"`\\{}<>;&"; + + private static final String INVALID_COMPONENT_ID_MESSAGE = + "must not contain quotes, backslashes, '{', '}', '<', '>', ';', '&' or control characters."; + /** * Meta data key for line precise error logging for the moment of addition. Made package private * for access in {@link MarkupContainer} and {@link Page} @@ -2826,7 +2837,7 @@ public abstract class Component { throw new IllegalArgumentException("markupId must be String or Integer"); } - if (markupId instanceof String && !isValidId((String)markupId)) + if (markupId instanceof String && !isValidId((String)markupId, INVALID_ID_CHARS)) { throw new IllegalArgumentException( "The markup id '" + markupId + "' " + INVALID_ID_MESSAGE); @@ -4292,31 +4303,39 @@ public abstract class Component throw new WicketRuntimeException("The component ID must not contain ':' or '~' chars."); } - if ((id != null) && !isValidId(id)) + if ((id != null) && !isValidId(id, INVALID_COMPONENT_ID_CHARS)) { - throw new WicketRuntimeException("The component ID '" + id + "' " + INVALID_ID_MESSAGE); + throw new WicketRuntimeException( + "The component ID '" + id + "' " + INVALID_COMPONENT_ID_MESSAGE); } } /** * Checks the characters of a component id or a markup id. Wicket writes both into the - * JavaScript it generates, the markup id as it is and the component id as part of an input name, - * so characters that can end a JavaScript string literal or statement are not allowed: quotes, - * backslashes, brackets, {@code ';'} and control characters, line breaks included. {@code '&'} - * is not allowed either: in a script written into an attribute, such as an event handler, the - * browser decodes a character reference before the script runs, so {@code '} would end the - * literal as well. + * JavaScript it generates, so characters that can end a JavaScript string literal are not + * allowed: quotes, backslashes and control characters, line breaks included. {@code '&'} is not + * allowed either: in a script written into an attribute, such as an event handler, the browser + * decodes a character reference before the script runs, so {@code '} would end the literal + * as well. + * <p> + * A markup id is also written into the JavaScript outside a string literal, for instance as + * {@code Wicket.Timer.<markupId>}, so it must not contain brackets or {@code ';'} either, see + * {@link #INVALID_ID_CHARS}. A component id is only written into a string literal, as part of an + * input name, and may contain {@code '('}, {@code ')'}, {@code '['} and {@code ']'}, see + * {@link #INVALID_COMPONENT_ID_CHARS}. * * @param id * the id to check + * @param invalidChars + * the characters the id must not contain, besides control characters * @return {@code true} if no character of the id is disallowed */ - private static boolean isValidId(final String id) + private static boolean isValidId(final String id, final String invalidChars) { for (int i = 0; i < id.length(); i++) { char c = id.charAt(i); - if (INVALID_ID_CHARS.indexOf(c) != -1 || Character.isISOControl(c) || c == '\u2028' || + if (invalidChars.indexOf(c) != -1 || Character.isISOControl(c) || c == '\u2028' || c == '\u2029') { return false; @@ -4333,11 +4352,11 @@ public abstract class Component * @return the id attribute * @throws WicketRuntimeException * if the id attribute contains a character that is not allowed - * @see #isValidId(String) + * @see #isValidId(String, String) */ private String checkMarkupIdFromMarkup(final String markupId) { - if (!isValidId(markupId)) + if (!isValidId(markupId, INVALID_ID_CHARS)) { throw new WicketRuntimeException("The id attribute '" + markupId + "' in the markup of " + this + " " + INVALID_ID_MESSAGE); diff --git a/wicket-core/src/main/java/org/apache/wicket/DefaultMarkupIdGenerator.java b/wicket-core/src/main/java/org/apache/wicket/DefaultMarkupIdGenerator.java index 1a62943001..998d277f1c 100644 --- a/wicket-core/src/main/java/org/apache/wicket/DefaultMarkupIdGenerator.java +++ b/wicket-core/src/main/java/org/apache/wicket/DefaultMarkupIdGenerator.java @@ -81,6 +81,11 @@ public class DefaultMarkupIdGenerator implements IMarkupIdGenerator markupId = markupId.replace('.', '_'); markupId = markupId.replace('-', '_'); markupId = markupId.replace(' ', '_'); + // a component id may contain these, a markup id may not + markupId = markupId.replace('(', '_'); + markupId = markupId.replace(')', '_'); + markupId = markupId.replace('[', '_'); + markupId = markupId.replace(']', '_'); return markupId; } diff --git a/wicket-core/src/main/java/org/apache/wicket/IMarkupIdGenerator.java b/wicket-core/src/main/java/org/apache/wicket/IMarkupIdGenerator.java index 4ad2041a28..cf6caca4f6 100644 --- a/wicket-core/src/main/java/org/apache/wicket/IMarkupIdGenerator.java +++ b/wicket-core/src/main/java/org/apache/wicket/IMarkupIdGenerator.java @@ -29,7 +29,8 @@ public interface IMarkupIdGenerator * generated markup id must therefore not contain quotes, backslashes, brackets, {@code ';'}, * {@code '&'} or control characters. An id the component already has, from * {@link Component#setMarkupId(String)} or from the id attribute in its markup, has been - * checked already. + * checked already. The component id may contain {@code '('}, {@code ')'}, {@code '['} and + * {@code ']'}, so a generator that builds the markup id from it must replace these. * * @param component * The component for which to generate a markup id
