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