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