You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by pa...@apache.org on 2026/08/21 15:15:53 UTC
(wicket) branch wicket-8.x updated (79bb80948e -> 6d6930c0ec)
This is an automated email from the ASF dual-hosted git repository.
papegaaij pushed a change to branch wicket-8.x
in repository https://gitbox.apache.org/repos/asf/wicket.git
from 79bb80948e WICKET-7194 Deprecate the Include component for security reasons
new f4ff8c8abc Escape the model string a Button writes into a button element body
new b055a7bd38 Escape the label text wicket:label takes from a model or a bundle
new 7f53ba4dff Escape the attribute values of a MetaDataHeaderItem as markup
new 8c1eff3d42 Escape the additional option attributes of a palette
new 28b50bbd21 Escape the body of the default option of a single select choice
new 0d566fb2a9 Say in the javadoc which methods have their value written to the markup as is
new 629b79fbcb Document how model data, markup and message bundles are escaped
new 6d6930c0ec Escape the value an editable label shows for an empty model
The 8 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
SECURITY.md | 49 ++++++
.../wicket/markup/head/MetaDataHeaderItem.java | 2 +-
.../wicket/markup/html/form/AbstractChoice.java | 28 +++
.../html/form/AbstractSingleSelectChoice.java | 21 ++-
.../markup/html/form/AutoLabelTextResolver.java | 62 ++++---
.../org/apache/wicket/markup/html/form/Button.java | 12 +-
.../markup/html/form/CheckBoxMultipleChoice.java | 36 ++++
.../wicket/markup/html/form/RadioChoice.java | 36 ++++
.../html/navigation/paging/PagingNavigation.java | 12 ++
.../wicket/markup/transformer/ITransformer.java | 6 +
.../markup/head/HtmlImportHeaderItemTest.java | 7 +-
.../wicket/markup/head/MetaDataHeaderItemTest.java | 35 +++-
.../form/AbstractSingleSelectChoiceEscapeTest.java | 192 +++++++++++++++++++++
.../form/AutoLabelEscapeMarkupTest$LabelPage.html | 14 ++
.../AutoLabelEscapeMarkupTest$LabelPage.properties | 2 +
.../html/form/AutoLabelEscapeMarkupTest.java | 140 +++++++++++++++
.../apache/wicket/markup/html/form/ButtonTest.java | 33 +++-
.../ajax/markup/html/AjaxEditableChoiceLabel.java | 4 +-
.../ajax/markup/html/AjaxEditableLabel.java | 5 +-
.../markup/html/AjaxEditableMultiLineLabel.java | 5 +-
.../autocomplete/AbstractAutoCompleteRenderer.java | 6 +
.../html/autocomplete/IAutoCompleteRenderer.java | 18 ++
.../extensions/breadcrumb/BreadCrumbBar.java | 6 +
.../form/palette/component/AbstractOptions.java | 4 +-
.../html/AjaxEditableLabelNullLabelEscapeTest.java | 154 +++++++++++++++++
...tteAdditionalAttributesTest$AttributesPage.html | 2 +-
.../palette/PaletteAdditionalAttributesTest.java | 113 ++++++++++++
.../wicket/velocity/VelocityContributor.java | 6 +
.../wicket/velocity/markup/html/VelocityPanel.java | 6 +
29 files changed, 976 insertions(+), 40 deletions(-)
create mode 100644 wicket-core/src/test/java/org/apache/wicket/markup/html/form/AbstractSingleSelectChoiceEscapeTest.java
create mode 100644 wicket-core/src/test/java/org/apache/wicket/markup/html/form/AutoLabelEscapeMarkupTest$LabelPage.html
create mode 100644 wicket-core/src/test/java/org/apache/wicket/markup/html/form/AutoLabelEscapeMarkupTest$LabelPage.properties
create mode 100644 wicket-core/src/test/java/org/apache/wicket/markup/html/form/AutoLabelEscapeMarkupTest.java
create mode 100644 wicket-extensions/src/test/java/org/apache/wicket/extensions/ajax/markup/html/AjaxEditableLabelNullLabelEscapeTest.java
copy wicket-core/src/test/java/org/apache/wicket/markup/html/form/FormDefaultButtonTestPage.html => wicket-extensions/src/test/java/org/apache/wicket/extensions/markup/html/form/palette/PaletteAdditionalAttributesTest$AttributesPage.html (61%)
create mode 100644 wicket-extensions/src/test/java/org/apache/wicket/extensions/markup/html/form/palette/PaletteAdditionalAttributesTest.java
(wicket) 03/08: Escape the attribute values of a MetaDataHeaderItem as markup
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
papegaaij pushed a commit to branch wicket-8.x
in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 7f53ba4dff2f0041cc3c433c4b96cc4c4cfdf2e3
Author: Emond Papegaaij <pa...@apache.org>
AuthorDate: Wed Aug 19 22:24:56 2026 +0200
Escape the attribute values of a MetaDataHeaderItem as markup
generateString escaped attribute names with Strings.escapeMarkup but ran the
values through Strings.replaceAll(value, "\"", "\\\""). A backslash before a
double quote means nothing in HTML, so a value holding a double quote ended its
own attribute and whatever followed was read as further attributes. Values may
come from an IModel, so they are not necessarily literals. WICKET-6178 set out to
escape these values and this is what it left behind.
They now go through Strings.escapeMarkup, like the names beside them and like
every attribute ComponentTag writes.
Two tests asserted the old output. testEscapeOnlyDoubleQuotes was named after the
behaviour that was the problem and is renamed. HtmlImportHeaderItemTest expects
an escaped ampersand in the href of a link tag, which is what a url holding two
parameters has to carry, and is what ComponentTag writes for every other
attribute.
Co-Authored-By: Claude Opus 5 (1M context) <no...@anthropic.com>
---
.../wicket/markup/head/MetaDataHeaderItem.java | 2 +-
.../markup/head/HtmlImportHeaderItemTest.java | 7 +++--
.../wicket/markup/head/MetaDataHeaderItemTest.java | 35 ++++++++++++++++++++--
3 files changed, 39 insertions(+), 5 deletions(-)
diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/head/MetaDataHeaderItem.java b/wicket-core/src/main/java/org/apache/wicket/markup/head/MetaDataHeaderItem.java
index 77fce4bea1..09d1d8c444 100644
--- a/wicket-core/src/main/java/org/apache/wicket/markup/head/MetaDataHeaderItem.java
+++ b/wicket-core/src/main/java/org/apache/wicket/markup/head/MetaDataHeaderItem.java
@@ -131,7 +131,7 @@ public class MetaDataHeaderItem extends HeaderItem
{
buffer.append('=')
.append('"')
- .append(Strings.replaceAll(value.toString(), "\"", "\\\""))
+ .append(Strings.escapeMarkup(value.toString()))
.append('"');
}
}
diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/head/HtmlImportHeaderItemTest.java b/wicket-core/src/test/java/org/apache/wicket/markup/head/HtmlImportHeaderItemTest.java
index 79244b6f5e..b629da4c84 100644
--- a/wicket-core/src/test/java/org/apache/wicket/markup/head/HtmlImportHeaderItemTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/markup/head/HtmlImportHeaderItemTest.java
@@ -20,6 +20,7 @@ import static org.junit.Assert.assertEquals;
import org.apache.wicket.markup.html.basic.SimplePage;
import org.apache.wicket.request.mapper.parameter.PageParameters;
+import org.apache.wicket.util.string.Strings;
import org.apache.wicket.util.tester.WicketTester;
import org.junit.Test;
@@ -53,7 +54,9 @@ public class HtmlImportHeaderItemTest
MetaDataHeaderItem importLink = HtmlImportHeaderItem
.forImportLinkTag(SimplePage.class, parameters, "monitor", true);
- assertEquals("<link rel=\"import\" href=\"" + pageUrl + "\" media=\"monitor\" async />\n",
- importLink.generateString());
+ // the href is escaped as markup, like every other attribute Wicket writes. The url holds
+ // a literal & between its two parameters, which has to be written as an entity
+ assertEquals("<link rel=\"import\" href=\"" + Strings.escapeMarkup(pageUrl)
+ + "\" media=\"monitor\" async />\n", importLink.generateString());
}
}
diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/head/MetaDataHeaderItemTest.java b/wicket-core/src/test/java/org/apache/wicket/markup/head/MetaDataHeaderItemTest.java
index 6cff0a5ab6..c8b5ea1699 100644
--- a/wicket-core/src/test/java/org/apache/wicket/markup/head/MetaDataHeaderItemTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/markup/head/MetaDataHeaderItemTest.java
@@ -43,12 +43,43 @@ public class MetaDataHeaderItemTest
assertEquals(expectedString, metaTag.generateString());
}
+ /**
+ * Attribute values are escaped as markup. A backslash before a double quote means nothing in
+ * HTML, so escaping that way left the value able to close its own attribute.
+ */
@Test
- public void testEscapeOnlyDoubleQuotes() throws Exception
+ public void testEscapesAttributeValuesAsMarkup() throws Exception
{
- String expectedString = "<link rel=\"single quote \' double quotes\\\"\" href=\"\" />\n";
+ String expectedString = "<link rel=\"single quote ' double quotes"\" href=\"\" />\n";
MetaDataHeaderItem metaTag = MetaDataHeaderItem.forLinkTag("single quote \' double quotes\"", "");
assertEquals(expectedString, metaTag.generateString());
}
+
+ /**
+ * A value cannot end its attribute and add another one.
+ */
+ @Test
+ public void testAttributeValueCannotInjectAnotherAttribute() throws Exception
+ {
+ MetaDataHeaderItem metaTag = MetaDataHeaderItem.forMetaTag("description",
+ "x\" onload=\"x=1");
+
+ assertEquals("<meta name=\"description\" content=\"x" onload="x=1\" />\n",
+ metaTag.generateString());
+ }
+
+ /**
+ * The same for a value taken from a model.
+ */
+ @Test
+ public void testAttributeValueFromModelIsEscaped() throws Exception
+ {
+ MetaDataHeaderItem metaTag = MetaDataHeaderItem.forLinkTag("stylesheet", "");
+ metaTag.addTagAttribute("title", Model.of("<script>x=1</script>"));
+
+ assertEquals(
+ "<link rel=\"stylesheet\" href=\"\" title=\"<script>x=1</script>\" />\n",
+ metaTag.generateString());
+ }
}
(wicket) 02/08: Escape the label text wicket:label takes from a model or a bundle
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
papegaaij pushed a commit to branch wicket-8.x
in repository https://gitbox.apache.org/repos/asf/wicket.git
commit b055a7bd38f30f52b50bcb0296cd86ec22c1c8df
Author: Emond Papegaaij <pa...@apache.org>
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) <no...@anthropic.com>
---
.../markup/html/form/AutoLabelTextResolver.java | 62 +++++----
.../form/AutoLabelEscapeMarkupTest$LabelPage.html | 14 +++
.../AutoLabelEscapeMarkupTest$LabelPage.properties | 2 +
.../html/form/AutoLabelEscapeMarkupTest.java | 140 +++++++++++++++++++++
4 files changed, 195 insertions(+), 23 deletions(-)
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 bf48aaae34..1c7cf0adc8 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;
}
}
diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/form/AutoLabelEscapeMarkupTest$LabelPage.html b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/AutoLabelEscapeMarkupTest$LabelPage.html
new file mode 100644
index 0000000000..8f80d7150d
--- /dev/null
+++ b/wicket-core/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/src/test/java/org/apache/wicket/markup/html/form/AutoLabelEscapeMarkupTest$LabelPage.properties b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/AutoLabelEscapeMarkupTest$LabelPage.properties
new file mode 100644
index 0000000000..c510bb62f2
--- /dev/null
+++ b/wicket-core/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/src/test/java/org/apache/wicket/markup/html/form/AutoLabelEscapeMarkupTest.java b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/AutoLabelEscapeMarkupTest.java
new file mode 100644
index 0000000000..babadd9af3
--- /dev/null
+++ b/wicket-core/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.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.wicket.markup.html.WebPage;
+import org.apache.wicket.model.Model;
+import org.apache.wicket.util.tester.WicketTestCase;
+import org.junit.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.
+ */
+public 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
+ public void labelFromModelIsEscaped()
+ {
+ tester.startPage(new LabelPage("<script>x=0</script>"));
+
+ String response = tester.getLastResponseAsString();
+ assertTrue("label from the model should be escaped",
+ response.contains("<script>x=0</script>"));
+ assertFalse("label from the model should not reach the markup as markup",
+ response.contains("<script>x=0</script>"));
+ }
+
+ /** The default label, looked up in the bundle by the component's id. */
+ @Test
+ public void labelFromDefaultLabelIsEscaped()
+ {
+ tester.startPage(new LabelPage(""));
+
+ String response = tester.getLastResponseAsString();
+ assertTrue("default label should be escaped",
+ response.contains("<script>x=1</script>"));
+ assertFalse("default label should not reach the markup as markup",
+ response.contains("<script>x=1</script>"));
+ }
+
+ /** The message key on the tag, {@literal <wicket:label key="markupKey"/>}. */
+ @Test
+ public void labelFromMessageKeyIsEscaped()
+ {
+ tester.startPage(new LabelPage(""));
+
+ String response = tester.getLastResponseAsString();
+ assertTrue("label from a message key should be escaped",
+ response.contains("<script>x=2</script>"));
+ assertFalse("label from a message key should not reach the markup as markup",
+ response.contains("<script>x=2</script>"));
+ }
+
+ /**
+ * 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
+ public void labelFromTagBodyIsRenderedAsMarkup()
+ {
+ tester.startPage(new LabelPage(""));
+
+ String response = tester.getLastResponseAsString();
+ assertTrue("the tag body should be rendered as markup",
+ response.contains(BODY_MARKUP));
+ assertFalse("the tag body should not be escaped",
+ response.contains("<em>emphasis</em>"));
+ }
+
+ /**
+ * 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
+ public 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
+ public void tagBodyBecomesTheComponentLabel()
+ {
+ tester.startPage(new LabelPage(""));
+
+ assertEquals(BODY_MARKUP,
+ ((FormComponent<?>)tester.getComponentFromLastRenderedPage("form:fromBody")).getLabel()
+ .getObject());
+ }
+}
(wicket) 04/08: Escape the additional option attributes of a palette
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
papegaaij pushed a commit to branch wicket-8.x
in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 8c1eff3d42266a68b14c3628a9ff324837269c55
Author: Emond Papegaaij <pa...@apache.org>
AuthorDate: Wed Aug 19 22:25:09 2026 +0200
Escape the additional option attributes of a palette
AbstractOptions escapes the id and the display value of an option according to
escapeModelStrings, and wrote the keys and values of getAdditionalAttributes as
they came. RadioChoice and CheckBoxMultipleChoice escape both for the hook of the
same name, and have since bf8e0e1d7b, so the palette was the odd one out.
Both go through Strings.escapeMarkup now, unconditionally, the way the other two
do it: the buffer here builds the attribute itself, so there is nothing else that
would encode it and nothing that could encode it twice.
Note this escapes a name well enough to keep it inside its own attribute and no
further. Strings.escapeMarkup leaves a space and an equals sign alone, so a name
assembled from something other than an identifier is still able to end up as more
than one attribute. Attribute names are identifiers an application writes, which
is the assumption RadioChoice makes as well, and the test says so.
Co-Authored-By: Claude Opus 5 (1M context) <no...@anthropic.com>
---
.../form/palette/component/AbstractOptions.java | 4 +-
...tteAdditionalAttributesTest$AttributesPage.html | 7 ++
.../palette/PaletteAdditionalAttributesTest.java | 113 +++++++++++++++++++++
3 files changed, 122 insertions(+), 2 deletions(-)
diff --git a/wicket-extensions/src/main/java/org/apache/wicket/extensions/markup/html/form/palette/component/AbstractOptions.java b/wicket-extensions/src/main/java/org/apache/wicket/extensions/markup/html/form/palette/component/AbstractOptions.java
index 9c276fddfd..980c093a24 100644
--- a/wicket-extensions/src/main/java/org/apache/wicket/extensions/markup/html/form/palette/component/AbstractOptions.java
+++ b/wicket-extensions/src/main/java/org/apache/wicket/extensions/markup/html/form/palette/component/AbstractOptions.java
@@ -124,9 +124,9 @@ public abstract class AbstractOptions<T> extends FormComponent<T>
for (Map.Entry<String, String> entry : additionalAttributesMap.entrySet())
{
buffer.append(' ')
- .append(entry.getKey())
+ .append(Strings.escapeMarkup(entry.getKey()))
.append("=\"")
- .append(entry.getValue())
+ .append(Strings.escapeMarkup(entry.getValue()))
.append("\"");
}
}
diff --git a/wicket-extensions/src/test/java/org/apache/wicket/extensions/markup/html/form/palette/PaletteAdditionalAttributesTest$AttributesPage.html b/wicket-extensions/src/test/java/org/apache/wicket/extensions/markup/html/form/palette/PaletteAdditionalAttributesTest$AttributesPage.html
new file mode 100644
index 0000000000..ae1230fd0a
--- /dev/null
+++ b/wicket-extensions/src/test/java/org/apache/wicket/extensions/markup/html/form/palette/PaletteAdditionalAttributesTest$AttributesPage.html
@@ -0,0 +1,7 @@
+<html>
+<body>
+ <form wicket:id="form">
+ <div wicket:id="palette"></div>
+ </form>
+</body>
+</html>
diff --git a/wicket-extensions/src/test/java/org/apache/wicket/extensions/markup/html/form/palette/PaletteAdditionalAttributesTest.java b/wicket-extensions/src/test/java/org/apache/wicket/extensions/markup/html/form/palette/PaletteAdditionalAttributesTest.java
new file mode 100644
index 0000000000..4c399c2fdf
--- /dev/null
+++ b/wicket-extensions/src/test/java/org/apache/wicket/extensions/markup/html/form/palette/PaletteAdditionalAttributesTest.java
@@ -0,0 +1,113 @@
+/*
+ * 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.markup.html.form.palette;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.wicket.extensions.markup.html.form.palette.component.AbstractOptions;
+import org.apache.wicket.markup.html.WebPage;
+import org.apache.wicket.markup.html.form.Form;
+import org.apache.wicket.markup.html.form.IChoiceRenderer;
+import org.apache.wicket.model.IModel;
+import org.apache.wicket.model.util.ListModel;
+import org.apache.wicket.util.tester.WicketTestCase;
+import org.junit.Test;
+
+/**
+ * The attributes {@link AbstractOptions} adds to an option are escaped, the way
+ * {@code RadioChoice} and {@code CheckBoxMultipleChoice} escape the attributes of the same hook.
+ */
+public class PaletteAdditionalAttributesTest extends WicketTestCase
+{
+ public static class AttributesPage extends WebPage
+ {
+ AttributesPage(final Map<String, String> additionalAttributes)
+ {
+ Form<Object> form = new Form<>("form");
+ add(form);
+
+ IChoiceRenderer<String> renderer = new IChoiceRenderer<String>()
+ {
+ @Override
+ public Object getDisplayValue(String s)
+ {
+ return s;
+ }
+
+ @Override
+ public String getIdValue(String s, int index)
+ {
+ return s;
+ }
+ };
+
+ IModel<List<String>> selected = new ListModel<>(new ArrayList<>(Arrays.asList("A")));
+ IModel<List<String>> all = new ListModel<>(new ArrayList<>(Arrays.asList("A", "B")));
+
+ form.add(new Palette<String>("palette", selected, all, renderer, 10, true)
+ {
+ @Override
+ protected Map<String, String> getAdditionalAttributesForChoices(Object choice)
+ {
+ return additionalAttributes;
+ }
+
+ @Override
+ protected Map<String, String> getAdditionalAttributesForSelection(Object choice)
+ {
+ return additionalAttributes;
+ }
+ });
+ }
+ }
+
+ @Test
+ public void additionalAttributeValueIsEscaped()
+ {
+ tester.startPage(new AttributesPage(
+ Collections.singletonMap("title", "x\" onload=\"x=1")));
+
+ String response = tester.getLastResponseAsString();
+ assertTrue("the attribute value should be escaped",
+ response.contains("title=\"x" onload="x=1\""));
+ assertFalse("the attribute value should not be able to add another attribute",
+ response.contains("onload=\"x=1\""));
+ }
+
+ /**
+ * The attribute name is escaped the same way, which is what {@code RadioChoice} and
+ * {@code CheckBoxMultipleChoice} do with the name from the same hook. Note that
+ * {@code Strings#escapeMarkup} leaves a space and an equals sign alone, so escaping a name is
+ * not by itself a defence against a name that is built from untrusted input. Attribute names
+ * are identifiers the application writes, not data.
+ */
+ @Test
+ public void additionalAttributeNameIsEscaped()
+ {
+ tester.startPage(new AttributesPage(Collections.singletonMap("a\"b", "v")));
+
+ assertTrue("the attribute name should be escaped",
+ tester.getLastResponseAsString().contains("a"b=\"v\""));
+ }
+}
(wicket) 01/08: Escape the model string a Button writes into a button element body
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
papegaaij pushed a commit to branch wicket-8.x
in repository https://gitbox.apache.org/repos/asf/wicket.git
commit f4ff8c8abcdc55292c41dbf8ffaf6ecc8d283096
Author: Emond Papegaaij <pa...@apache.org>
AuthorDate: Wed Aug 19 22:24:25 2026 +0200
Escape the model string a Button writes into a button element body
Button clears escapeModelStrings in its constructor so that its value attribute
is not encoded twice: ComponentTag#writeOutput encodes every attribute when it
writes the tag. That reasoning covers the attribute and nothing else. When the
tag is a button element, onComponentTagBody writes the same model string into
the element body, and nothing encodes a body, so markup in the model was
rendered as markup. Setting the flag back was not an alternative, because the
value attribute would then be encoded twice.
The body is escaped where it is written, and only when the flag is off. With the
flag on getDefaultModelObjectAsString() has escaped already and escaping again
would encode twice.
whenButtonElement_thenModelObjectIsUsedAsTextContent asserted the old output and
is renamed, since a button body now carries an escaped ampersand the same way
the value attribute of an input always has.
Co-Authored-By: Claude Opus 5 (1M context) <no...@anthropic.com>
---
.../org/apache/wicket/markup/html/form/Button.java | 12 ++++++--
.../apache/wicket/markup/html/form/ButtonTest.java | 33 ++++++++++++++++++++--
2 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Button.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Button.java
index 0b4bcdf91f..d89974cc4b 100644
--- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Button.java
+++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Button.java
@@ -218,8 +218,16 @@ public class Button extends FormComponent<String> implements IFormSubmittingComp
if ("button".equals(openTag.getName()))
{
String modelObjectAsString = getDefaultModelObjectAsString();
- if (Strings.isEmpty(modelObjectAsString) == false) {
- replaceComponentTagBody(markupStream, openTag, modelObjectAsString);
+ if (Strings.isEmpty(modelObjectAsString) == false)
+ {
+ // The constructor clears escapeModelStrings so that the value attribute is not
+ // encoded twice, ComponentTag#writeOutput already encodes it. An element body is
+ // not encoded anywhere, so escape here instead of relying on that flag. When an
+ // application does set the flag, getDefaultModelObjectAsString() has escaped
+ // already and escaping again would double encode.
+ replaceComponentTagBody(markupStream, openTag,
+ getEscapeModelStrings() ? modelObjectAsString
+ : Strings.escapeMarkup(modelObjectAsString));
return;
}
}
diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/form/ButtonTest.java b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/ButtonTest.java
index fe303e6a16..c442a3f6db 100644
--- a/wicket-core/src/test/java/org/apache/wicket/markup/html/form/ButtonTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/ButtonTest.java
@@ -16,8 +16,10 @@
*/
package org.apache.wicket.markup.html.form;
+import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
@@ -58,9 +60,14 @@ public class ButtonTest extends WicketTestCase
/**
* https://issues.apache.org/jira/browse/WICKET-6225
+ *
+ * The body of a button element is escaped once, like the value attribute of an input element
+ * is. Button clears escapeModelStrings in its constructor so that ComponentTag#writeOutput does
+ * not encode that attribute twice, and nothing encodes an element body, so the body is escaped
+ * where it is written instead.
*/
@Test
- public void whenButtonElement_thenModelObjectIsUsedAsTextContent()
+ public void whenButtonElement_thenModelObjectIsUsedAsEscapedTextContent()
{
tester.getApplication().getMarkupSettings().setStripWicketTags(false);
String text = "some text & another text";
@@ -77,7 +84,29 @@ public class ButtonTest extends WicketTestCase
TagTester buttonTagTester = tester.getTagByWicketId("button");
assertThat(buttonTagTester, is(notNullValue()));
assertThat(buttonTagTester.getAttribute("value"), is(nullValue()));
- assertThat(buttonTagTester.getValue(), is(equalTo(text)));
+ assertThat(buttonTagTester.getValue(), is(equalTo("some text & another text")));
+ }
+
+ /**
+ * Markup in the model of a button does not become markup in the body of the button element.
+ */
+ @Test
+ public void whenButtonElement_thenMarkupInModelObjectIsEscaped()
+ {
+ tester.getApplication().getMarkupSettings().setStripWicketTags(false);
+ TestPage testPage = new TestPage(Model.of("<script>x=1</script>")) {
+ @Override
+ public IResourceStream getMarkupResourceStream(MarkupContainer container, Class<?> containerClass)
+ {
+ return new StringResourceStream("<html><body>"
+ + "<form wicket:id=\"form\"><button wicket:id=\"button\"></button></form></body></html>");
+ }
+ };
+ tester.startPage(testPage);
+
+ assertThat(tester.getTagByWicketId("button").getValue(),
+ is(equalTo("<script>x=1</script>")));
+ assertThat(tester.getLastResponseAsString(), not(containsString("<script>x=1</script>")));
}
/**
(wicket) 06/08: Say in the javadoc which methods have their value written to the markup as is
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
papegaaij pushed a commit to branch wicket-8.x
in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 0d566fb2a998d1019db62859fa6ea114168e98f3
Author: Emond Papegaaij <pa...@apache.org>
AuthorDate: Thu Aug 20 12:32:01 2026 +0200
Say in the javadoc which methods have their value written to the markup as is
Escaping in the render path is a convention among callers rather than something
the path itself guarantees. replaceComponentTagBody and Response#write write what
they are handed, and Label is safe only because it hands them
getDefaultModelObjectAsString(). Where an overridable method's value reaches one
of those writes, an implementation has to know that, and until now it had to work
it out by reading the caller.
Twenty-six methods now say so and name the escaping to apply: the body of an
option points at escapeOptionHtml, everything else at Strings#escapeMarkup. The
wording follows what the method does with the value. A getter returns it, a
setter is handed it, appendOptionHtml and setOptionAttributes append it to the
option buffer, the renderer methods write it to a Response, and escapeHtml
decides whether the output of a template is escaped at all.
Where the framework calls the method and the default implementation escapes
correctly, the note says so and addresses the override instead, since relying on
the default is not what puts anything at risk. That covers the three hooks of
AbstractChoice: renderValue escapes the body of the option, setOptionAttributes
escapes the id it writes, and getDefaultChoice returns no choice at all unless a
subclass builds one.
Methods whose caller escapes for them are left alone, since telling an
implementation to escape there would have it encode twice: the display value
methods of AbstractSingleSelectChoice, defaultNullLabel of the editable labels,
the additional attributes of RadioChoice and CheckBoxMultipleChoice, and those of
the palette.
Include is left alone as well. What its method returns is the least of what that
component does with the value it is given, and it is being looked at on its own.
Co-Authored-By: Claude Opus 5 (1M context) <no...@anthropic.com>
---
.../wicket/markup/html/form/AbstractChoice.java | 28 +++++++++++++++++
.../markup/html/form/CheckBoxMultipleChoice.java | 36 ++++++++++++++++++++++
.../wicket/markup/html/form/RadioChoice.java | 36 ++++++++++++++++++++++
.../html/navigation/paging/PagingNavigation.java | 12 ++++++++
.../wicket/markup/transformer/ITransformer.java | 6 ++++
.../autocomplete/AbstractAutoCompleteRenderer.java | 6 ++++
.../html/autocomplete/IAutoCompleteRenderer.java | 18 +++++++++++
.../extensions/breadcrumb/BreadCrumbBar.java | 6 ++++
.../wicket/velocity/VelocityContributor.java | 6 ++++
.../wicket/velocity/markup/html/VelocityPanel.java | 6 ++++
10 files changed, 160 insertions(+)
diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/AbstractChoice.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/AbstractChoice.java
index fd1cacf273..aeba64719b 100644
--- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/AbstractChoice.java
+++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/AbstractChoice.java
@@ -324,6 +324,14 @@ public abstract class AbstractChoice<T, E> extends FormComponent<T>
/**
* Get a default choice to be rendered additionally to the choices available in the model.
*
+ * <p>
+ * <b>The return value of an override is written to the markup as is, without escaping.</b>
+ * This implementation returns no choice at all, and the override in
+ * {@link AbstractSingleSelectChoice} escapes the body of the option it builds. An override
+ * that builds an option itself must escape the option body with
+ * {@link Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @param selectedValue
* The currently selected value
* @return Any default choice, such as "Choose One", depending on the subclass
@@ -394,6 +402,13 @@ public abstract class AbstractChoice<T, E> extends FormComponent<T>
/**
* Generates and appends html for a single choice into the provided buffer
*
+ * <p>
+ * <b>Whatever an override appends reaches the markup as is, without escaping.</b> This
+ * implementation escapes the body of the option it appends, according to
+ * {@link #getEscapeModelStrings()}. An override that assembles an option itself must do
+ * the same for whatever it puts there, with {@link Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @param buffer
* Appending string buffer that will have the generated html appended
* @param choice
@@ -445,6 +460,13 @@ public abstract class AbstractChoice<T, E> extends FormComponent<T>
/**
* Sets the attributes of a single choice into the provided buffer.
*
+ * <p>
+ * <b>Whatever an override appends reaches the markup as is, without escaping.</b> This
+ * implementation escapes the value of the id attribute it writes. An override that adds
+ * attributes of its own must escape their values with
+ * {@link Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @param buffer
* Appending string buffer that will have the generated html appended
* @param choice
@@ -474,6 +496,12 @@ public abstract class AbstractChoice<T, E> extends FormComponent<T>
/**
* Method to override if you want special escaping of the options html.
*
+ * <p>
+ * <b>The return value is written to the body of the option as is, without escaping.</b> An
+ * override that returns the display value without escaping it puts user input and other
+ * dynamic content into the page as markup.
+ * </p>
+ *
* @param displayValue
* @return The escaped display value
*/
diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/CheckBoxMultipleChoice.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/CheckBoxMultipleChoice.java
index f1c3520cf0..88d733f983 100644
--- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/CheckBoxMultipleChoice.java
+++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/CheckBoxMultipleChoice.java
@@ -234,6 +234,12 @@ public class CheckBoxMultipleChoice<T> extends ListMultipleChoice<T>
}
/**
+ * <p>
+ * <b>The return value is written to the markup as is, without escaping.</b> Any part of
+ * it that comes from user input or other dynamic content must be escaped with
+ * {@link Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @return Prefix to use before choice
*/
public String getPrefix()
@@ -242,6 +248,12 @@ public class CheckBoxMultipleChoice<T> extends ListMultipleChoice<T>
}
/**
+ * <p>
+ * <b>The return value is written to the markup as is, without escaping.</b> Any part of
+ * it that comes from user input or other dynamic content must be escaped with
+ * {@link Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @param index
* index of the choice
* @param choice
@@ -255,6 +267,12 @@ public class CheckBoxMultipleChoice<T> extends ListMultipleChoice<T>
}
/**
+ * <p>
+ * <b>The return value is written to the markup as is, without escaping.</b> Any part of
+ * it that comes from user input or other dynamic content must be escaped with
+ * {@link Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @param index
* index of the choice
* @param choice
@@ -268,6 +286,12 @@ public class CheckBoxMultipleChoice<T> extends ListMultipleChoice<T>
}
/**
+ * <p>
+ * <b>The value passed here is written to the markup as is, without escaping.</b> Any
+ * part of it that comes from user input or other dynamic content must be escaped with
+ * {@link Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @param prefix
* Prefix to use before choice
* @return this
@@ -286,6 +310,12 @@ public class CheckBoxMultipleChoice<T> extends ListMultipleChoice<T>
}
/**
+ * <p>
+ * <b>The return value is written to the markup as is, without escaping.</b> Any part of
+ * it that comes from user input or other dynamic content must be escaped with
+ * {@link Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @return Separator to use between radio options
*/
public String getSuffix()
@@ -294,6 +324,12 @@ public class CheckBoxMultipleChoice<T> extends ListMultipleChoice<T>
}
/**
+ * <p>
+ * <b>The value passed here is written to the markup as is, without escaping.</b> Any
+ * part of it that comes from user input or other dynamic content must be escaped with
+ * {@link Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @param suffix
* Separator to use between radio options
* @return this
diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/RadioChoice.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/RadioChoice.java
index 39942047ed..f1299de054 100644
--- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/RadioChoice.java
+++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/RadioChoice.java
@@ -239,6 +239,12 @@ public class RadioChoice<T> extends AbstractSingleSelectChoice<T>
}
/**
+ * <p>
+ * <b>The return value is written to the markup as is, without escaping.</b> Any part of
+ * it that comes from user input or other dynamic content must be escaped with
+ * {@link Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @return Prefix to use before choice
*/
public String getPrefix()
@@ -247,6 +253,12 @@ public class RadioChoice<T> extends AbstractSingleSelectChoice<T>
}
/**
+ * <p>
+ * <b>The return value is written to the markup as is, without escaping.</b> Any part of
+ * it that comes from user input or other dynamic content must be escaped with
+ * {@link Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @param index
* index of the choice
* @param choice
@@ -260,6 +272,12 @@ public class RadioChoice<T> extends AbstractSingleSelectChoice<T>
}
/**
+ * <p>
+ * <b>The return value is written to the markup as is, without escaping.</b> Any part of
+ * it that comes from user input or other dynamic content must be escaped with
+ * {@link Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @param index
* index of the choice
* @param choice
@@ -273,6 +291,12 @@ public class RadioChoice<T> extends AbstractSingleSelectChoice<T>
}
/**
+ * <p>
+ * <b>The value passed here is written to the markup as is, without escaping.</b> Any
+ * part of it that comes from user input or other dynamic content must be escaped with
+ * {@link Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @param prefix
* Prefix to use before choice
* @return this
@@ -286,6 +310,12 @@ public class RadioChoice<T> extends AbstractSingleSelectChoice<T>
}
/**
+ * <p>
+ * <b>The return value is written to the markup as is, without escaping.</b> Any part of
+ * it that comes from user input or other dynamic content must be escaped with
+ * {@link Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @return Separator to use between radio options
*/
public String getSuffix()
@@ -294,6 +324,12 @@ public class RadioChoice<T> extends AbstractSingleSelectChoice<T>
}
/**
+ * <p>
+ * <b>The value passed here is written to the markup as is, without escaping.</b> Any
+ * part of it that comes from user input or other dynamic content must be escaped with
+ * {@link Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @param suffix
* Separator to use between radio options
* @return this
diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/navigation/paging/PagingNavigation.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/navigation/paging/PagingNavigation.java
index cce7c63247..b05d77ab31 100644
--- a/wicket-core/src/main/java/org/apache/wicket/markup/html/navigation/paging/PagingNavigation.java
+++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/navigation/paging/PagingNavigation.java
@@ -188,6 +188,12 @@ public class PagingNavigation extends Loop
/**
* Gets the seperator.
*
+ * <p>
+ * <b>The return value is written to the markup as is, without escaping.</b> Any part of
+ * it that comes from user input or other dynamic content must be escaped with
+ * {@link org.apache.wicket.util.string.Strings#escapeMarkup(CharSequence) Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @return the seperator
*/
public String getSeparator()
@@ -229,6 +235,12 @@ public class PagingNavigation extends Loop
/**
* Sets the seperator. Null meaning, no separator at all.
*
+ * <p>
+ * <b>The value passed here is written to the markup as is, without escaping.</b> Any
+ * part of it that comes from user input or other dynamic content must be escaped with
+ * {@link org.apache.wicket.util.string.Strings#escapeMarkup(CharSequence) Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @param separator
* the seperator
*/
diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/transformer/ITransformer.java b/wicket-core/src/main/java/org/apache/wicket/markup/transformer/ITransformer.java
index b883e83516..e03a449150 100644
--- a/wicket-core/src/main/java/org/apache/wicket/markup/transformer/ITransformer.java
+++ b/wicket-core/src/main/java/org/apache/wicket/markup/transformer/ITransformer.java
@@ -32,6 +32,12 @@ public interface ITransformer
* Will be invoked after all child components have been processed to allow for transforming the
* markup generated.
*
+ * <p>
+ * <b>The return value is written to the markup as is, without escaping.</b> Any part of
+ * it that comes from user input or other dynamic content must be escaped with
+ * {@link org.apache.wicket.util.string.Strings#escapeMarkup(CharSequence) Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @param component
* The associated Wicket component
* @param output
diff --git a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/autocomplete/AbstractAutoCompleteRenderer.java b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/autocomplete/AbstractAutoCompleteRenderer.java
index d7af7cc356..938509fddf 100644
--- a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/autocomplete/AbstractAutoCompleteRenderer.java
+++ b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/autocomplete/AbstractAutoCompleteRenderer.java
@@ -74,6 +74,12 @@ public abstract class AbstractAutoCompleteRenderer<T> implements IAutoCompleteRe
* Render the visual portion of the assist. Usually the html representing the assist choice
* object is written out to the response use {@link Response#write(CharSequence)}
*
+ * <p>
+ * <b>Whatever is written to the response reaches the markup as is, without escaping.</b>
+ * Any part of it that comes from user input or other dynamic content must be escaped
+ * before it is written, with {@link Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @param object
* current assist choice
* @param response
diff --git a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/autocomplete/IAutoCompleteRenderer.java b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/autocomplete/IAutoCompleteRenderer.java
index 55a0d6b1c3..3421f18c47 100644
--- a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/autocomplete/IAutoCompleteRenderer.java
+++ b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/autocomplete/IAutoCompleteRenderer.java
@@ -66,6 +66,12 @@ public interface IAutoCompleteRenderer<T> extends IDetachable
* Render the html fragment for the given completion object. Usually the html is written out by
* calling {@link Response#write(CharSequence)}.
*
+ * <p>
+ * <b>Whatever is written to the response reaches the markup as is, without escaping.</b>
+ * Any part of it that comes from user input or other dynamic content must be escaped
+ * before it is written, with {@link org.apache.wicket.util.string.Strings#escapeMarkup(CharSequence) Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @param object
* completion choice object
* @param response
@@ -80,6 +86,12 @@ public interface IAutoCompleteRenderer<T> extends IDetachable
* Render the html header fragment for the completion. Usually the html is written out by
* calling {@link Response#write(CharSequence)}.
*
+ * <p>
+ * <b>Whatever is written to the response reaches the markup as is, without escaping.</b>
+ * Any part of it that comes from user input or other dynamic content must be escaped
+ * before it is written, with {@link org.apache.wicket.util.string.Strings#escapeMarkup(CharSequence) Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @param response
*/
void renderHeader(Response response);
@@ -88,6 +100,12 @@ public interface IAutoCompleteRenderer<T> extends IDetachable
* Render the html footer fragment for the completion. Usually the html is written out by
* calling {@link Response#write(CharSequence)}.
*
+ * <p>
+ * <b>Whatever is written to the response reaches the markup as is, without escaping.</b>
+ * Any part of it that comes from user input or other dynamic content must be escaped
+ * before it is written, with {@link org.apache.wicket.util.string.Strings#escapeMarkup(CharSequence) Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @param response
* @param count
* The number of choices rendered
diff --git a/wicket-extensions/src/main/java/org/apache/wicket/extensions/breadcrumb/BreadCrumbBar.java b/wicket-extensions/src/main/java/org/apache/wicket/extensions/breadcrumb/BreadCrumbBar.java
index ec85ec0e20..e48f1aa560 100644
--- a/wicket-extensions/src/main/java/org/apache/wicket/extensions/breadcrumb/BreadCrumbBar.java
+++ b/wicket-extensions/src/main/java/org/apache/wicket/extensions/breadcrumb/BreadCrumbBar.java
@@ -245,6 +245,12 @@ public class BreadCrumbBar extends Panel implements IBreadCrumbModel
}
/**
+ * <p>
+ * <b>The return value is written to the markup as is, without escaping.</b> Any part of
+ * it that comes from user input or other dynamic content must be escaped with
+ * {@link org.apache.wicket.util.string.Strings#escapeMarkup(CharSequence) Strings#escapeMarkup(CharSequence)}.
+ * </p>
+ *
* @return markup used as a separator between breadcrumbs. By default <code>/</code> is used,
* but <code>>></code> is also a popular choice.
*/
diff --git a/wicket-velocity/src/main/java/org/apache/wicket/velocity/VelocityContributor.java b/wicket-velocity/src/main/java/org/apache/wicket/velocity/VelocityContributor.java
index c78aa34fbf..d6204267a9 100644
--- a/wicket-velocity/src/main/java/org/apache/wicket/velocity/VelocityContributor.java
+++ b/wicket-velocity/src/main/java/org/apache/wicket/velocity/VelocityContributor.java
@@ -104,6 +104,12 @@ public class VelocityContributor extends Behavior
}
/**
+ * <p>
+ * <b>While this returns false, the output of the template is written to the markup as
+ * is, without escaping.</b> A template that interpolates user input or other dynamic
+ * content then puts it in the page unescaped.
+ * </p>
+ *
* @return whether to escape HTML characters. The default value is false
*/
protected boolean escapeHtml()
diff --git a/wicket-velocity/src/main/java/org/apache/wicket/velocity/markup/html/VelocityPanel.java b/wicket-velocity/src/main/java/org/apache/wicket/velocity/markup/html/VelocityPanel.java
index 218bafb9dc..2b83b4a9a1 100644
--- a/wicket-velocity/src/main/java/org/apache/wicket/velocity/markup/html/VelocityPanel.java
+++ b/wicket-velocity/src/main/java/org/apache/wicket/velocity/markup/html/VelocityPanel.java
@@ -192,6 +192,12 @@ public abstract class VelocityPanel extends Panel
/**
* Gets whether to escape HTML characters.
*
+ * <p>
+ * <b>While this returns false, the output of the template is written to the markup as
+ * is, without escaping.</b> A template that interpolates user input or other dynamic
+ * content then puts it in the page unescaped.
+ * </p>
+ *
* @return whether to escape HTML characters. The default value is false.
*/
protected boolean escapeHtml()
(wicket) 08/08: Escape the value an editable label shows for an empty model
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
papegaaij pushed a commit to branch wicket-8.x
in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 6d6930c0ec3aa3f2e80933f74ebb4abd7359ad09
Author: Emond Papegaaij <pa...@apache.org>
AuthorDate: Thu Aug 20 12:32:22 2026 +0200
Escape the value an editable label shows for an empty model
The label of each of the three editable labels writes defaultNullLabel() to the
markup as it came when the model is empty, and hands the value over to the label
component when it is not, which escapes it according to escapeModelStrings. So the
placeholder was written as markup while the value beside it was escaped, and
defaultNullLabel() is protected, so what it returns is not necessarily the
constant the default implementation gives back.
AjaxEditableChoiceLabel shows it plainest. One branch of its onComponentTagBody
writes the value the IChoiceRenderer produced, which is escaped, and the other
writes defaultNullLabel(), which was not.
The value is escaped where it is written, following escapeModelStrings. The label
takes that setting from the panel in onConfigure, next to the delegation that is
already there, so turning escaping off on the panel turns it off for this value as
well; the test covers both.
Co-Authored-By: Claude Opus 5 (1M context) <no...@anthropic.com>
---
.../ajax/markup/html/AjaxEditableChoiceLabel.java | 4 +-
.../ajax/markup/html/AjaxEditableLabel.java | 5 +-
.../markup/html/AjaxEditableMultiLineLabel.java | 5 +-
.../html/AjaxEditableLabelNullLabelEscapeTest.java | 154 +++++++++++++++++++++
4 files changed, 165 insertions(+), 3 deletions(-)
diff --git a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/AjaxEditableChoiceLabel.java b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/AjaxEditableChoiceLabel.java
index 431b790e7b..c546046950 100644
--- a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/AjaxEditableChoiceLabel.java
+++ b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/AjaxEditableChoiceLabel.java
@@ -304,7 +304,9 @@ public class AjaxEditableChoiceLabel<T> extends AjaxEditableLabel<T>
if (Strings.isEmpty(displayValue))
{
- replaceComponentTagBody(markupStream, openTag, defaultNullLabel());
+ String nullLabel = defaultNullLabel();
+ replaceComponentTagBody(markupStream, openTag,
+ getEscapeModelStrings() ? Strings.escapeMarkup(nullLabel) : nullLabel);
}
else
{
diff --git a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/AjaxEditableLabel.java b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/AjaxEditableLabel.java
index 47329aa74e..0dc999ed03 100644
--- a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/AjaxEditableLabel.java
+++ b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/AjaxEditableLabel.java
@@ -40,6 +40,7 @@ import org.apache.wicket.model.IModel;
import org.apache.wicket.model.IObjectClassAwareModel;
import org.apache.wicket.request.cycle.RequestCycle;
import org.apache.wicket.util.convert.IConverter;
+import org.apache.wicket.util.string.Strings;
import org.apache.wicket.validation.IValidator;
/**
@@ -415,7 +416,9 @@ public class AjaxEditableLabel<T> extends Panel implements IGenericComponent<T,
Object modelObject = getDefaultModelObject();
if ((modelObject == null) || (modelObject instanceof String && ((String) modelObject).isEmpty()))
{
- replaceComponentTagBody(markupStream, openTag, defaultNullLabel());
+ String nullLabel = defaultNullLabel();
+ replaceComponentTagBody(markupStream, openTag,
+ getEscapeModelStrings() ? Strings.escapeMarkup(nullLabel) : nullLabel);
}
else
{
diff --git a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/AjaxEditableMultiLineLabel.java b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/AjaxEditableMultiLineLabel.java
index 65e297d8ec..c1587574a7 100644
--- a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/AjaxEditableMultiLineLabel.java
+++ b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/AjaxEditableMultiLineLabel.java
@@ -27,6 +27,7 @@ import org.apache.wicket.markup.html.basic.MultiLineLabel;
import org.apache.wicket.markup.html.form.FormComponent;
import org.apache.wicket.markup.html.form.TextArea;
import org.apache.wicket.model.IModel;
+import org.apache.wicket.util.string.Strings;
/**
* An inplace editor much like {@link AjaxEditableLabel}, but now with support for multi line
@@ -100,7 +101,9 @@ public class AjaxEditableMultiLineLabel<T> extends AjaxEditableLabel<T>
Object modelObject = getDefaultModelObject();
if ((modelObject == null) || (modelObject instanceof String && ((String) modelObject).isEmpty()))
{
- replaceComponentTagBody(markupStream, openTag, defaultNullLabel());
+ String nullLabel = defaultNullLabel();
+ replaceComponentTagBody(markupStream, openTag,
+ getEscapeModelStrings() ? Strings.escapeMarkup(nullLabel) : nullLabel);
}
else
{
diff --git a/wicket-extensions/src/test/java/org/apache/wicket/extensions/ajax/markup/html/AjaxEditableLabelNullLabelEscapeTest.java b/wicket-extensions/src/test/java/org/apache/wicket/extensions/ajax/markup/html/AjaxEditableLabelNullLabelEscapeTest.java
new file mode 100644
index 0000000000..4e25b66128
--- /dev/null
+++ b/wicket-extensions/src/test/java/org/apache/wicket/extensions/ajax/markup/html/AjaxEditableLabelNullLabelEscapeTest.java
@@ -0,0 +1,154 @@
+/*
+ * 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 static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Arrays;
+
+import org.apache.wicket.MarkupContainer;
+import org.apache.wicket.markup.IMarkupResourceStreamProvider;
+import org.apache.wicket.markup.html.WebPage;
+import org.apache.wicket.model.IModel;
+import org.apache.wicket.model.Model;
+import org.apache.wicket.util.resource.IResourceStream;
+import org.apache.wicket.util.resource.StringResourceStream;
+import org.apache.wicket.util.tester.WicketTestCase;
+import org.junit.Test;
+
+/**
+ * The value an editable label shows when its model is empty comes from an overridable method, so it
+ * is escaped where it is written, the same way the label escapes the model value it normally shows.
+ * <p>
+ * The escaping follows escapeModelStrings, which the label takes from the panel in onConfigure, so
+ * setting it on the panel decides whether the null label is escaped.
+ */
+public class AjaxEditableLabelNullLabelEscapeTest extends WicketTestCase
+{
+ private static final String MARKUP = "<script>x=1</script>";
+
+ private static final String ESCAPED = "<script>x=1</script>";
+
+ /** Renders one editable label over an empty model, so the null label is what is shown. */
+ private static class NullLabelPage extends WebPage implements IMarkupResourceStreamProvider
+ {
+ private NullLabelPage(AjaxEditableLabel<String> label)
+ {
+ add(label);
+ }
+
+ @Override
+ public IResourceStream getMarkupResourceStream(MarkupContainer container,
+ Class<?> containerClass)
+ {
+ return new StringResourceStream(
+ "<html><body><div wicket:id=\"label\"></div></body></html>");
+ }
+ }
+
+ private static AjaxEditableLabel<String> plainLabel()
+ {
+ return new AjaxEditableLabel<String>("label", Model.of(""))
+ {
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected String defaultNullLabel()
+ {
+ return MARKUP;
+ }
+ };
+ }
+
+ private static AjaxEditableLabel<String> multiLineLabel()
+ {
+ return new AjaxEditableMultiLineLabel<String>("label", Model.of(""))
+ {
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected String defaultNullLabel()
+ {
+ return MARKUP;
+ }
+ };
+ }
+
+ private static AjaxEditableLabel<String> choiceLabel()
+ {
+ IModel<java.util.List<String>> choices = Model.ofList(Arrays.asList("A", "B"));
+ return new AjaxEditableChoiceLabel<String>("label", Model.of(""), choices)
+ {
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected String defaultNullLabel()
+ {
+ return MARKUP;
+ }
+ };
+ }
+
+ private void assertNullLabelEscaped(AjaxEditableLabel<String> label)
+ {
+ tester.startPage(new NullLabelPage(label));
+
+ String response = tester.getLastResponseAsString();
+ assertTrue("the null label should be escaped",
+ response.contains(ESCAPED));
+ assertFalse("the null label should not reach the markup as markup",
+ response.contains(MARKUP));
+ }
+
+ @Test
+ public void nullLabelOfAnEditableLabelIsEscaped()
+ {
+ assertNullLabelEscaped(plainLabel());
+ }
+
+ @Test
+ public void nullLabelOfAnEditableMultiLineLabelIsEscaped()
+ {
+ assertNullLabelEscaped(multiLineLabel());
+ }
+
+ /**
+ * The choice label writes the null label in one branch and the value the renderer produced in
+ * the other, so both branches of the same method have to escape.
+ */
+ @Test
+ public void nullLabelOfAnEditableChoiceLabelIsEscaped()
+ {
+ assertNullLabelEscaped(choiceLabel());
+ }
+
+ /**
+ * Escaping follows the setting the panel passes to its label, so markup can still be shown on
+ * purpose.
+ */
+ @Test
+ public void nullLabelIsNotEscapedWhenEscapeModelStringsIsFalse()
+ {
+ AjaxEditableLabel<String> label = plainLabel();
+ label.setEscapeModelStrings(false);
+ tester.startPage(new NullLabelPage(label));
+
+ assertTrue("escaping is off, so the null label should be written as is",
+ tester.getLastResponseAsString().contains(MARKUP));
+ }
+}
(wicket) 05/08: Escape the body of the default option of a single select choice
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
papegaaij pushed a commit to branch wicket-8.x
in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 28b50bbd21cd210603093638aadcb5b939264522
Author: Emond Papegaaij <pa...@apache.org>
AuthorDate: Wed Aug 19 22:25:56 2026 +0200
Escape the body of the default option of a single select choice
getDefaultChoice wrote getNullValidDisplayValue() and getNullKeyDisplayValue()
into the body of the option as they came, while renderValue escapes the body of
every other option in the same select according to escapeModelStrings. Both
methods are protected, so what they hand back is not necessarily the plain text
the default implementation reads from a bundle; a subclass that shows the label of
the field instead returns the label model.
The value is escaped where the option is written, and through escapeOptionHtml, so
the whole select follows one rule and a choice that gave itself special escaping
of its options gets it for the default option too.
It is deliberately not escaped inside the two display value methods. A subclass
may take what super returns and compare it against a resource it looks up itself,
to tell whether the value is still the general one; escaping there would compare
an escaped value against an unescaped one and quietly stop matching.
The shipped bundles hold nullValid= and null=Choose One, both plain text, so
nothing changes for them. An application whose own nullValid or null entry holds
markup will see it escaped, and can still turn escaping off.
Co-Authored-By: Claude Opus 5 (1M context) <no...@anthropic.com>
---
.../html/form/AbstractSingleSelectChoice.java | 21 ++-
.../form/AbstractSingleSelectChoiceEscapeTest.java | 192 +++++++++++++++++++++
2 files changed, 211 insertions(+), 2 deletions(-)
diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/AbstractSingleSelectChoice.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/AbstractSingleSelectChoice.java
index d12f6d96f1..d2bee6c813 100644
--- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/AbstractSingleSelectChoice.java
+++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/AbstractSingleSelectChoice.java
@@ -289,6 +289,10 @@ public abstract class AbstractSingleSelectChoice<T> extends AbstractChoice<T, T>
*
* Otherwise no additional default choice will be returned.
*
+ * The body of the option is escaped according to {@link #getEscapeModelStrings()}, the same way
+ * the body of every other option is. An override that builds an option itself is responsible for
+ * escaping what it puts in the body.
+ *
* @see #getNullValidKey()
* @see #getNullKey()
* @see org.apache.wicket.markup.html.form.AbstractChoice#getDefaultChoice(String)
@@ -315,7 +319,7 @@ public abstract class AbstractSingleSelectChoice<T> extends AbstractChoice<T, T>
}
// Add body of option tag
- buffer.append(" value=\"\">").append(option).append("</option>");
+ buffer.append(" value=\"\">").append(escapeOptionBody(option)).append("</option>");
return buffer;
}
else
@@ -325,12 +329,25 @@ public abstract class AbstractSingleSelectChoice<T> extends AbstractChoice<T, T>
{
// Force the user to pick a non-null value
String option = getNullKeyDisplayValue();
- return "\n<option selected=\"selected\" value=\"\">" + option + "</option>";
+ return "\n<option selected=\"selected\" value=\"\">" + escapeOptionBody(option)
+ + "</option>";
}
}
return "";
}
+ /**
+ * Escapes the body of the default option, the way
+ * {@code AbstractChoice#renderValue} escapes the body of every other option. The value
+ * is escaped here rather than in {@link #getNullValidDisplayValue()} and
+ * {@link #getNullKeyDisplayValue()} because those return a display value, which an override may
+ * compare against a resource it looks up itself.
+ */
+ private CharSequence escapeOptionBody(String option)
+ {
+ return getEscapeModelStrings() ? escapeOptionHtml(option) : option;
+ }
+
/**
* Returns the display value for the null value. The default behavior is to look the value up by
* using the key from <code>getNullValidKey()</code>.
diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/form/AbstractSingleSelectChoiceEscapeTest.java b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/AbstractSingleSelectChoiceEscapeTest.java
new file mode 100644
index 0000000000..dcb4a7da20
--- /dev/null
+++ b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/AbstractSingleSelectChoiceEscapeTest.java
@@ -0,0 +1,192 @@
+/*
+ * 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.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Arrays;
+
+import org.apache.wicket.MarkupContainer;
+import org.apache.wicket.markup.IMarkupResourceStreamProvider;
+import org.apache.wicket.markup.html.WebPage;
+import org.apache.wicket.model.Model;
+import org.apache.wicket.util.resource.IResourceStream;
+import org.apache.wicket.util.resource.StringResourceStream;
+import org.apache.wicket.util.tester.WicketTestCase;
+import org.junit.Test;
+
+/**
+ * The body of the default ("null") option of a single select choice is escaped the same way the body
+ * of every other option is. The display value is looked up by an overridable method, so it is not
+ * necessarily the plain text from a resource bundle that the default implementation returns.
+ */
+public class AbstractSingleSelectChoiceEscapeTest extends WicketTestCase
+{
+ private static final String MARKUP = "<script>x=1</script>";
+
+ private static final String ESCAPED = "<script>x=1</script>";
+
+ /**
+ * A choice whose null display values come from somewhere other than a bundle, as an application
+ * override does when it shows the field's label instead of the generic text.
+ */
+ private static class NullDisplayValueChoice extends DropDownChoice<String>
+ {
+ private static final long serialVersionUID = 1L;
+
+ private final String displayValue;
+
+ /** What the hooks handed back, to show escaping did not happen inside them. */
+ private String observedDisplayValue;
+
+ private NullDisplayValueChoice(String id, String displayValue)
+ {
+ super(id, Model.of((String)null), Arrays.asList("A", "B"));
+ this.displayValue = displayValue;
+ }
+
+ @Override
+ protected String getNullValidDisplayValue()
+ {
+ observedDisplayValue = displayValue;
+ return displayValue;
+ }
+
+ @Override
+ protected String getNullKeyDisplayValue()
+ {
+ observedDisplayValue = displayValue;
+ return displayValue;
+ }
+
+ @Override
+ protected CharSequence escapeOptionHtml(String value)
+ {
+ return "[" + super.escapeOptionHtml(value) + "]";
+ }
+ }
+
+ private static class ChoicePage extends WebPage implements IMarkupResourceStreamProvider
+ {
+ private static final long serialVersionUID = 1L;
+
+ private final NullDisplayValueChoice choice;
+
+ private ChoicePage(String displayValue, boolean nullValid)
+ {
+ Form<Void> form = new Form<>("form");
+ add(form);
+ choice = new NullDisplayValueChoice("dropdown", displayValue);
+ choice.setNullValid(nullValid);
+ form.add(choice);
+ }
+
+ @Override
+ public IResourceStream getMarkupResourceStream(MarkupContainer container,
+ Class<?> containerClass)
+ {
+ return new StringResourceStream(
+ "<html><body><form wicket:id=\"form\"><select wicket:id=\"dropdown\"></select>"
+ + "</form></body></html>");
+ }
+ }
+
+ /** {@code nullValid} is true, so the "nullValid" display value is rendered. */
+ @Test
+ public void nullValidDisplayValueIsEscaped()
+ {
+ tester.startPage(new ChoicePage(MARKUP, true));
+
+ String response = tester.getLastResponseAsString();
+ assertTrue("the default option body should be escaped",
+ response.contains(ESCAPED));
+ assertFalse("the default option body should not reach the markup as markup",
+ response.contains(MARKUP));
+ }
+
+ /** {@code nullValid} is false and nothing is selected, so the "null" display value is used. */
+ @Test
+ public void nullKeyDisplayValueIsEscaped()
+ {
+ tester.startPage(new ChoicePage(MARKUP, false));
+
+ String response = tester.getLastResponseAsString();
+ assertTrue("the default option body should be escaped",
+ response.contains(ESCAPED));
+ assertFalse("the default option body should not reach the markup as markup",
+ response.contains(MARKUP));
+ }
+
+ /**
+ * The escaping follows escapeModelStrings, like the other options, so an application that means
+ * to put markup in the default option can still do so.
+ */
+ @Test
+ public void defaultOptionIsNotEscapedWhenEscapeModelStringsIsFalse()
+ {
+ ChoicePage page = new ChoicePage(MARKUP, true);
+ page.choice.setEscapeModelStrings(false);
+ tester.startPage(page);
+
+ assertTrue("escaping is off, so the default option body should be written as is",
+ tester.getLastResponseAsString().contains(MARKUP));
+ }
+
+ /**
+ * The escape goes through {@code escapeOptionHtml}, so an application that customised the
+ * escaping of its options gets it applied to the default option too.
+ */
+ @Test
+ public void escapeOptionHtmlIsUsedForTheDefaultOption()
+ {
+ tester.startPage(new ChoicePage(MARKUP, true));
+
+ // the marker the override adds has to show up in the body of the default option itself.
+ // Asserting that the method was called says nothing, it is called for every other option
+ assertTrue("the default option should be escaped through escapeOptionHtml",
+ tester.getLastResponseAsString().contains("value=\"\">[" + ESCAPED + "]</option>"));
+ }
+
+ /**
+ * Escaping happens where the option is written, not inside the display value methods. An
+ * override that compares what super returns against a resource it looks up itself would stop
+ * matching if those methods escaped.
+ */
+ @Test
+ public void displayValueHooksAreNotEscapedThemselves()
+ {
+ ChoicePage page = new ChoicePage(MARKUP, true);
+ tester.startPage(page);
+
+ assertEquals("the display value hook should see and return unescaped text",
+ MARKUP, page.choice.observedDisplayValue);
+ assertTrue("while the rendered option body is escaped",
+ tester.getLastResponseAsString().contains(ESCAPED));
+ }
+
+ /** Plain text, which is what the shipped bundles hold, renders unchanged. */
+ @Test
+ public void plainDisplayValueRendersUnchanged()
+ {
+ tester.startPage(new ChoicePage("Choose one", true));
+
+ assertTrue("plain text should pass through the escaping unchanged",
+ tester.getLastResponseAsString().contains("value=\"\">[Choose one]</option>"));
+ }
+}
(wicket) 07/08: Document how model data, markup and message bundles are escaped
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
papegaaij pushed a commit to branch wicket-8.x
in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 629b79fbcb353ce4c146f5023d30fe19fdd10446
Author: Emond Papegaaij <pa...@apache.org>
AuthorDate: Wed Aug 19 22:26:29 2026 +0200
Document how model data, markup and message bundles are escaped
The security model said which inputs Wicket trusts and nothing about what it does
with them on the way out, so there was no written statement of the escaping
contract to measure a report against. It now says that escapeModelStrings is that
contract and is on by default, that clearing it hands the responsibility to the
application, and that a component writing model data to the markup unescaped in
the default configuration is a bug in the framework.
It also draws the line on the two inputs that are trusted, since both are authored
by the developer rather than received at runtime: markup files, and message
bundles, where wicket:message renders its value as markup unless the tag asks for
escaping. The value a bundle string interpolates is not covered by that, because
it is resolved from the surrounding component's model.
Finally it states what Strings#escapeMarkup is for. It escapes the five
characters that matter in element text and in a quoted attribute value, and it is
not enough anywhere else: not inside a script or style element, not in an unquoted
attribute, and not where a url scheme is the payload. Wicket does not escape for a
JavaScript context anywhere, so a value an application puts in one, through
TextTemplate substitution for instance, has to be encoded by the application.
Co-Authored-By: Claude Opus 5 (1M context) <no...@anthropic.com>
---
SECURITY.md | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/SECURITY.md b/SECURITY.md
index ffc7a91a2c..fd3239f80a 100644
--- a/SECURITY.md
+++ b/SECURITY.md
@@ -253,6 +253,55 @@ at must therefore be storage only the application itself can read and write.
The encrypted page store, and the trust statement that goes with it, exist on
9.x and 10.x only.
+### Model data is escaped; markup and message bundles are trusted
+
+Wicket escapes the text a component renders from its model. `Component`'s
+`escapeModelStrings` flag is **on by default**, and a component renders
+model-derived text either through `Component#getDefaultModelObjectAsString()` or
+by applying `Strings#escapeMarkup` when that flag is set. A component that writes
+application model data into the markup unescaped in the default configuration is
+a bug in the framework and an opening for cross-site scripting (XSS). We want to
+hear about it.
+
+`setEscapeModelStrings(false)` is the application saying the content is markup
+and taking responsibility for it. Reports that depend on an application having
+cleared the flag are configuration issues rather than framework vulnerabilities.
+Note that a few components clear it themselves because their value is written
+into an attribute, which is escaped when the tag is written and would otherwise
+be encoded twice; that is an implementation detail of those components and not an
+invitation to render untrusted markup through them.
+
+Two inputs on the other side of the boundary are trusted, because both are
+authored by the developer and neither is data the application received at
+runtime:
+
+- **Markup files are trusted.** A `.html` file on the classpath is a template,
+ exactly like a JSP or a Thymeleaf template, and Wicket renders it as markup. An
+ application that serves markup from somewhere an untrusted party can write —
+ through a custom `IMarkupResourceStreamProvider`, for instance — has taken
+ that trust on itself.
+- **Message bundles are trusted.** `<wicket:message key="…"/>` renders its
+ property value as markup by default, and `escape="true"` opts in to escaping.
+ Markup in a bundle is therefore a supported way to format a message.
+
+The value a bundle string interpolates is a different matter. `${name}` in a
+message resolves first to a child component with `wicket:id="name"`, whose
+rendered markup carries that component's own escaping. Only when there is no
+such child does Wicket fall back to reading `name` from the surrounding
+component's model, and that value is written as it came — so a static bundle can
+still place model data in the markup unescaped. Prefer the child component.
+Where the fallback is unavoidable and the data is not trusted, the message needs
+`escape="true"`, which escapes the whole message and therefore any markup the
+bundle itself contains.
+
+Finally, `Strings#escapeMarkup` escapes `<`, `>`, `&`, `"` and `'`. That is
+enough for element text and for a quoted attribute value, and it is not enough
+for anything else: it does not make a value safe inside `<script>` or `<style>`,
+in an unquoted attribute, or in a URL where the scheme itself is the payload.
+Wicket does not escape for a JavaScript context anywhere, so a value the
+application places in one — through `TextTemplate` variable substitution, for
+example — has to be encoded by the application.
+
### Another origin may not invoke a listener
Where `CsrfPreventionRequestCycleListener` is registered, a request originating