Hi Sergiu, I don't like several of the changes here since I believe they're increasing our technical debt so I'd like we find a better way. Here's some feedback/ideas: 1) com.xpn.util.Util and com.xpn.xwiki.web.Utils are deprecated and shouldn't be used. On the contrary we should work towards removing stuff from them, not adding to them :) I see you've added a new method which isn't too good IMO. 2) The EscapeTool you've added to the velocity module doesn't belong there IMO since I don't see why it would be restricted to Velocity. It's not about a language limitation that would be only needed for Velocity. I believe other scripts and even other java part of XWiki might need to escape XML. In addition you haven't commented why there's a need to write a new class when an escape tool already exists and is provided by the Velocity Tools project (in a few days/months we'll wonder why). For 2) what I think is best is to add the escape code to the xwiki-xml module and have a ScriptService to make it available to all scripts. BTW there's already a XMLUtils in there and which already does xml escaping (and which I don't like and it could a good time to extract part of its stuff into an Escaper/Encoder component). So XML and HTML escaping should go in xwiki-xml IMO and URL escaping should go in the xwiki-URL module (should we need them). WDYT? Thanks -Vincent On Dec 8, 2010, at 4:47 AM, sdumitriu (SVN) wrote:

Author: sdumitriu Date: 2010-12-08 04:47:08 +0100 (Wed, 08 Dec 2010) New Revision: 33301

Added: platform/core/trunk/xwiki-velocity/src/main/java/org/xwiki/velocity/tools/EscapeTool.java platform/core/trunk/xwiki-velocity/src/test/java/org/xwiki/velocity/tools/EscapeToolTest.java Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/XWiki.java platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/util/Util.java platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/web/Utils.java platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/renderer/printer/XHTMLWriter.java platform/core/trunk/xwiki-velocity/src/main/java/org/xwiki/velocity/internal/DefaultVelocityConfiguration.java Log: XWIKI-5514: "apostrophe" character badly displayed in Internet Explorer XWIKI-5763: Remove entity encoding from UTF-8 text in XHTML Fixed.

Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/XWiki.java =================================================================== --- platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/XWiki.java 2010-12-07 16:01:32 UTC (rev 33300) +++ platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/XWiki.java 2010-12-08 03:47:08 UTC (rev 33301) @@ -5806,12 +5806,12 @@ try { userdoc = getDocument(user, context); if (userdoc == null) { - return StringEscapeUtils.escapeXml(user); + return Utils.escapeXml(user); }

BaseObject userobj = userdoc.getObject("XWiki.XWikiUsers"); if (userobj == null) { - return StringEscapeUtils.escapeXml(userdoc.getName()); + return Utils.escapeXml(userdoc.getName()); }

Set<String> proplist = userobj.getPropertyList(); @@ -5832,7 +5832,7 @@ + context.getDoc().getPrefixedFullName() + ">", vcontext, context); }

- text = StringEscapeUtils.escapeXml(text.trim()); + text = Utils.escapeXml(text.trim());

