This is an automated email from the ASF dual-hosted git repository. dashorst pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 506cb12906ed0aafbe443d2b1972d4fcbe2d45a6 Author: Emond Papegaaij <[email protected]> AuthorDate: Thu Oct 8 11:33:39 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 only contain letters, digits, '_', '-' and '.', components bound like that threw in their constructor. A component id may now contain '(', ')', '[' and ']' as well when MarkupSettings#setPropertyExpressionIdsAllowed(true) says so. It is off by default: with these characters a component id is safe only inside a JavaScript string literal, and whether every place it reaches is one depends on the application and its libraries as much as on Wicket, so the application decides. The exception for a rejected id names the setting; a PropertyModel with the expression remains the alternative. A markup id still may only contain letters, digits, '_', '-' and '.', 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 '(', ')', '[' and ']' with '_', and the IMarkupIdGenerator javadoc asks the same of a custom generator. Co-Authored-By: Pedro Santos <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-Authored-By: Claude Mythos 5.1 <[email protected]> --- .../apache/wicket/ComponentIdCharactersTest.java | 121 ++++++++++++++++++++- .../src/main/java/org/apache/wicket/Component.java | 58 ++++++++-- .../apache/wicket/DefaultMarkupIdGenerator.java | 5 + .../java/org/apache/wicket/IMarkupIdGenerator.java | 4 +- .../org/apache/wicket/settings/MarkupSettings.java | 36 ++++++ 5 files changed, 210 insertions(+), 14 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 7001edd2eb..e890dfb852 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,26 @@ 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.settings.MarkupSettings; 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,10 +45,16 @@ 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 b", "a&b", + 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 b", "a&b", "a=b", "a/b", "a+b", "a,b" }; + /** + * A component id may contain brackets and parentheses when + * {@link MarkupSettings#setPropertyExpressionIdsAllowed(boolean)} says so, a markup id never + */ + private static final String[] PROPERTY_EXPRESSION_IDS = { "a(b", "a)b", "a[b", "a]b" }; + @Test void componentIdMayContainLettersDigitsAndPunctuation() { @@ -52,10 +69,57 @@ class ComponentIdCharactersTest extends WicketTestCase { assertThrows(WicketRuntimeException.class, () -> new WebComponent(id), id); } + for (String id : PROPERTY_EXPRESSION_IDS) + { + assertThrows(WicketRuntimeException.class, () -> new WebComponent(id), id); + } assertThrows(WicketRuntimeException.class, () -> new WebComponent("a:b")); assertThrows(WicketRuntimeException.class, () -> new WebComponent("a~b")); } + @Test + void componentIdMayContainBracketsWhenAllowed() + { + tester.getApplication().getMarkupSettings().setPropertyExpressionIdsAllowed(true); + + for (String id : PROPERTY_EXPRESSION_IDS) + { + assertEquals(id, new WebComponent(id).getId()); + } + for (String id : INVALID_IDS) + { + assertThrows(WicketRuntimeException.class, () -> new WebComponent(id), id); + } + } + + /** + * 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() + { + tester.getApplication().getMarkupSettings().setPropertyExpressionIdsAllowed(true); + + 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 markupIdMayContainLettersDigitsAndPunctuation() { @@ -74,6 +138,12 @@ class ComponentIdCharactersTest extends WicketTestCase assertThrows(IllegalArgumentException.class, () -> component.setMarkupId(id), id); assertThrows(IllegalArgumentException.class, () -> component.setMarkupIdImpl(id), id); } + tester.getApplication().getMarkupSettings().setPropertyExpressionIdsAllowed(true); + for (String id : PROPERTY_EXPRESSION_IDS) + { + assertThrows(IllegalArgumentException.class, () -> component.setMarkupId(id), id); + assertThrows(IllegalArgumentException.class, () -> component.setMarkupIdImpl(id), id); + } } @Test @@ -99,6 +169,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 64731fc8c8..40cca3f066 100644 --- a/wicket-core/src/main/java/org/apache/wicket/Component.java +++ b/wicket-core/src/main/java/org/apache/wicket/Component.java @@ -80,6 +80,7 @@ import org.apache.wicket.request.resource.ResourceReference; import org.apache.wicket.response.StringResponse; import org.apache.wicket.settings.DebugSettings; import org.apache.wicket.settings.ExceptionSettings; +import org.apache.wicket.settings.MarkupSettings; import org.apache.wicket.util.IHierarchical; import org.apache.wicket.util.convert.IConverter; import org.apache.wicket.util.io.IClusterable; @@ -406,6 +407,21 @@ public abstract class Component */ private static final String MARKUP_ID_ATTR_NAME = "id"; + /** + * Characters a markup id may contain besides letters and digits, see + * {@link #isValidId(String, String)} + */ + private static final String VALID_ID_CHARS = "_-."; + + /** + * Characters a component id may contain besides letters and digits when + * {@link MarkupSettings#isPropertyExpressionIdsAllowed()} is true, see + * {@link #isValidId(String, String)}. Unlike a markup id, it may then 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 VALID_COMPONENT_ID_CHARS = "_-.()[]"; + /** * Meta data key for line precise error logging for the moment of addition. Made package private * for access in {@link MarkupContainer} and {@link Page} @@ -2654,7 +2670,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, VALID_ID_CHARS)) { throw new IllegalArgumentException("The markup id '" + markupId + "' may only contain letters, digits, '_', '-' and '.' chars."); @@ -4071,29 +4087,49 @@ public abstract class Component } } - if ((id != null) && !isValidId(id)) + if (id != null) { - throw new WicketRuntimeException("The component ID '" + id + - "' may only contain letters, digits, '_', '-' and '.' chars."); + if (getApplication().getMarkupSettings().isPropertyExpressionIdsAllowed()) + { + if (!isValidId(id, VALID_COMPONENT_ID_CHARS)) + { + throw new WicketRuntimeException("The component ID '" + id + + "' may only contain letters, digits, '_', '-', '.', '(', ')', '[' and ']' chars."); + } + } + else if (!isValidId(id, VALID_ID_CHARS)) + { + throw new WicketRuntimeException("The component ID '" + id + + "' may only contain letters, digits, '_', '-' and '.' chars" + + " (see MarkupSettings#setPropertyExpressionIdsAllowed for '(', ')', '[' and ']')."); + } } } /** * 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 only characters that cannot change the meaning of that JavaScript are allowed: letters, - * digits, {@code '_'}, {@code '-'} and {@code '.'}. + * JavaScript it generates, so only characters that cannot change the meaning of that JavaScript + * are allowed: letters, digits and the given characters. + * <p> + * A markup id is written into the JavaScript as it is, also outside a string literal, for + * instance as {@code Wicket.Timer.<markupId>}, so besides letters and digits it may only contain + * {@code '_'}, {@code '-'} and {@code '.'}, see {@link #VALID_ID_CHARS}. A component id is only + * written into a string literal, as part of an input name, and when + * {@link MarkupSettings#isPropertyExpressionIdsAllowed()} is true it may also contain + * {@code '('}, {@code ')'}, {@code '['} and {@code ']'}, see {@link #VALID_COMPONENT_ID_CHARS}. * * @param id * the id to check + * @param validChars + * the characters the id may contain besides letters and digits * @return {@code true} if every character of the id is allowed */ - private static boolean isValidId(final String id) + private static boolean isValidId(final String id, final String validChars) { for (int i = 0; i < id.length();) { int c = id.codePointAt(i); - if (!Character.isLetterOrDigit(c) && c != '_' && c != '-' && c != '.') + if (!Character.isLetterOrDigit(c) && validChars.indexOf(c) == -1) { return false; } @@ -4110,11 +4146,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, VALID_ID_CHARS)) { throw new WicketRuntimeException("The id attribute '" + markupId + "' in the markup of " + this + 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 553581942c..e5130d127b 100644 --- a/wicket-core/src/main/java/org/apache/wicket/IMarkupIdGenerator.java +++ b/wicket-core/src/main/java/org/apache/wicket/IMarkupIdGenerator.java @@ -28,7 +28,9 @@ public interface IMarkupIdGenerator * Wicket writes the markup id into the JavaScript it generates, without escaping it. A * generated markup id may therefore only contain letters, digits, {@code '_'}, {@code '-'} and * {@code '.'}. An id the component already has, from {@link Component#setMarkupId(String)} or - * from the id attribute in its markup, has been checked already. + * from the id attribute in its markup, has been checked already. The component id may also + * 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 diff --git a/wicket-core/src/main/java/org/apache/wicket/settings/MarkupSettings.java b/wicket-core/src/main/java/org/apache/wicket/settings/MarkupSettings.java index dd464fb6ce..ce34362d91 100644 --- a/wicket-core/src/main/java/org/apache/wicket/settings/MarkupSettings.java +++ b/wicket-core/src/main/java/org/apache/wicket/settings/MarkupSettings.java @@ -77,6 +77,12 @@ public class MarkupSettings */ private IMarkupIdGenerator markupIdGenerator = new DefaultMarkupIdGenerator(); + /** + * If true, a component id may contain '(', ')', '[' and ']' as well, so it can be a map[key] + * or name() expression for a CompoundPropertyModel. The default is false. + */ + private boolean propertyExpressionIdsAllowed = false; + /** * Construct */ @@ -300,4 +306,34 @@ public class MarkupSettings this.updateAutoLabelsTogetherWithFormComponent = updateAutoLabelsTogetherWithFormComponent; return this; } + + /** + * @return If true, a component id may contain {@code '('}, {@code ')'}, {@code '['} and + * {@code ']'} besides letters, digits, {@code '_'}, {@code '-'} and {@code '.'}. The + * default is false. + */ + public boolean isPropertyExpressionIdsAllowed() + { + return propertyExpressionIdsAllowed; + } + + /** + * Allows {@code '('}, {@code ')'}, {@code '['} and {@code ']'} in a component id, so it can be + * a {@code map[key]} or {@code name()} expression for a + * {@link org.apache.wicket.model.CompoundPropertyModel}. Off by default: a component id is + * written into input names and, by a custom {@link IMarkupIdGenerator}, into markup ids, and + * with these characters it is only safe inside a JavaScript string literal. A component can + * instead keep an id without them and take a {@link org.apache.wicket.model.PropertyModel} + * with the expression. + * + * @param propertyExpressionIdsAllowed + * If true, a component id may contain {@code '('}, {@code ')'}, {@code '['} and + * {@code ']'} + * @return {@code this} object for chaining + */ + public MarkupSettings setPropertyExpressionIdsAllowed(boolean propertyExpressionIdsAllowed) + { + this.propertyExpressionIdsAllowed = propertyExpressionIdsAllowed; + return this; + } }
