SPR-5605 spring:url tag should use htmlEscape instead of escapeXml for entity encoding

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/tags/UrlTag.java

@@ -27,9 +27,10 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.jsp.JspException;
 import javax.servlet.jsp.PageContext;
-import javax.servlet.jsp.tagext.TagSupport;
 
-import org.springframework.util.StringUtils;
+import org.springframework.web.util.ExpressionEvaluationUtils;
+import org.springframework.web.util.HtmlUtils;
+import org.springframework.web.util.JavaScriptUtils;
 import org.springframework.web.util.TagUtils;
 
 /**
@@ -39,7 +40,8 @@ import org.springframework.web.util.TagUtils;
  * <p>Enhancements to the JSTL functionality include:
  * <ul>
  * <li>URL encoded template URI variables</li>
- * <li>XML escaping of URLs</li>
+ * <li>HTML/XML escaping of URLs</li>
+ * <li>JavaScipt escaping of URLs</li>
  * </ul>
  * 
  * <p>Template URI variables are indicated in the {@link #setValue(String) 'value'}
@@ -53,8 +55,10 @@ import org.springframework.web.util.TagUtils;
  * over direct EL substitution as the values are URL encoded.  Failure to properly 
  * encode URL can leave an application vulnerable to XSS and other injection attacks.
  * 
- * <p>URLs can be XML escaped by setting the {@link #setEscapeXml(String) 
- * 'escapeXml'} attribute to 'true', the default is 'false'.
+ * <p>URLs can be HTML/XML escaped by setting the {@link #setHtmlEscape(String) 
+ * 'htmlEscape'} attribute to 'true'.  Detects an HTML escaping setting, either on 
+ * this tag instance, the page level, or the <code>web.xml</code> level. The default 
+ * is 'false'.  When setting the URL value into a variable, escaping is not recommended.
  * 
  * <p>Example usage:
  * <pre>&lt;spring:url value="/url/path/{variableName}"&gt;
@@ -67,7 +71,7 @@ import org.springframework.web.util.TagUtils;
  * @since 3.0
  * @see ParamTag
  */
