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 4766658865d5377c2cd63b35cfbeca0feb4aa4a5 Author: Emond Papegaaij <[email protected]> AuthorDate: Tue Aug 18 14:59:01 2026 +0200 Respect escapeModelStrings in the editable labels and their editors The label of AjaxEditableChoiceLabel starts from getDefaultModelObjectAsString(), which escapes according to escapeModelStrings, and then replaces that value with the one the IChoiceRenderer returns. That value was written to the markup as it came, both where it is used directly and where it is first run through a converter, so a choice whose display value contains markup was rendered as markup. The same value in the dropdown was escaped, because AbstractChoice#renderValue escapes what the renderer returns. The escaping is applied to the value taken from the renderer only. The value it replaces has already been escaped by getDefaultModelObjectAsString(), so escaping where the body is replaced instead would escape that one a second time; choiceLabelWithoutRendererEscapesExactlyOnce covers it. escapeModelStrings had no effect on any of the three editable labels to begin with. It is read by the label component the panel renders with, not by the panel, and nothing carried it across, so calling setEscapeModelStrings on an AjaxEditableLabel, an AjaxEditableMultiLineLabel or an AjaxEditableChoiceLabel changed nothing. Component#getEscapeModelStrings is final and cannot be delegated, so each label component now reads the setting of the panel in onConfigure, beside the getConverter delegation already there. The editor of AjaxEditableChoiceLabel reads it the same way. The choices it renders are display values just like the label's, and escaping one but not the other shows the same value in two different ways depending on whether the label is in edit mode. The text editors of the other two are left as they are. TextField puts the value in an attribute, which is escaped when the tag is written whatever the setting says, so it makes no difference there. TextArea writes it into the element body, where the escaping is what makes the value round-trip: an unescaped body ends the textarea early at a </textarea> the value happens to contain. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --- .../ajax/markup/html/AjaxEditableChoiceLabel.java | 25 ++- .../ajax/markup/html/AjaxEditableLabel.java | 7 + .../markup/html/AjaxEditableMultiLineLabel.java | 7 + .../AjaxEditableLabelEscapeModelStringsTest.java | 246 +++++++++++++++++++++ 4 files changed, 283 insertions(+), 2 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 5dfa693ef3..431b790e7b 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 @@ -194,6 +194,13 @@ public class AjaxEditableChoiceLabel<T> extends AjaxEditableLabel<T> AjaxEditableChoiceLabel.this.onModelChanging(); } + @Override + protected void onConfigure() + { + super.onConfigure(); + setEscapeModelStrings(AjaxEditableChoiceLabel.this.getEscapeModelStrings()); + } + }; editor.setOutputMarkupId(true); @@ -255,6 +262,13 @@ public class AjaxEditableChoiceLabel<T> extends AjaxEditableLabel<T> return c != null ? c : super.getConverter(type); } + @Override + protected void onConfigure() + { + super.onConfigure(); + setEscapeModelStrings(AjaxEditableChoiceLabel.this.getEscapeModelStrings()); + } + /** * {@inheritDoc} */ @@ -269,15 +283,22 @@ public class AjaxEditableChoiceLabel<T> extends AjaxEditableLabel<T> Object displayObject = renderer.getDisplayValue(getModelObject()); Class<?> objectClass = (displayObject == null ? null : displayObject.getClass()); + String rendered = null; if ((objectClass != null) && (objectClass != String.class)) { @SuppressWarnings("rawtypes") final IConverter converter = getConverter(objectClass); - displayValue = converter.convertToString(displayObject, getLocale()); + rendered = converter.convertToString(displayObject, getLocale()); } else if (displayObject != null) { - displayValue = displayObject.toString(); + rendered = displayObject.toString(); + } + + if (rendered != null) + { + displayValue = getEscapeModelStrings() + ? Strings.escapeMarkup(rendered).toString() : rendered; } } 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 81b8436fb1..47329aa74e 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 @@ -401,6 +401,13 @@ public class AjaxEditableLabel<T> extends Panel implements IGenericComponent<T, return c != null ? c : super.getConverter(type); } + @Override + protected void onConfigure() + { + super.onConfigure(); + setEscapeModelStrings(AjaxEditableLabel.this.getEscapeModelStrings()); + } + @Override public void onComponentTagBody(final MarkupStream markupStream, final ComponentTag openTag) 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 712bc31f88..65e297d8ec 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 @@ -86,6 +86,13 @@ public class AjaxEditableMultiLineLabel<T> extends AjaxEditableLabel<T> { private static final long serialVersionUID = 1L; + @Override + protected void onConfigure() + { + super.onConfigure(); + setEscapeModelStrings(AjaxEditableMultiLineLabel.this.getEscapeModelStrings()); + } + @Override public void onComponentTagBody(final MarkupStream markupStream, final ComponentTag openTag) diff --git a/wicket-extensions/src/test/java/org/apache/wicket/extensions/ajax/markup/html/AjaxEditableLabelEscapeModelStringsTest.java b/wicket-extensions/src/test/java/org/apache/wicket/extensions/ajax/markup/html/AjaxEditableLabelEscapeModelStringsTest.java new file mode 100644 index 0000000000..04011d89d3 --- /dev/null +++ b/wicket-extensions/src/test/java/org/apache/wicket/extensions/ajax/markup/html/AjaxEditableLabelEscapeModelStringsTest.java @@ -0,0 +1,246 @@ +/* + * 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 java.util.List; + +import org.apache.wicket.Component; +import org.apache.wicket.behavior.AbstractAjaxBehavior; +import org.apache.wicket.markup.html.form.ChoiceRenderer; +import org.apache.wicket.markup.html.form.IChoiceRenderer; +import org.apache.wicket.model.IModel; +import org.apache.wicket.model.Model; +import org.apache.wicket.util.tester.WicketTestCase; +import org.junit.Test; + +/** + * Tests that the editable labels escape their model value by default, and that + * {@link Component#setEscapeModelStrings(boolean)} on the editable label itself is honoured + * rather than the setting of the label component it renders with. + */ +public class AjaxEditableLabelEscapeModelStringsTest extends WicketTestCase +{ + private static final String MARKUP = "<img src=x onerror=alert(1)>"; + + private static final String ESCAPED = "<img src=x onerror=alert(1)>"; + + private static final String DOUBLE_ESCAPED = "&lt;img"; + + private static final List<String> CHOICES = Arrays.asList(MARKUP, "other"); + + /** + * A display value that is not a String, so that it is rendered through an + * {@link org.apache.wicket.util.convert.IConverter} rather than by {@code toString()} directly. + */ + private static class DisplayValue + { + @Override + public String toString() + { + return MARKUP; + } + } + + private String render(Component component) + { + tester.startComponentInPage(component); + return tester.getLastResponse().getDocument(); + } + + /** + * Renders the editable label, then puts it in edit mode so that the response holds the markup + * of the editor instead of the label. + */ + private String renderEditor(AjaxEditableLabel<?> label) + { + tester.startComponentInPage(label); + AbstractAjaxBehavior labelBehavior = (AbstractAjaxBehavior)label.get("label") + .getBehaviors() + .get(0); + tester.executeBehavior(labelBehavior); + return tester.getLastResponse().getDocument(); + } + + @Test + public void choiceLabelWithRendererEscapesByDefault() + { + String document = render(new AjaxEditableChoiceLabel<>("label", Model.of(MARKUP), CHOICES, + new ChoiceRenderer<String>())); + + assertTrue(document, document.contains(ESCAPED)); + assertFalse(document, document.contains(MARKUP)); + } + + @Test + public void choiceLabelWithRendererDoesNotEscapeWhenTurnedOff() + { + AjaxEditableChoiceLabel<String> label = new AjaxEditableChoiceLabel<>("label", + Model.of(MARKUP), CHOICES, new ChoiceRenderer<String>()); + label.setEscapeModelStrings(false); + + assertTrue(render(label).contains(MARKUP)); + } + + /** + * A renderer returning a non-String display value is rendered through a converter, which does + * not escape either. + */ + @Test + public void choiceLabelWithConvertedDisplayValueEscapesByDefault() + { + IChoiceRenderer<String> renderer = new ChoiceRenderer<String>() + { + private static final long serialVersionUID = 1L; + + @Override + public Object getDisplayValue(String object) + { + return new DisplayValue(); + } + }; + + String document = render( + new AjaxEditableChoiceLabel<>("label", Model.of("value"), CHOICES, renderer)); + + assertTrue(document, document.contains(ESCAPED)); + assertFalse(document, document.contains(MARKUP)); + } + + /** + * Without a renderer the value is escaped by {@code getDefaultModelObjectAsString()}, so it + * must not be escaped a second time. + */ + @Test + public void choiceLabelWithoutRendererEscapesExactlyOnce() + { + String document = render( + new AjaxEditableChoiceLabel<>("label", Model.of(MARKUP), CHOICES)); + + assertTrue(document, document.contains(ESCAPED)); + assertFalse(document, document.contains(DOUBLE_ESCAPED)); + } + + @Test + public void choiceLabelWithoutRendererDoesNotEscapeWhenTurnedOff() + { + AjaxEditableChoiceLabel<String> label = new AjaxEditableChoiceLabel<>("label", + Model.of(MARKUP), CHOICES); + label.setEscapeModelStrings(false); + + assertTrue(render(label).contains(MARKUP)); + } + + @Test + public void labelEscapesByDefault() + { + String document = render(new AjaxEditableLabel<>("label", Model.of(MARKUP))); + + assertTrue(document, document.contains(ESCAPED)); + assertFalse(document, document.contains(MARKUP)); + } + + @Test + public void labelDoesNotEscapeWhenTurnedOff() + { + AjaxEditableLabel<String> label = new AjaxEditableLabel<>("label", Model.of(MARKUP)); + label.setEscapeModelStrings(false); + + assertTrue(render(label).contains(MARKUP)); + } + + @Test + public void multiLineLabelEscapesByDefault() + { + String document = render(new AjaxEditableMultiLineLabel<>("label", Model.of(MARKUP))); + + assertTrue(document, document.contains(ESCAPED)); + assertFalse(document, document.contains(MARKUP)); + } + + @Test + public void multiLineLabelDoesNotEscapeWhenTurnedOff() + { + AjaxEditableMultiLineLabel<String> label = new AjaxEditableMultiLineLabel<>("label", + Model.of(MARKUP)); + label.setEscapeModelStrings(false); + + assertTrue(render(label).contains(MARKUP)); + } + + /** + * The setting has to be picked up even when it is changed after the label component has been + * created. + */ + @Test + public void settingIsHonouredWhenChangedAfterLabelWasCreated() + { + IModel<String> model = Model.of(MARKUP); + AjaxEditableChoiceLabel<String> label = new AjaxEditableChoiceLabel<>("label", model, + CHOICES, new ChoiceRenderer<String>()); + // forces the internal label component to be created up front + label.setRequired(true); + label.setEscapeModelStrings(false); + + assertTrue(render(label).contains(MARKUP)); + } + + /** + * The choices rendered by the editor are display values just like the label, so they follow + * the same setting. Escaping one but not the other would show the same value in two different + * ways depending on whether the label is in edit mode. + */ + @Test + public void choiceEditorEscapesOptionsByDefault() + { + String document = renderEditor(new AjaxEditableChoiceLabel<>("label", Model.of(MARKUP), + CHOICES, new ChoiceRenderer<String>())); + + assertTrue(document, document.contains(ESCAPED)); + assertFalse(document, document.contains(MARKUP)); + } + + @Test + public void choiceEditorDoesNotEscapeOptionsWhenTurnedOff() + { + AjaxEditableChoiceLabel<String> label = new AjaxEditableChoiceLabel<>("label", + Model.of(MARKUP), CHOICES, new ChoiceRenderer<String>()); + label.setEscapeModelStrings(false); + + String document = renderEditor(label); + + assertTrue(document, document.contains(MARKUP)); + assertFalse(document, document.contains(ESCAPED)); + } + + /** + * The text editors are deliberately not covered by the setting: they carry the value as text + * to be edited, and a textarea body that is not escaped does not round-trip the value. + */ + @Test + public void multiLineEditorEscapesEvenWhenTurnedOff() + { + AjaxEditableMultiLineLabel<String> label = new AjaxEditableMultiLineLabel<>("label", + Model.of(MARKUP)); + label.setEscapeModelStrings(false); + + assertTrue(renderEditor(label).contains(ESCAPED)); + } +}
