From 99fa91d56e221fdcd8b55243d10e6905aef06ac4 Mon Sep 17 00:00:00 2001 From: Scott Andrews Date: Thu, 26 Mar 2009 04:37:26 +0000 Subject: [PATCH] SPR-5605 spring:url tag should use htmlEscape instead of escapeXml for entity encoding --- .../webapp/WEB-INF/jsp/dataAccessFailure.jsp | 2 +- .../src/main/webapp/WEB-INF/jsp/footer.jsp | 4 +- .../src/main/webapp/WEB-INF/jsp/header.jsp | 2 +- .../main/webapp/WEB-INF/jsp/owners/search.jsp | 2 +- .../src/main/webapp/WEB-INF/jsp/vets.jsp | 2 +- .../src/main/webapp/WEB-INF/jsp/welcome.jsp | 8 +- .../web/servlet/tags/UrlTag.java | 70 +++++---------- .../src/main/resources/META-INF/spring.tld | 15 ++-- .../web/servlet/tags/UrlTagTests.java | 88 +++++++++++-------- 9 files changed, 95 insertions(+), 98 deletions(-) diff --git a/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/dataAccessFailure.jsp b/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/dataAccessFailure.jsp index e4770371086..256cca17786 100644 --- a/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/dataAccessFailure.jsp +++ b/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/dataAccessFailure.jsp @@ -14,6 +14,6 @@ ex.printStackTrace(new java.io.PrintWriter(out));


-">Home +">Home <%@ include file="/WEB-INF/jsp/footer.jsp" %> diff --git a/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/footer.jsp b/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/footer.jsp index 9acf4ce9373..52aaffc47e7 100644 --- a/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/footer.jsp +++ b/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/footer.jsp @@ -1,8 +1,8 @@ - - + +
">Home" alt="Sponsored by SpringSource"/>">Home" alt="Sponsored by SpringSource"/>
diff --git a/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/header.jsp b/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/header.jsp index f0876c20b88..49393d364c9 100644 --- a/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/header.jsp +++ b/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/header.jsp @@ -5,7 +5,7 @@ - " type="text/css"/> + " type="text/css"/> PetClinic :: a Spring Framework demonstration diff --git a/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/owners/search.jsp b/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/owners/search.jsp index 5ff288225c4..b972390a19f 100644 --- a/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/owners/search.jsp +++ b/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/owners/search.jsp @@ -21,6 +21,6 @@
-Add Owner +Add Owner <%@ include file="/WEB-INF/jsp/footer.jsp" %> diff --git a/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/vets.jsp b/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/vets.jsp index f2f9e6f5b91..cff2154f290 100644 --- a/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/vets.jsp +++ b/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/vets.jsp @@ -23,7 +23,7 @@
- ">View as XML + ">View as XML
diff --git a/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/welcome.jsp b/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/welcome.jsp index 9e9ec97f055..1b07c65c3c2 100644 --- a/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/welcome.jsp +++ b/org.springframework.samples.petclinic/src/main/webapp/WEB-INF/jsp/welcome.jsp @@ -1,13 +1,13 @@ <%@ include file="/WEB-INF/jsp/includes.jsp" %> <%@ include file="/WEB-INF/jsp/header.jsp" %> -" align="right" style="position:relative;right:30px;"> +" align="right" style="position:relative;right:30px;">

 

diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/tags/UrlTag.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/tags/UrlTag.java index 9a03b6be643..5bc0c0a0d08 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/tags/UrlTag.java +++ b/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; *

Enhancements to the JSTL functionality include: *

* *

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. * - *

URLs can be XML escaped by setting the {@link #setEscapeXml(String) - * 'escapeXml'} attribute to 'true', the default is 'false'. + *

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 web.xml level. The default + * is 'false'. When setting the URL value into a variable, escaping is not recommended. * *

Example usage: * 

<spring:url value="/url/path/{variableName}">
@@ -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 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.
-	 * 
Defaults to false to maintain compatibility with the JSTL c:url tag.
-	 * 
NOTE: 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();
 		this.templateParams = new HashSet();
 		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. &, <, >, ' and
-	 * " 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.
-	 * 
For example, 'A' is converted to "&#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
 	}
 
diff --git a/org.springframework.web.servlet/src/main/resources/META-INF/spring.tld b/org.springframework.web.servlet/src/main/resources/META-INF/spring.tld
index 85ae51afd32..fb2675d8b85 100644
--- a/org.springframework.web.servlet/src/main/resources/META-INF/spring.tld
+++ b/org.springframework.web.servlet/src/main/resources/META-INF/spring.tld
@@ -379,13 +379,18 @@
 			effect unless the var attribute is also defined.
 		
 		
-			escapeXml
+			htmlEscape
 			false
 			true
-			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.
+			Set HTML escaping for this tag, as boolean value. Overrides the
+			default HTML escaping setting for the current page.
+		
+		
+			javaScriptEscape
+			false
+			true
+			Set JavaScript escaping for this tag, as boolean value.
+			Default is false.
 		
 	
 
diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/tags/UrlTagTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/tags/UrlTagTests.java
index 9182a30a50a..9477c7947ce 100644
--- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/tags/UrlTagTests.java
+++ b/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("