diff --git a/spring-beans/src/testFixtures/java/org/springframework/beans/testfixture/beans/TestBean.java b/spring-beans/src/testFixtures/java/org/springframework/beans/testfixture/beans/TestBean.java index e4513a0fae4..76ea5a53fed 100644 --- a/spring-beans/src/testFixtures/java/org/springframework/beans/testfixture/beans/TestBean.java +++ b/spring-beans/src/testFixtures/java/org/springframework/beans/testfixture/beans/TestBean.java @@ -60,6 +60,8 @@ public class TestBean implements BeanNameAware, BeanFactoryAware, ITestBean, IOt private boolean jedi; + private String favoriteCafé; + private ITestBean spouse; private String touchy; @@ -210,6 +212,14 @@ public class TestBean implements BeanNameAware, BeanFactoryAware, ITestBean, IOt this.jedi = jedi; } + public String getFavoriteCafé() { + return this.favoriteCafé; + } + + public void setFavoriteCafé(String favoriteCafé) { + this.favoriteCafé = favoriteCafé; + } + @Override public ITestBean getSpouse() { return this.spouse; diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/form/OptionWriter.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/form/OptionWriter.java index 0d6831ecba4..5c61769fdf1 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/form/OptionWriter.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/form/OptionWriter.java @@ -28,6 +28,7 @@ import org.springframework.beans.PropertyAccessorFactory; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; import org.springframework.web.servlet.support.BindStatus; +import org.springframework.web.util.HtmlUtils; /** * Provides supporting functionality to render a list of '{@code option}' @@ -100,18 +101,25 @@ class OptionWriter { private final boolean htmlEscape; + private final @Nullable String encoding; + /** - * Create a new {@code OptionWriter} for the supplied {@code objectSource}. + * Create a new {@code OptionWriter} for the supplied {@code optionSource}. * @param optionSource the source of the {@code options} (never {@code null}) * @param bindStatus the {@link BindStatus} for the bound value (never {@code null}) * @param valueProperty the name of the property used to render {@code option} values * (optional) * @param labelProperty the name of the property used to render {@code option} labels * (optional) + * @param htmlEscape whether special characters should be converted into HTML + * character references + * @param encoding the character encoding to use, or {@code null} if response + * encoding should not be used with HTML escaping */ public OptionWriter(Object optionSource, BindStatus bindStatus, - @Nullable String valueProperty, @Nullable String labelProperty, boolean htmlEscape) { + @Nullable String valueProperty, @Nullable String labelProperty, + boolean htmlEscape, @Nullable String encoding) { Assert.notNull(optionSource, "'optionSource' must not be null"); Assert.notNull(bindStatus, "'bindStatus' must not be null"); @@ -120,6 +128,7 @@ class OptionWriter { this.valueProperty = valueProperty; this.labelProperty = labelProperty; this.htmlEscape = htmlEscape; + this.encoding = encoding; } @@ -248,7 +257,14 @@ class OptionWriter { */ private String getDisplayString(@Nullable Object value) { PropertyEditor editor = (value != null ? this.bindStatus.findEditor(value.getClass()) : null); - return ValueFormatter.getDisplayString(value, editor, this.htmlEscape); + String displayString = ValueFormatter.getDisplayString(value, editor, false); + return (this.htmlEscape ? htmlEscape(displayString) : displayString); + } + + private String htmlEscape(String content) { + return (this.encoding != null ? + HtmlUtils.htmlEscape(content, this.encoding) : + HtmlUtils.htmlEscape(content)); } /** diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/form/OptionsTag.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/form/OptionsTag.java index 8d1b6490a40..8c4ed9627ac 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/form/OptionsTag.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/form/OptionsTag.java @@ -187,6 +187,7 @@ import org.springframework.web.util.TagUtils; * @author Rob Harrop * @author Juergen Hoeller * @author Scott Andrews + * @author Sam Brannen * @since 2.0 */ @SuppressWarnings("serial") @@ -306,7 +307,10 @@ public class OptionsTag extends AbstractHtmlElementTag { (itemValue != null ? ObjectUtils.getDisplayString(evaluate("itemValue", itemValue)) : null); String labelProperty = (itemLabel != null ? ObjectUtils.getDisplayString(evaluate("itemLabel", itemLabel)) : null); - OptionsWriter optionWriter = new OptionsWriter(selectName, itemsObject, valueProperty, labelProperty); + String encodingToUse = + (isResponseEncodedHtmlEscape() ? this.pageContext.getResponse().getCharacterEncoding() : null); + OptionsWriter optionWriter = + new OptionsWriter(selectName, itemsObject, valueProperty, labelProperty, encodingToUse); optionWriter.writeOptions(tagWriter); } return SKIP_BODY; @@ -345,9 +349,9 @@ public class OptionsTag extends AbstractHtmlElementTag { private final @Nullable String selectName; public OptionsWriter(@Nullable String selectName, Object optionSource, - @Nullable String valueProperty, @Nullable String labelProperty) { + @Nullable String valueProperty, @Nullable String labelProperty, @Nullable String encoding) { - super(optionSource, getBindStatus(), valueProperty, labelProperty, isHtmlEscape()); + super(optionSource, getBindStatus(), valueProperty, labelProperty, isHtmlEscape(), encoding); this.selectName = selectName; } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/form/SelectTag.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/form/SelectTag.java index 20fea9d9d62..ad6ad0550ba 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/form/SelectTag.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/form/SelectTag.java @@ -235,6 +235,7 @@ import org.springframework.web.servlet.support.BindStatus; * * @author Rob Harrop * @author Juergen Hoeller + * @author Sam Brannen * @since 2.0 * @see OptionTag */ @@ -407,8 +408,12 @@ public class SelectTag extends AbstractHtmlInputElementTag { ObjectUtils.getDisplayString(evaluate("itemValue", getItemValue())) : null); String labelProperty = (getItemLabel() != null ? ObjectUtils.getDisplayString(evaluate("itemLabel", getItemLabel())) : null); + String encodingToUse = (isResponseEncodedHtmlEscape() ? + this.pageContext.getResponse().getCharacterEncoding() : null); OptionWriter optionWriter = - new OptionWriter(itemsObject, getBindStatus(), valueProperty, labelProperty, isHtmlEscape()) { + new OptionWriter(itemsObject, getBindStatus(), valueProperty, labelProperty, + isHtmlEscape(), encodingToUse) { + @Override protected String processOptionValue(String resolvedValue) { return processFieldValue(selectName, resolvedValue, "option"); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/tags/form/OptionsTagTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/tags/form/OptionsTagTests.java index 9f969adbae5..ac715358023 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/tags/form/OptionsTagTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/tags/form/OptionsTagTests.java @@ -21,6 +21,7 @@ import java.io.StringReader; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -50,6 +51,7 @@ import static org.assertj.core.api.Assertions.assertThat; * @author Juergen Hoeller * @author Scott Andrews * @author Jeremy Grelle + * @author Sam Brannen */ @SuppressWarnings({ "rawtypes", "unchecked" }) class OptionsTagTests extends AbstractHtmlElementTagTests { @@ -115,6 +117,86 @@ class OptionsTagTests extends AbstractHtmlElementTagTests { assertThat(element.attribute("onclick").getValue()).isEqualTo("CLICK"); } + @Test // gh-35783 + void withListWithHtmlEscaping() throws Exception { + getPageContext().setAttribute( + SelectTag.LIST_VALUE_PAGE_ATTRIBUTE, new BindStatus(getRequestContext(), "testBean.country", false)); + + this.tag.setItems(List.of("café", "Jane \"I Love Cafés\" Smith")); + this.tag.setId("myOption"); + + var expectedOutput = """ + + + """.replace("\n", ""); + + assertThat(this.tag.doStartTag()).isEqualTo(Tag.SKIP_BODY); + assertThat(getOutput()).isEqualTo(expectedOutput); + } + + @Test // gh-35783 + void withListWithHtmlEscapingAndCharacterEncoding() throws Exception { + this.getPageContext().getResponse().setCharacterEncoding("UTF-8"); + + getPageContext().setAttribute( + SelectTag.LIST_VALUE_PAGE_ATTRIBUTE, new BindStatus(getRequestContext(), "testBean.country", false)); + + this.tag.setItems(List.of("café", "Jane \"I Love Cafés\" Smith")); + this.tag.setId("myOption"); + + var expectedOutput = """ + + + """.replace("\n", ""); + + assertThat(this.tag.doStartTag()).isEqualTo(Tag.SKIP_BODY); + assertThat(getOutput()).isEqualTo(expectedOutput); + } + + @Test // gh-35783 + void withMapWithHtmlEscaping() throws Exception { + getPageContext().setAttribute( + SelectTag.LIST_VALUE_PAGE_ATTRIBUTE, new BindStatus(getRequestContext(), "testBean.country", false)); + + var map = new LinkedHashMap(); + map.put("one", "Jane \"I Love Cafés\" Smith"); + map.put("two", "Joe Café"); + + this.tag.setItems(map); + this.tag.setId("myOption"); + + var expectedOutput = """ + + + """.replace("\n", ""); + + assertThat(this.tag.doStartTag()).isEqualTo(Tag.SKIP_BODY); + assertThat(getOutput()).isEqualTo(expectedOutput); + } + + @Test // gh-35783 + void withMapWithHtmlEscapingAndCharacterEncoding() throws Exception { + this.getPageContext().getResponse().setCharacterEncoding("UTF-8"); + + getPageContext().setAttribute( + SelectTag.LIST_VALUE_PAGE_ATTRIBUTE, new BindStatus(getRequestContext(), "testBean.country", false)); + + var map = new LinkedHashMap(); + map.put("one", "Jane \"I Love Cafés\" Smith"); + map.put("two", "Joe Café"); + + this.tag.setItems(map); + this.tag.setId("myOption"); + + var expectedOutput = """ + + + """.replace("\n", ""); + + assertThat(this.tag.doStartTag()).isEqualTo(Tag.SKIP_BODY); + assertThat(getOutput()).isEqualTo(expectedOutput); + } + @Test void withCollectionAndDynamicAttributes() throws Exception { String dynamicAttribute1 = "attr1"; diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/tags/form/SelectTagTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/tags/form/SelectTagTests.java index 81dc5ad2323..495a3c45c82 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/tags/form/SelectTagTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/tags/form/SelectTagTests.java @@ -23,6 +23,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -54,6 +55,7 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; * @author Juergen Hoeller * @author Jeremy Grelle * @author Dave Syer + * @author Sam Brannen */ @SuppressWarnings({ "rawtypes", "unchecked" }) public class SelectTagTests extends AbstractFormTagTests { @@ -78,6 +80,18 @@ public class SelectTagTests extends AbstractFormTagTests { this.tag.setPageContext(getPageContext()); } + @Override + protected TestBean createTestBean() { + this.bean = new TestBeanWithRealCountry(); + this.bean.setName("Rob"); + this.bean.setCountry("UK"); + this.bean.setSex("M"); + this.bean.setMyFloat(Float.valueOf("12.34")); + this.bean.setSomeIntegerArray(new Integer[]{12, 34}); + return this.bean; + } + + @Test void dynamicAttributes() throws JspException { String dynamicAttribute1 = "attr1"; @@ -133,6 +147,78 @@ public class SelectTagTests extends AbstractFormTagTests { assertList(true); } + @Test // gh-33023 + void withListWithHtmlEscapingInPath() throws Exception { + this.tag.setPath("favoriteCafé"); + this.tag.setItems(List.of("Cup of Joe", "Jane's Coffee Shop")); + this.tag.setSize("2"); + + var expectedOutput = """ + + """.replace("\n", ""); + + assertThat(this.tag.doStartTag()).isEqualTo(Tag.SKIP_BODY); + assertThat(getOutput()).isEqualTo(expectedOutput); + } + + @Test // gh-33023 + void withListWithHtmlEscapingAndCharacterEncodingInPath() throws Exception { + this.getPageContext().getResponse().setCharacterEncoding("UTF-8"); + + this.tag.setPath("favoriteCafé"); + this.tag.setItems(List.of("Cup of Joe", "Jane's Coffee Shop")); + this.tag.setSize("2"); + + var expectedOutput = """ + + """.replace("\n", ""); + + assertThat(this.tag.doStartTag()).isEqualTo(Tag.SKIP_BODY); + assertThat(getOutput()).isEqualTo(expectedOutput); + } + + @Test // gh-35783 + void withListWithHtmlEscapingInOptions() throws Exception { + this.tag.setPath("name"); + this.tag.setItems(List.of("café", "Jane \"I Love Cafés\" Smith")); + this.tag.setSize("2"); + + var expectedOutput = """ + + """.replace("\n", ""); + + assertThat(this.tag.doStartTag()).isEqualTo(Tag.SKIP_BODY); + assertThat(getOutput()).isEqualTo(expectedOutput); + } + + @Test // gh-35783 + void withListWithHtmlEscapingAndCharacterEncodingInOptions() throws Exception { + this.getPageContext().getResponse().setCharacterEncoding("UTF-8"); + + this.tag.setPath("name"); + this.tag.setItems(List.of("café", "Jane \"I Love Cafés\" Smith")); + this.tag.setSize("2"); + + var expectedOutput = """ + + """.replace("\n", ""); + + assertThat(this.tag.doStartTag()).isEqualTo(Tag.SKIP_BODY); + assertThat(getOutput()).isEqualTo(expectedOutput); + } + @Test void withResolvedList() throws Exception { this.tag.setPath("country"); @@ -336,8 +422,58 @@ public class SelectTagTests extends AbstractFormTagTests { void withMap() throws Exception { this.tag.setPath("sex"); this.tag.setItems(getSexes()); - int result = this.tag.doStartTag(); - assertThat(result).isEqualTo(Tag.SKIP_BODY); + + var expectedOutput = """ + + """.replace("\n", ""); + + assertThat(this.tag.doStartTag()).isEqualTo(Tag.SKIP_BODY); + assertThat(getOutput()).isEqualTo(expectedOutput); + } + + @Test // gh-35783 + void withMapWithHtmlEscapingInOptions() throws Exception { + var map = new LinkedHashMap(); + map.put("F", "Jane \"I Love Cafés\" Smith"); + map.put("M", "Joe Café"); + + this.tag.setPath("sex"); + this.tag.setItems(map); + + var expectedOutput = """ + + """.replace("\n", ""); + + assertThat(this.tag.doStartTag()).isEqualTo(Tag.SKIP_BODY); + assertThat(getOutput()).isEqualTo(expectedOutput); + } + + @Test // gh-35783 + void withMapWithHtmlEscapingAndCharacterEncodingInOptions() throws Exception { + this.getPageContext().getResponse().setCharacterEncoding("UTF-8"); + + var map = new LinkedHashMap(); + map.put("F", "Jane \"I Love Cafés\" Smith"); + map.put("M", "Joe Café"); + + this.tag.setPath("sex"); + this.tag.setItems(map); + + var expectedOutput = """ + + """.replace("\n", ""); + + assertThat(this.tag.doStartTag()).isEqualTo(Tag.SKIP_BODY); + assertThat(getOutput()).isEqualTo(expectedOutput); } @Test @@ -959,7 +1095,7 @@ public class SelectTagTests extends AbstractFormTagTests { } private Map getSexes() { - Map sexes = new HashMap<>(); + Map sexes = new LinkedHashMap<>(); sexes.put("F", "Female"); sexes.put("M", "Male"); return sexes; @@ -997,17 +1133,6 @@ public class SelectTagTests extends AbstractFormTagTests { } } - @Override - protected TestBean createTestBean() { - this.bean = new TestBeanWithRealCountry(); - this.bean.setName("Rob"); - this.bean.setCountry("UK"); - this.bean.setSex("M"); - this.bean.setMyFloat(Float.valueOf("12.34")); - this.bean.setSomeIntegerArray(new Integer[]{12, 34}); - return this.bean; - } - private TestBean getTestBean() { return (TestBean) getPageContext().getRequest().getAttribute(COMMAND_NAME); }