This is an automated email from the ASF dual-hosted git repository. papegaaij pushed a commit to branch wicket-9.x in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 34da4c600e6a502916cbad8cf40ce9cacb388392 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 0b28b5f5be..e3e4e9dc1f 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); @@ -254,6 +261,13 @@ public class AjaxEditableChoiceLabel<T> extends AjaxEditableLabel<T> return AjaxEditableChoiceLabel.this.getConverter(type); } + @Override + protected void onConfigure() + { + super.onConfigure(); + setEscapeModelStrings(AjaxEditableChoiceLabel.this.getEscapeModelStrings()); + } + /** * {@inheritDoc} */ @@ -268,15 +282,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 91ce9b62ff..2ba1befb4f 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 @@ -389,6 +389,13 @@ public class AjaxEditableLabel<T> extends Panel implements IGenericComponent<T, return AjaxEditableLabel.this.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 db8c2a4066..f38883d44e 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 @@ -93,6 +93,13 @@ public class AjaxEditableMultiLineLabel<T> extends AjaxEditableLabel<T> return AjaxEditableMultiLineLabel.this.getConverter(type); } + @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..02ea828168 --- /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.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.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.jupiter.api.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. + */ +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 + void choiceLabelWithRendererEscapesByDefault() + { + String document = render(new AjaxEditableChoiceLabel<>("label", Model.of(MARKUP), CHOICES, + new ChoiceRenderer<String>())); + + assertTrue(document.contains(ESCAPED), document); + assertFalse(document.contains(MARKUP), document); + } + + @Test + 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 + 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.contains(ESCAPED), document); + assertFalse(document.contains(MARKUP), document); + } + + /** + * Without a renderer the value is escaped by {@code getDefaultModelObjectAsString()}, so it + * must not be escaped a second time. + */ + @Test + void choiceLabelWithoutRendererEscapesExactlyOnce() + { + String document = render( + new AjaxEditableChoiceLabel<>("label", Model.of(MARKUP), CHOICES)); + + assertTrue(document.contains(ESCAPED), document); + assertFalse(document.contains(DOUBLE_ESCAPED), document); + } + + @Test + void choiceLabelWithoutRendererDoesNotEscapeWhenTurnedOff() + { + AjaxEditableChoiceLabel<String> label = new AjaxEditableChoiceLabel<>("label", + Model.of(MARKUP), CHOICES); + label.setEscapeModelStrings(false); + + assertTrue(render(label).contains(MARKUP)); + } + + @Test + void labelEscapesByDefault() + { + String document = render(new AjaxEditableLabel<>("label", Model.of(MARKUP))); + + assertTrue(document.contains(ESCAPED), document); + assertFalse(document.contains(MARKUP), document); + } + + @Test + void labelDoesNotEscapeWhenTurnedOff() + { + AjaxEditableLabel<String> label = new AjaxEditableLabel<>("label", Model.of(MARKUP)); + label.setEscapeModelStrings(false); + + assertTrue(render(label).contains(MARKUP)); + } + + @Test + void multiLineLabelEscapesByDefault() + { + String document = render(new AjaxEditableMultiLineLabel<>("label", Model.of(MARKUP))); + + assertTrue(document.contains(ESCAPED), document); + assertFalse(document.contains(MARKUP), document); + } + + @Test + 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 + 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 + void choiceEditorEscapesOptionsByDefault() + { + String document = renderEditor(new AjaxEditableChoiceLabel<>("label", Model.of(MARKUP), + CHOICES, new ChoiceRenderer<String>())); + + assertTrue(document.contains(ESCAPED), document); + assertFalse(document.contains(MARKUP), document); + } + + @Test + void choiceEditorDoesNotEscapeOptionsWhenTurnedOff() + { + AjaxEditableChoiceLabel<String> label = new AjaxEditableChoiceLabel<>("label", + Model.of(MARKUP), CHOICES, new ChoiceRenderer<String>()); + label.setEscapeModelStrings(false); + + String document = renderEditor(label); + + assertTrue(document.contains(MARKUP), document); + assertFalse(document.contains(ESCAPED), document); + } + + /** + * 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 + void multiLineEditorEscapesEvenWhenTurnedOff() + { + AjaxEditableMultiLineLabel<String> label = new AjaxEditableMultiLineLabel<>("label", + Model.of(MARKUP)); + label.setEscapeModelStrings(false); + + assertTrue(renderEditor(label).contains(ESCAPED)); + } +}
