This is an automated email from the ASF dual-hosted git repository. papegaaij pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 915dbbb15645ea54f08660ba9de72d740f3c2a0b Author: Emond Papegaaij <[email protected]> AuthorDate: Wed Aug 19 22:24:42 2026 +0200 Escape the label text wicket:label takes from a model or a bundle The resolver wrote whatever findLabelContent returned straight to replaceComponentTagBody, which writes what it is given. The class read no escape flag at all, so markup in a label was rendered as markup and an application had no way to ask for anything else. SimpleFormComponentLabel renders the same label model through getDefaultModelObjectAsString() and escapes it. Of the four sources findLabelContent had, only the tag body has to stay as it is. That body is markup this label has just rendered itself, nested components and wicket:message included, and the class javadoc offers it as the way to put markup in a label. Escaping it would encode that a second time. So the method is split: findLabelModel returns the label as text from the labeled component or from a bundle, and the tag body is handled where it is written. The flag is read from the label rather than from the labeled component. TextField and Button clear it in their constructor so that their value attribute is not encoded twice, so reading it there would leave the label as it was for exactly the components wicket:label is used with. The label is created by the resolver and never handed to the application, so its flag stays at the default. labelFromModelIsEscaped labels a TextField and covers that. Escaping happens where the label is written, so the model the FormComponent keeps for its error messages is still the value the application supplied. A bundle entry that holds an entity, say Save & close, renders differently after this. The tag body remains the way to put markup in a label deliberately. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --- .../form/AutoLabelEscapeMarkupTest$LabelPage.html | 14 +++ .../AutoLabelEscapeMarkupTest$LabelPage.properties | 2 + .../html/form/AutoLabelEscapeMarkupTest.java | 140 +++++++++++++++++++++ .../markup/html/form/AutoLabelTextResolver.java | 62 +++++---- 4 files changed, 195 insertions(+), 23 deletions(-) diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/AutoLabelEscapeMarkupTest$LabelPage.html b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/AutoLabelEscapeMarkupTest$LabelPage.html new file mode 100644 index 0000000000..8f80d7150d --- /dev/null +++ b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/AutoLabelEscapeMarkupTest$LabelPage.html @@ -0,0 +1,14 @@ +<html> +<body> + <form wicket:id="form"> + <label wicket:for="fromModel"><wicket:label/></label> + <input type="text" wicket:id="fromModel"/> + <label wicket:for="fromDefaultLabel"><wicket:label/></label> + <input type="text" wicket:id="fromDefaultLabel"/> + <label wicket:for="fromKey"><wicket:label key="markupKey"/></label> + <input type="text" wicket:id="fromKey"/> + <label wicket:for="fromBody"><wicket:label><em>emphasis</em></wicket:label></label> + <input type="text" wicket:id="fromBody"/> + </form> +</body> +</html> diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/AutoLabelEscapeMarkupTest$LabelPage.properties b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/AutoLabelEscapeMarkupTest$LabelPage.properties new file mode 100644 index 0000000000..c510bb62f2 --- /dev/null +++ b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/AutoLabelEscapeMarkupTest$LabelPage.properties @@ -0,0 +1,2 @@ +fromDefaultLabel=<script>x=1</script> +markupKey=<script>x=2</script> diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/AutoLabelEscapeMarkupTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/AutoLabelEscapeMarkupTest.java new file mode 100644 index 0000000000..bdad8bcff2 --- /dev/null +++ b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/AutoLabelEscapeMarkupTest.java @@ -0,0 +1,140 @@ +/* + * 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.form; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.apache.wicket.markup.html.WebPage; +import org.apache.wicket.model.Model; +import org.apache.wicket.util.tester.WicketTestCase; +import org.junit.jupiter.api.Test; + +/** + * Tests that the text {@literal <wicket:label>} takes from a model or from a resource bundle is + * escaped, while the markup it takes from its own tag body is not. + */ +class AutoLabelEscapeMarkupTest extends WicketTestCase +{ + /** The label the tag body is used for, which has neither a model nor a bundle entry. */ + private static final String BODY_MARKUP = "<em>emphasis</em>"; + + @SuppressWarnings({ "rawtypes", "unchecked" }) + public static class LabelPage extends WebPage + { + LabelPage(String labelFromModel) + { + Form form = new Form("form"); + add(form); + form.add(new TextField("fromModel", Model.of("")).setLabel(Model.of(labelFromModel))); + form.add(new TextField("fromDefaultLabel", Model.of(""))); + form.add(new TextField("fromKey", Model.of(""))); + form.add(new TextField("fromBody", Model.of(""))); + } + } + + /** + * The label model is the case reported: it is the same model + * {@link SimpleFormComponentLabel} renders escaped. + * <p> + * The labeled component here is a {@link TextField}, which clears escapeModelStrings in its + * constructor so that its value attribute is not encoded twice. Reading the flag from the + * labeled component instead of from the label would leave this case unescaped, so this test is + * what keeps that mistake out. + */ + @Test + void labelFromModelIsEscaped() + { + tester.startPage(new LabelPage("<script>x=0</script>")); + + String response = tester.getLastResponseAsString(); + assertTrue(response.contains("<script>x=0</script>"), + "label from the model should be escaped"); + assertFalse(response.contains("<script>x=0</script>"), + "label from the model should not reach the markup as markup"); + } + + /** The default label, looked up in the bundle by the component's id. */ + @Test + void labelFromDefaultLabelIsEscaped() + { + tester.startPage(new LabelPage("")); + + String response = tester.getLastResponseAsString(); + assertTrue(response.contains("<script>x=1</script>"), + "default label should be escaped"); + assertFalse(response.contains("<script>x=1</script>"), + "default label should not reach the markup as markup"); + } + + /** The message key on the tag, {@literal <wicket:label key="markupKey"/>}. */ + @Test + void labelFromMessageKeyIsEscaped() + { + tester.startPage(new LabelPage("")); + + String response = tester.getLastResponseAsString(); + assertTrue(response.contains("<script>x=2</script>"), + "label from a message key should be escaped"); + assertFalse(response.contains("<script>x=2</script>"), + "label from a message key should not reach the markup as markup"); + } + + /** + * The tag body is markup the label has just rendered itself, so it is written as is. This is + * the documented way to put markup in a label, and escaping it would double encode whatever + * nested components and {@literal <wicket:message>} produced. + */ + @Test + void labelFromTagBodyIsRenderedAsMarkup() + { + tester.startPage(new LabelPage("")); + + String response = tester.getLastResponseAsString(); + assertTrue(response.contains(BODY_MARKUP), + "the tag body should be rendered as markup"); + assertFalse(response.contains("<em>emphasis</em>"), + "the tag body should not be escaped"); + } + + /** + * Escaping happens where the label is written, not in the model, so the label the + * FormComponent keeps for its error messages is still the value the application supplied. + */ + @Test + void labelModelIsNotEscapedItself() + { + String label = "<script>x=0</script>"; + tester.startPage(new LabelPage(label)); + + assertEquals(label, + ((FormComponent<?>)tester.getComponentFromLastRenderedPage("form:fromModel")).getLabel() + .getObject()); + } + + /** The tag body is handed to the FormComponent as its label, as it always was. */ + @Test + void tagBodyBecomesTheComponentLabel() + { + tester.startPage(new LabelPage("")); + + assertEquals(BODY_MARKUP, + ((FormComponent<?>)tester.getComponentFromLastRenderedPage("form:fromBody")).getLabel() + .getObject()); + } +} diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/AutoLabelTextResolver.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/AutoLabelTextResolver.java index 22384bcef9..9bfc3958fc 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/AutoLabelTextResolver.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/AutoLabelTextResolver.java @@ -132,10 +132,40 @@ public class AutoLabelTextResolver implements IComponentResolver { // try and find some form of label content... - IModel<String> labelModel = findLabelContent(markupStream, openTag); - // print the label text - replaceComponentTagBody(markupStream, openTag, - labelModel != null ? labelModel.getObject() : ""); + IModel<String> labelModel = findLabelModel(openTag); + + if (labelModel != null) + { + // the label is text, coming from a model or from a resource bundle, so it has to + // be escaped before it goes into the markup. Escaping is read from this component + // and not from the labeled one: TextField and Button clear the flag in their + // constructor so that their value attribute is not encoded twice, which would + // leave the label unescaped for exactly the components <wicket:label> is used with + String text = labelModel.getObject(); + replaceComponentTagBody(markupStream, openTag, + getEscapeModelStrings() ? Strings.escapeMarkup(text) : text); + } + else + { + // as a last resort use the tag body. That body is markup this label has just + // rendered itself, nested components and <wicket:message> included, so it is + // written as is. It is also the way to put markup in a label deliberately + CharSequence body = new ResponseBufferZone(RequestCycle.get(), markupStream) + { + @Override + protected void executeInsideBufferedZone() + { + TextLabel.super.onComponentTagBody(markupStream, openTag); + } + }.execute(); + + replaceComponentTagBody(markupStream, openTag, body); + + if (!Strings.isEmpty(body)) + { + labelModel = Model.of(body.toString()); + } + } // store the label text in FormComponent's label model so its available to errors if (labelModel != null) @@ -153,8 +183,11 @@ public class AutoLabelTextResolver implements IComponentResolver } } - private IModel<String> findLabelContent(final MarkupStream markupStream, - final ComponentTag tag) + /** + * Finds the label as text, from the labeled component or from a resource bundle. Returns + * null when there is none, in which case the tag body is used instead. + */ + private IModel<String> findLabelModel(final ComponentTag tag) { if (labeled instanceof ILabelProvider) { @@ -201,23 +234,6 @@ public class AutoLabelTextResolver implements IComponentResolver } } - // as last resort use the tag body - { - String text = new ResponseBufferZone(RequestCycle.get(), markupStream) - { - @Override - protected void executeInsideBufferedZone() - { - TextLabel.super.onComponentTagBody(markupStream, tag); - } - }.execute().toString(); - - if (!Strings.isEmpty(text)) - { - return Model.of(text); - } - } - return null; } }
