From f5a6c2e74ba110917a47954bf0aba0bdb113eb8f Mon Sep 17 00:00:00 2001 From: Maciej Przepióra Date: Wed, 16 Sep 2015 10:48:31 +0200 Subject: Support HTML entities when reading/writing declarative format #18882 Change-Id: I08099673c5dd0d688d3243e5fd169edb05f046f3 --- server/src/com/vaadin/ui/AbstractSelect.java | 10 +- server/src/com/vaadin/ui/Button.java | 14 +- server/src/com/vaadin/ui/Grid.java | 10 +- server/src/com/vaadin/ui/Label.java | 14 +- server/src/com/vaadin/ui/OptionGroup.java | 9 ++ server/src/com/vaadin/ui/Table.java | 12 +- server/src/com/vaadin/ui/TextArea.java | 5 +- .../com/vaadin/ui/declarative/DesignFormatter.java | 51 +++++++ .../tests/design/DeclarativeTestBaseBase.java | 19 ++- .../AbstractSelectDeclarativeTest.java | 22 +++ .../OptionGroupDeclarativeTests.java | 6 +- .../component/button/ButtonDeclarativeTest.java | 39 ++++++ .../GridHeaderFooterDeclarativeTest.java | 152 ++++++++++++++++----- .../declarative/GridInlineDataDeclarativeTest.java | 24 ++++ .../component/label/LabelDeclarativeTest.java | 36 ++++- .../component/table/TableDeclarativeTest.java | 28 ++++ .../textarea/TextAreaDeclarativeTest.java | 22 +++ 17 files changed, 409 insertions(+), 64 deletions(-) (limited to 'server') diff --git a/server/src/com/vaadin/ui/AbstractSelect.java b/server/src/com/vaadin/ui/AbstractSelect.java index fdf819f031..bbd486a8f9 100644 --- a/server/src/com/vaadin/ui/AbstractSelect.java +++ b/server/src/com/vaadin/ui/AbstractSelect.java @@ -55,6 +55,7 @@ import com.vaadin.shared.ui.dd.VerticalDropLocation; import com.vaadin.ui.declarative.DesignAttributeHandler; import com.vaadin.ui.declarative.DesignContext; import com.vaadin.ui.declarative.DesignException; +import com.vaadin.ui.declarative.DesignFormatter; /** *