if (link) { text =

Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/util/Util.java =================================================================== --- platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/util/Util.java 2010-12-07 16:01:32 UTC (rev 33300) +++ platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/util/Util.java 2010-12-08 03:47:08 UTC (rev 33301) @@ -51,7 +51,6 @@ import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; import org.apache.commons.lang.ArrayUtils; -import org.apache.commons.lang.StringEscapeUtils; import org.apache.commons.lang.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -623,8 +622,8 @@

public static String getHTMLExceptionMessage(XWikiException xe, XWikiContext context) { - String title = StringEscapeUtils.escapeXml(xe.getMessage()); - String text = StringEscapeUtils.escapeXml(xe.getFullMessage()); + String title = Utils.escapeXml(xe.getMessage()); + String text = Utils.escapeXml(xe.getFullMessage()); String id = (String) context.get("xwikierrorid"); if (id == null) { id = "1";

Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/web/Utils.java =================================================================== --- platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/web/Utils.java 2010-12-07 16:01:32 UTC (rev 33300) +++ platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/web/Utils.java 2010-12-08 03:47:08 UTC (rev 33301) @@ -40,8 +40,6 @@ import org.apache.commons.lang.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.apache.ecs.Filter; -import org.apache.ecs.filter.CharacterFilter; import org.apache.log4j.MDC; import org.apache.struts.upload.MultipartRequestWrapper; import org.xwiki.component.manager.ComponentLookupException; @@ -512,14 +510,58 @@ map.put(name, newValues); }

+ /** + * Escapes the XML special characters in a <code>String</code> using numerical XML entities. + * + * @param value the text to escape, may be null + * @return a new escaped <code>String</code>, <code>null</code> if null input + * @deprecated starting with 2.7 use {@link #escapeXml(String)} + */ + @Deprecated public static String formEncode(String value) { - Filter filter = new CharacterFilter(); - filter.removeAttribute("'"); - String svalue = filter.process(value); - return svalue; + return escapeXml(value); }

+ /** + * Escapes the XML special characters in a <code>String</code> using numerical XML entities. + * + * @param value the text to escape, may be {@code null} + * @return a new escaped {@code String}, {@code null} if {@code null} input + */ + public static String escapeXml(String value) + { + if (value == null) { + return null; + } + StringBuilder result = new StringBuilder((int) (value.length() * 1.1)); + int length = value.length(); + char c; + for (int i = 0; i < length; ++i) { + c = value.charAt(i); + switch (c) { + case '&': + result.append("&"); + break; + case '<': + result.append("<"); + break; + case '>': + result.append(">"); + break; + case '"': + result.append("""); + break; + case '\'': + result.append("'"); + break; + default: + result.append(c); + } + } + return result.toString(); + } + public static String SQLFilter(String text) { try {

Modified: platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/renderer/printer/XHTMLWriter.java =================================================================== --- platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/renderer/printer/XHTMLWriter.java 2010-12-07 16:01:32 UTC (rev 33300) +++ platform/core/trunk/xwiki-rendering/xwiki-rendering-api/src/main/java/org/xwiki/rendering/internal/renderer/printer/XHTMLWriter.java 2010-12-08 03:47:08 UTC (rev 33301) @@ -44,6 +44,19 @@ super(writer, DEFAULT_XHTML_FORMAT);

// escape all non US-ASCII to have as less encoding problems as possible - setMaximumAllowedCharacter(127); + setMaximumAllowedCharacter(-1); } + + /** + * Escapes a string to be used as an attribute value. Unlike the original method in {@link XMLWriter}, apostrophes + * are replaced by a numerical entity &#38;, since &apos; is not valid in HTML documents. + * + * @param text the attribute value to escape + * @return the text with all occurrences of special XML characters replaced by entity references. + */ + @Override + protected String escapeAttributeEntities(String text) + { + return super.escapeAttributeEntities(text).replace("'", "&"); + } }

Modified: platform/core/trunk/xwiki-velocity/src/main/java/org/xwiki/velocity/internal/DefaultVelocityConfiguration.java =================================================================== --- platform/core/trunk/xwiki-velocity/src/main/java/org/xwiki/velocity/internal/DefaultVelocityConfiguration.java 2010-12-07 16:01:32 UTC (rev 33300) +++ platform/core/trunk/xwiki-velocity/src/main/java/org/xwiki/velocity/internal/DefaultVelocityConfiguration.java 2010-12-08 03:47:08 UTC (rev 33301) @@ -22,7 +22,6 @@ import java.util.Properties;

import org.apache.velocity.tools.generic.ComparisonDateTool; -import org.apache.velocity.tools.generic.EscapeTool; import org.apache.velocity.tools.generic.ListTool; import org.apache.velocity.tools.generic.MathTool; import org.apache.velocity.tools.generic.NumberTool; @@ -37,6 +36,7 @@ import org.xwiki.velocity.VelocityConfiguration; import org.xwiki.velocity.introspection.ChainingUberspector; import org.xwiki.velocity.introspection.DeprecatedCheckUberspector; +import org.xwiki.velocity.tools.EscapeTool; import org.xwiki.velocity.tools.RegexTool;

/**

Added: platform/core/trunk/xwiki-velocity/src/main/java/org/xwiki/velocity/tools/EscapeTool.java =================================================================== --- platform/core/trunk/xwiki-velocity/src/main/java/org/xwiki/velocity/tools/EscapeTool.java (rev 0) +++ platform/core/trunk/xwiki-velocity/src/main/java/org/xwiki/velocity/tools/EscapeTool.java 2010-12-08 03:47:08 UTC (rev 33301) @@ -0,0 +1,72 @@ +/* + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ + +package org.xwiki.velocity.tools; + +/** + * Tool for working with escaping in Velocity templates. It provides methods to escape outputs for Velocity, Java, + * JavaScript, HTML, XML and SQL. + * + * @version $Id$ + * @since 2.7RC1 + */ +public class EscapeTool extends org.apache.velocity.tools.generic.EscapeTool +{ + /** + * Escapes the XML special characters in a <code>String</code> using numerical XML entities. + * + * @param value the text to escape, may be {@code null} + * @return a new escaped {@code String}, {@code null} if {@code null} input + */ + @Override + public String xml(Object source) + { + if (source == null) { + return null; + } + String str = String.valueOf(source); + StringBuilder result = new StringBuilder((int) (str.length() * 1.1)); + int length = str.length(); + char c; + for (int i = 0; i < length; ++i) { + c = str.charAt(i); + switch (c) { + case '&': + result.append("&"); + break; + case '<': + result.append("<"); + break; + case '>': + result.append(">"); + break; + case '"': + result.append("""); + break; + case '\'': + result.append("'"); + break; + default: + result.append(c); + } + } + return result.toString(); + } +}

Property changes on: platform/core/trunk/xwiki-velocity/src/main/java/org/xwiki/velocity/tools/EscapeTool.java ___________________________________________________________________ Added: svn:keywords + Author Id Revision HeadURL Added: svn:eol-style + native

Added: platform/core/trunk/xwiki-velocity/src/test/java/org/xwiki/velocity/tools/EscapeToolTest.java =================================================================== --- platform/core/trunk/xwiki-velocity/src/test/java/org/xwiki/velocity/tools/EscapeToolTest.java (rev 0) +++ platform/core/trunk/xwiki-velocity/src/test/java/org/xwiki/velocity/tools/EscapeToolTest.java 2010-12-08 03:47:08 UTC (rev 33301) @@ -0,0 +1,70 @@ +/* + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ + +package org.xwiki.velocity.tools; + +import org.junit.Assert; +import org.junit.Test; + +/** + * Unit tests for {@link EscapeTool}. + * + * @version $Id$ + * @since 2.7RC1 + */ +public class EscapeToolTest +{ + @Test + public void testEscapeSimpleXML() + { + EscapeTool tool = new EscapeTool(); + String escapedText = tool.xml("a < a' && a' < a\" => a < a\""); + + Assert.assertFalse("Failed to escape <", escapedText.contains("<")); + Assert.assertFalse("Failed to escape >", escapedText.contains(">")); + Assert.assertFalse("Failed to escape '", escapedText.contains("'")); + Assert.assertFalse("Failed to escape \"", escapedText.contains("\"")); + Assert.assertFalse("Failed to escape &", escapedText.contains("&&")); + } + + @Test + public void testEscapeXMLApos() + { + EscapeTool tool = new EscapeTool(); + + Assert.assertFalse("' wrongly escaped to non-HTML '", tool.xml("'").equals("'")); + } + + @Test + public void testEscapeXMLWithNull() + { + EscapeTool tool = new EscapeTool(); + + Assert.assertNull("null should be null", tool.xml(null)); + } + + @Test + public void testEscapeXMLNonAscii() + { + EscapeTool tool = new EscapeTool(); + + Assert.assertTrue("Non-ASCII characters shouldn't be escaped", tool.xml("\u0123").equals("\u0123")); + } +}

Property changes on: platform/core/trunk/xwiki-velocity/src/test/java/org/xwiki/velocity/tools/EscapeToolTest.java ___________________________________________________________________ Added: svn:keywords + Author Id Revision HeadURL Added: svn:eol-style + native