-public class UrlTag extends TagSupport implements ParamAware {
+public class UrlTag extends HtmlEscapingAwareTag implements ParamAware {
 
 	private static final String URL_TEMPLATE_DELIMITER_PREFIX = "{";
 
@@ -75,8 +79,6 @@ public class UrlTag extends TagSupport implements ParamAware {
 
 	private static final String URL_TYPE_ABSOLUTE = "://";
 
-	private static final char[] XML_CHARS = { '&', '<', '>', '"', '\'' };
-
 
 	private List<Param> params;
 
@@ -92,7 +94,7 @@ public class UrlTag extends TagSupport implements ParamAware {
 
 	private int scope = PageContext.PAGE_SCOPE;
 
-	private boolean escapeXml = false;
+	private boolean javaScriptEscape = false;
 
 
 	/**
@@ -142,13 +144,12 @@ public class UrlTag extends TagSupport implements ParamAware {
 	}
 
 	/**
-	 * Instructs the tag to XML entity encode the resulting URL.
-	 * <p>Defaults to false to maintain compatibility with the JSTL c:url tag.
-	 * <p><b>NOTE:</b> Strongly recommended to set as 'true' when rendering
-	 * directly to the JspWriter in an XML or HTML based file.
+	 * Set JavaScript escaping for this tag, as boolean value.
+	 * Default is "false".
 	 */
-	public void setEscapeXml(String escapeXml) {
-		this.escapeXml = Boolean.valueOf(escapeXml);
+	public void setJavaScriptEscape(String javaScriptEscape) throws JspException {
+		this.javaScriptEscape =
+				ExpressionEvaluationUtils.evaluateBoolean("javaScriptEscape", javaScriptEscape, pageContext);
 	}
 
 	public void addParam(Param param) {
@@ -157,7 +158,7 @@ public class UrlTag extends TagSupport implements ParamAware {
 
 
 	@Override
-	public int doStartTag() throws JspException {
+	public int doStartTagInternal() throws JspException {
 		this.params = new LinkedList<Param>();
 		this.templateParams = new HashSet<String>();
 		return EVAL_BODY_INCLUDE;
@@ -213,9 +214,11 @@ public class UrlTag extends TagSupport implements ParamAware {
 			// (Do not embed the session identifier in a remote link!)
 			urlStr = response.encodeURL(urlStr);
 		}
-		if (this.escapeXml) {
-			urlStr = escapeXml(urlStr);
-		}
+
+		// HTML and/or JavaScript escape, if demanded.
+		urlStr = isHtmlEscape() ? HtmlUtils.htmlEscape(urlStr) : urlStr;
+		urlStr = this.javaScriptEscape ? JavaScriptUtils.javaScriptEscape(urlStr) : urlStr;
+		
 		return urlStr;
 	}
 
@@ -295,40 +298,11 @@ public class UrlTag extends TagSupport implements ParamAware {
 		}
 	}
 
-	/**
-	 * XML entity encode the provided string. &#38;, &#60;, &#62;, &#39; and
-	 * &#34; are encoded to their entity values.
-	 * @param xml the value to escape
-	 * @return the escaped value
-	 */
-	protected String escapeXml(String xml) {
-		if (xml == null) {
-			return null;
-		}
-		String escapedXml = xml;
-		for (char xmlChar : XML_CHARS) {
-			escapedXml = StringUtils.replace(escapedXml, String.valueOf(xmlChar), entityValue(xmlChar));
-		}
-		return escapedXml;
-	}
-
-	/**
-	 * Convert a character value to a XML entity value.
-	 * The decimal value of the character is used.
-	 * <p>For example, 'A' is converted to "&amp;#65;".
-	 * @param xmlChar the character to encode
-	 * @return the entity value
-	 */
-	protected String entityValue(char xmlChar) {
-		return new StringBuilder().append("&#").append(Integer.toString(xmlChar)).append(";").toString();
-	}
-
 
 	/**
 	 * Internal enum that classifies URLs by type.
 	 */
 	private enum UrlType {
-
 		CONTEXT_RELATIVE, RELATIVE, ABSOLUTE
 	}
 

org.springframework.web.servlet/src/main/resources/META-INF/spring.tld

@@ -379,13 +379,18 @@
 			effect unless the var attribute is also defined.</description>
 		</attribute>
 		<attribute>
-			<name>escapeXml</name>
+			<name>htmlEscape</name>
 			<required>false</required>
 			<rtexprvalue>true</rtexprvalue>
-			<description>Escape XML special characters in the resulting URL.  'true' and 
-			'false' are supported.  Defaults to 'false' to maintain compatibility with 
-			the JSTL c:url tag.  Strongly recommended to set as 'true' when rendering 
-			directly to the JspWriter in an XML or HTML based document.</description>
+			<description>Set HTML escaping for this tag, as boolean value. Overrides the
+			default HTML escaping setting for the current page.</description>
+		</attribute>
+		<attribute>
+			<name>javaScriptEscape</name>
+			<required>false</required>
+			<rtexprvalue>true</rtexprvalue>
+			<description>Set JavaScript escaping for this tag, as boolean value.
+			Default is false.</description>
 		</attribute>
 	</tag>
 

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/tags/UrlTagTests.java

@@ -90,7 +90,7 @@ public class UrlTagTests extends AbstractTagTests {
 				PageContext.REQUEST_SCOPE));
 	}
 
-	public void testSetEscapeXmlDefault() throws JspException {
+	public void testSetHtmlEscapeDefault() throws JspException {
 		tag.setValue("url/path");
 		tag.setVar("var");
 
@@ -112,10 +112,10 @@ public class UrlTagTests extends AbstractTagTests {
 				.getAttribute("var"));
 	}
 
-	public void testSetEscapeXmlFalse() throws JspException {
+	public void testSetHtmlEscapeFalse() throws JspException {
 		tag.setValue("url/path");
 		tag.setVar("var");
-		tag.setEscapeXml("false");
+		tag.setHtmlEscape("false");
 
 		tag.doStartTag();
 
@@ -135,10 +135,10 @@ public class UrlTagTests extends AbstractTagTests {
 				.getAttribute("var"));
 	}
 
-	public void testSetEscapeXmlTrue() throws JspException {
+	public void testSetHtmlEscapeTrue() throws JspException {
 		tag.setValue("url/path");
 		tag.setVar("var");
-		tag.setEscapeXml("true");
+		tag.setHtmlEscape("true");
 
 		tag.doStartTag();
 
@@ -154,7 +154,54 @@ public class UrlTagTests extends AbstractTagTests {
 
 		tag.doEndTag();
 
-		assertEquals("url/path?n+me=v%26l%3De&name=value2", context
+		assertEquals("url/path?n+me=v%26l%3De&name=value2", context
+				.getAttribute("var"));
+	}
+
+	public void testSetJavaScriptEscapeTrue() throws JspException {
+		tag.setValue("url/path");
+		tag.setVar("var");
+		tag.setJavaScriptEscape("true");
+
+		tag.doStartTag();
+
+		Param param = new Param();
+		param.setName("n me");
+		param.setValue("v&l=e");
+		tag.addParam(param);
+
+		param = new Param();
+		param.setName("name");
+		param.setValue("value2");
+		tag.addParam(param);
+
+		tag.doEndTag();
+
+		assertEquals("url\\/path?n+me=v%26l%3De&name=value2", context
+				.getAttribute("var"));
+	}
+
+	public void testSetHtmlAndJavaScriptEscapeTrue() throws JspException {
+		tag.setValue("url/path");
+		tag.setVar("var");
+		tag.setHtmlEscape("true");
+		tag.setJavaScriptEscape("true");
+
+		tag.doStartTag();
+
+		Param param = new Param();
+		param.setName("n me");
+		param.setValue("v&l=e");
+		tag.addParam(param);
+
+		param = new Param();
+		param.setName("name");
+		param.setValue("value2");
+		tag.addParam(param);
+
+		tag.doEndTag();
+
+		assertEquals("url\\/path?n+me=v%26l%3De&name=value2", context
 				.getAttribute("var"));
 	}
 
@@ -521,35 +568,6 @@ public class UrlTagTests extends AbstractTagTests {
 		}
 	}
 
-	public void testEscapeXml() {
-		assertEquals("<script type="text/javascript">", tag
-				.escapeXml("<script type=\"text/javascript\">"));
-	}
-
-	public void testEscapeXmlNull() {
-		assertNull(tag.escapeXml(null));
-	}
-
-	public void testEntityValueAmpersand() {
-		assertEquals("&#38;", tag.entityValue('&'));
-	}
-
-	public void testEntityValueLessThan() {
-		assertEquals("&#60;", tag.entityValue('<'));
-	}
-
-	public void testEntityValueGreaterThan() {
-		assertEquals("&#62;", tag.entityValue('>'));
-	}
-
-	public void testEntityValueSingleQuote() {
-		assertEquals("&#39;", tag.entityValue('\''));
-	}
-
-	public void testEntityValueDoubleQuote() {
-		assertEquals("&#34;", tag.entityValue('"'));
-	}
-
 	public void testJspWriterOutput() {
 		// TODO assert that the output to the JspWriter is the expected output
 	}