@@ -2232,12 +2233,13 @@ public abstract class AbstractSelect extends AbstractField implements } String itemId; + String caption = DesignFormatter.unencodeFromTextNode(child.html()); if (child.hasAttr("item-id")) { itemId = child.attr("item-id"); addItem(itemId); - setItemCaption(itemId, child.html()); + setItemCaption(itemId, caption); } else { - addItem(itemId = child.html()); + addItem(itemId = caption); } if (child.hasAttr("icon")) { @@ -2300,10 +2302,10 @@ public abstract class AbstractSelect extends AbstractField implements String caption = getItemCaption(itemId); if (caption != null && !caption.equals(itemId.toString())) { - element.html(caption); + element.html(DesignFormatter.encodeForTextNode(caption)); element.attr("item-id", itemId.toString()); } else { - element.html(itemId.toString()); + element.html(DesignFormatter.encodeForTextNode(itemId.toString())); } Resource icon = getItemIcon(itemId); diff --git a/server/src/com/vaadin/ui/Button.java b/server/src/com/vaadin/ui/Button.java index 6beb6ed686..c272662740 100644 --- a/server/src/com/vaadin/ui/Button.java +++ b/server/src/com/vaadin/ui/Button.java @@ -41,6 +41,7 @@ import com.vaadin.shared.ui.button.ButtonState; import com.vaadin.ui.Component.Focusable; import com.vaadin.ui.declarative.DesignAttributeHandler; import com.vaadin.ui.declarative.DesignContext; +import com.vaadin.ui.declarative.DesignFormatter; import com.vaadin.util.ReflectTools; /** @@ -676,14 +677,19 @@ public class Button extends AbstractComponent implements public void readDesign(Element design, DesignContext designContext) { super.readDesign(design, designContext); Attributes attr = design.attributes(); - String content = design.html(); - setCaption(content); + String content; // plain-text (default is html) Boolean plain = DesignAttributeHandler.readAttribute( DESIGN_ATTR_PLAIN_TEXT, attr, Boolean.class); if (plain == null || !plain) { setHtmlContentAllowed(true); + content = design.html(); + } else { + // content is not intended to be interpreted as HTML, + // so html entities need to be decoded + content = DesignFormatter.unencodeFromTextNode(design.html()); } + setCaption(content); if (attr.hasKey("icon-alt")) { setIconAlternateText(DesignAttributeHandler.readAttribute( "icon-alt", attr, String.class)); @@ -733,6 +739,10 @@ public class Button extends AbstractComponent implements // plain-text (default is html) if (!isHtmlContentAllowed()) { design.attr(DESIGN_ATTR_PLAIN_TEXT, ""); + // encode HTML entities + if (content != null) { + design.html(DesignFormatter.encodeForTextNode(content)); + } } // icon-alt DesignAttributeHandler.writeAttribute("icon-alt", attr, diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index c57d9d523b..12d800c1a6 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -100,6 +100,7 @@ import com.vaadin.shared.util.SharedUtil; import com.vaadin.ui.declarative.DesignAttributeHandler; import com.vaadin.ui.declarative.DesignContext; import com.vaadin.ui.declarative.DesignException; +import com.vaadin.ui.declarative.DesignFormatter; import com.vaadin.ui.renderers.HtmlRenderer; import com.vaadin.ui.renderers.Renderer; import com.vaadin.ui.renderers.TextRenderer; @@ -1928,7 +1929,9 @@ public class Grid extends AbstractComponent implements SelectionNotifier, setHtml(cellElement.html()); } } else { - setText(cellElement.html()); + // text – need to unescape HTML entities + setText(DesignFormatter.unencodeFromTextNode(cellElement + .html())); } } } @@ -4607,7 +4610,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /* * This method is a workaround for the fact that Vaadin re-applies * widget dimensions (height/width) on each state change event. The - * original design was to have setHeight an setHeightByRow be equals, + * original design was to have setHeight and setHeightByRow be equals, * and whichever was called the latest was considered in effect. * * But, because of Vaadin always calling setHeight on the widget, this @@ -6252,7 +6255,8 @@ public class Grid extends AbstractComponent implements SelectionNotifier, Object value = datasource.getItem(itemId) .getItemProperty(c.getPropertyId()).getValue(); tableRow.appendElement("td").append( - (value != null ? value.toString() : "")); + (value != null ? DesignFormatter + .encodeForTextNode(value.toString()) : "")); } } } diff --git a/server/src/com/vaadin/ui/Label.java b/server/src/com/vaadin/ui/Label.java index a6ee11bdd5..0ead70933b 100644 --- a/server/src/com/vaadin/ui/Label.java +++ b/server/src/com/vaadin/ui/Label.java @@ -31,6 +31,7 @@ import com.vaadin.shared.ui.label.ContentMode; import com.vaadin.shared.ui.label.LabelState; import com.vaadin.shared.util.SharedUtil; import com.vaadin.ui.declarative.DesignContext; +import com.vaadin.ui.declarative.DesignFormatter; /** * Label component for showing non-editable short texts. @@ -585,14 +586,18 @@ public class Label extends AbstractComponent implements Property, public void readDesign(Element design, DesignContext designContext) { super.readDesign(design, designContext); String innerHtml = design.html(); - if (innerHtml != null && !"".equals(innerHtml)) { - setValue(innerHtml); - } - if (design.hasAttr(DESIGN_ATTR_PLAIN_TEXT)) { + boolean plainText = design.hasAttr(DESIGN_ATTR_PLAIN_TEXT); + if (plainText) { setContentMode(ContentMode.TEXT); } else { setContentMode(ContentMode.HTML); } + if (innerHtml != null && !"".equals(innerHtml)) { + if (plainText) { + innerHtml = DesignFormatter.unencodeFromTextNode(innerHtml); + } + setValue(innerHtml); + } } /* @@ -625,6 +630,7 @@ public class Label extends AbstractComponent implements Property, // plain-text (default is html) if (getContentMode() == ContentMode.TEXT) { design.attr(DESIGN_ATTR_PLAIN_TEXT, ""); + design.html(DesignFormatter.encodeForTextNode(getValue())); } } } diff --git a/server/src/com/vaadin/ui/OptionGroup.java b/server/src/com/vaadin/ui/OptionGroup.java index 10c263aca0..183fc36500 100644 --- a/server/src/com/vaadin/ui/OptionGroup.java +++ b/server/src/com/vaadin/ui/OptionGroup.java @@ -33,6 +33,7 @@ import com.vaadin.server.PaintException; import com.vaadin.server.PaintTarget; import com.vaadin.shared.ui.optiongroup.OptionGroupConstants; import com.vaadin.ui.declarative.DesignContext; +import com.vaadin.ui.declarative.DesignFormatter; /** * Configures select to be used as an option group. @@ -276,6 +277,14 @@ public class OptionGroup extends AbstractSelect implements if (!isItemEnabled(itemId)) { elem.attr("disabled", ""); } + if (isHtmlContentAllowed()) { + // need to unencode HTML entities. AbstractSelect.writeDesign can't + // check if HTML content is allowed, so it always encodes entities + // like '>', '<' and '&'; in case HTML content is allowed this is + // undesirable so we need to unencode entities. Entities other than + // '<' and '>' will be taken care by Jsoup. + elem.html(DesignFormatter.unencodeFromTextNode(elem.html())); + } return elem; } diff --git a/server/src/com/vaadin/ui/Table.java b/server/src/com/vaadin/ui/Table.java index 42c4beab6c..2ee34cc094 100644 --- a/server/src/com/vaadin/ui/Table.java +++ b/server/src/com/vaadin/ui/Table.java @@ -69,6 +69,7 @@ import com.vaadin.shared.util.SharedUtil; import com.vaadin.ui.declarative.DesignAttributeHandler; import com.vaadin.ui.declarative.DesignContext; import com.vaadin.ui.declarative.DesignException; +import com.vaadin.ui.declarative.DesignFormatter; /** *

@@ -278,7 +279,7 @@ public class Table extends AbstractSelect implements Action.Container, ICON_ONLY(ItemCaptionMode.ICON_ONLY), /** * Row caption mode: Item captions are read from property specified with - * {@link #setItemCaptionPropertyId(Object)}. + * {@link #setItemCaptionPropertyId(Object)} . */ PROPERTY(ItemCaptionMode.PROPERTY); @@ -2043,7 +2044,9 @@ public class Table extends AbstractSelect implements Action.Container, } else if (minPageBufferIndex < pageBufferFirstIndex) { newCachedRowCount -= pageBufferFirstIndex - minPageBufferIndex; } - /* calculate the internal location of the new rows within the new cache */ + /* + * calculate the internal location of the new rows within the new cache + */ int firstIndexInNewPageBuffer = firstIndex - pageBufferFirstIndex - rowsFromBeginning; @@ -6113,7 +6116,8 @@ public class Table extends AbstractSelect implements Action.Container, } Iterator propertyIt = propertyIds.iterator(); for (Element e : elems) { - String columnValue = e.html(); + String columnValue = DesignFormatter.unencodeFromTextNode(e + .html()); Object propertyId = propertyIt.next(); if (header) { setColumnHeader(propertyId, columnValue); @@ -6154,7 +6158,7 @@ public class Table extends AbstractSelect implements Action.Container, } Object[] data = new String[cells.size()]; for (int c = 0; c < cells.size(); ++c) { - data[c] = cells.get(c).html(); + data[c] = DesignFormatter.unencodeFromTextNode(cells.get(c).html()); } Object itemId = addItem(data, diff --git a/server/src/com/vaadin/ui/TextArea.java b/server/src/com/vaadin/ui/TextArea.java index b4dfb209e8..75ecc19d40 100644 --- a/server/src/com/vaadin/ui/TextArea.java +++ b/server/src/com/vaadin/ui/TextArea.java @@ -21,6 +21,7 @@ import org.jsoup.nodes.Element; import com.vaadin.data.Property; import com.vaadin.shared.ui.textarea.TextAreaState; import com.vaadin.ui.declarative.DesignContext; +import com.vaadin.ui.declarative.DesignFormatter; /** * A text field that supports multi line editing. @@ -145,7 +146,7 @@ public class TextArea extends AbstractTextField { @Override public void readDesign(Element design, DesignContext designContext) { super.readDesign(design, designContext); - setValue(design.html()); + setValue(DesignFormatter.unencodeFromTextNode(design.html())); } /* @@ -157,7 +158,7 @@ public class TextArea extends AbstractTextField { @Override public void writeDesign(Element design, DesignContext designContext) { super.writeDesign(design, designContext); - design.html(getValue()); + design.html(DesignFormatter.encodeForTextNode(getValue())); } @Override diff --git a/server/src/com/vaadin/ui/declarative/DesignFormatter.java b/server/src/com/vaadin/ui/declarative/DesignFormatter.java index b1d2520631..3f21db8288 100644 --- a/server/src/com/vaadin/ui/declarative/DesignFormatter.java +++ b/server/src/com/vaadin/ui/declarative/DesignFormatter.java @@ -28,12 +28,15 @@ import java.util.Set; import java.util.TimeZone; import java.util.concurrent.ConcurrentHashMap; +import org.jsoup.parser.Parser; + import com.vaadin.data.util.converter.Converter; import com.vaadin.data.util.converter.StringToBigDecimalConverter; import com.vaadin.data.util.converter.StringToDoubleConverter; import com.vaadin.data.util.converter.StringToFloatConverter; import com.vaadin.event.ShortcutAction; import com.vaadin.server.Resource; +import com.vaadin.ui.AbstractSelect; import com.vaadin.ui.declarative.converters.DesignDateConverter; import com.vaadin.ui.declarative.converters.DesignEnumConverter; import com.vaadin.ui.declarative.converters.DesignObjectConverter; @@ -356,4 +359,52 @@ public class DesignFormatter implements Serializable { return findConverterFor(sourceType, false); } + /** + *

+ * Encodes some special characters in a given input String to make + * it ready to be written as contents of a text node. WARNING: this will + * e.g. encode "<someTag>" to "&lt;someTag&gt;" as this method + * doesn't do any parsing and assumes that there are no intended HTML + * elements in the input. Only some entities are actually encoded: + * &,<, > It's assumed that other entities are taken care of by + * Jsoup. + *

+ *

+ * Typically, this method will be used by components to encode data (like + * option items in {@link AbstractSelect}) when dumping to HTML format + *

+ * + * @since + * @param input + * String to be encoded + * @return String with &,< and > replaced with their HTML entities + */ + public static String encodeForTextNode(String input) { + if (input == null) { + return null; + } + return input.replace("&", "&").replace(">", ">") + .replace("<", "<"); + } + + /** + *

+ * Decodes HTML entities in a text from text node and replaces them with + * actual characters. + *

+ * + *

+ * Typically this method will be used by components to read back data (like + * option items in {@link AbstractSelect}) from HTML. Note that this method + * unencodes more characters than {@link #encodeForTextNode(String)} encodes + *

+ * + * @since + * @param input + * @return + */ + public static String unencodeFromTextNode(String input) { + return Parser.unescapeEntities(input, false); + } + } diff --git a/server/tests/src/com/vaadin/tests/design/DeclarativeTestBaseBase.java b/server/tests/src/com/vaadin/tests/design/DeclarativeTestBaseBase.java index f0714ef3bd..635b3e37cb 100644 --- a/server/tests/src/com/vaadin/tests/design/DeclarativeTestBaseBase.java +++ b/server/tests/src/com/vaadin/tests/design/DeclarativeTestBaseBase.java @@ -36,6 +36,18 @@ import com.vaadin.ui.declarative.DesignContext; import com.vaadin.ui.declarative.ShouldWriteDataDelegate; public abstract class DeclarativeTestBaseBase { + private static final class AlwaysWriteDelegate implements + ShouldWriteDataDelegate { + private static final long serialVersionUID = -6345914431997793599L; + + @Override + public boolean shouldWriteData(Component component) { + return true; + } + } + + public static final ShouldWriteDataDelegate ALWAYS_WRITE_DATA = new AlwaysWriteDelegate(); + public interface EqualsAsserter { public void assertObjectEquals(TT o1, TT o2); } @@ -55,12 +67,7 @@ public abstract class DeclarativeTestBaseBase { DesignContext dc = new DesignContext(); if (writeData) { - dc.setShouldWriteDataDelegate(new ShouldWriteDataDelegate() { - @Override - public boolean shouldWriteData(Component component) { - return true; - } - }); + dc.setShouldWriteDataDelegate(DeclarativeTestBaseBase.ALWAYS_WRITE_DATA); } dc.setRootComponent(object); Design.write(dc, outputStream); diff --git a/server/tests/src/com/vaadin/tests/server/component/abstractselect/AbstractSelectDeclarativeTest.java b/server/tests/src/com/vaadin/tests/server/component/abstractselect/AbstractSelectDeclarativeTest.java index b3867a7a3a..6bb931ad78 100644 --- a/server/tests/src/com/vaadin/tests/server/component/abstractselect/AbstractSelectDeclarativeTest.java +++ b/server/tests/src/com/vaadin/tests/server/component/abstractselect/AbstractSelectDeclarativeTest.java @@ -26,6 +26,7 @@ import com.vaadin.data.util.IndexedContainer; import com.vaadin.server.ExternalResource; import com.vaadin.server.Resource; import com.vaadin.tests.design.DeclarativeTestBase; +import com.vaadin.tests.design.DeclarativeTestBaseBase; import com.vaadin.ui.AbstractSelect; import com.vaadin.ui.AbstractSelect.ItemCaptionMode; import com.vaadin.ui.ComboBox; @@ -257,6 +258,27 @@ public class AbstractSelectDeclarativeTest extends || "true".equals(e.attr("multi-select"))); } + @Test + public void testHtmlEntities() { + String design = "" + + " " + + " " + ""; + AbstractSelect read = read(design); + + Assert.assertEquals("> One", read.getItemCaption("one")); + + AbstractSelect underTest = new ComboBox(); + underTest.addItem("> One"); + + Element root = new Element(Tag.valueOf("v-combo-box"), ""); + DesignContext dc = new DesignContext(); + dc.setShouldWriteDataDelegate(DeclarativeTestBaseBase.ALWAYS_WRITE_DATA); + underTest.writeDesign(root, dc); + + Assert.assertEquals("> One", root.getElementsByTag("option").first() + .html()); + } + public ComboBox createSingleSelectWithOnlyAttributes() { ComboBox cb = new ComboBox(); Container dataSource = new IndexedContainer(); diff --git a/server/tests/src/com/vaadin/tests/server/component/abstractselect/OptionGroupDeclarativeTests.java b/server/tests/src/com/vaadin/tests/server/component/abstractselect/OptionGroupDeclarativeTests.java index 4d75e0b59f..93b565eddd 100644 --- a/server/tests/src/com/vaadin/tests/server/component/abstractselect/OptionGroupDeclarativeTests.java +++ b/server/tests/src/com/vaadin/tests/server/component/abstractselect/OptionGroupDeclarativeTests.java @@ -128,13 +128,13 @@ public class OptionGroupDeclarativeTests extends og.addItems("foo", "bar", "baz", "bang"); og.setItemCaption("foo", "True"); - og.setItemCaption("bar", "False"); + og.setItemCaption("bar", "False"); //@formatter:off String expected = "" - + "" - + "" + + "" + + "" + "" + "" + ""; diff --git a/server/tests/src/com/vaadin/tests/server/component/button/ButtonDeclarativeTest.java b/server/tests/src/com/vaadin/tests/server/component/button/ButtonDeclarativeTest.java index 51abf6be96..a9fb69a791 100644 --- a/server/tests/src/com/vaadin/tests/server/component/button/ButtonDeclarativeTest.java +++ b/server/tests/src/com/vaadin/tests/server/component/button/ButtonDeclarativeTest.java @@ -15,6 +15,9 @@ */ package com.vaadin.tests.server.component.button; +import org.jsoup.nodes.Element; +import org.jsoup.parser.Tag; +import org.junit.Assert; import org.junit.Test; import com.vaadin.event.ShortcutAction.KeyCode; @@ -22,6 +25,7 @@ import com.vaadin.event.ShortcutAction.ModifierKey; import com.vaadin.tests.design.DeclarativeTestBase; import com.vaadin.ui.Button; import com.vaadin.ui.NativeButton; +import com.vaadin.ui.declarative.DesignContext; /** * Tests declarative support for implementations of {@link Button} and @@ -71,6 +75,41 @@ public class ButtonDeclarativeTest extends DeclarativeTestBase