Struts 2
  1. Struts 2
  2. WW-1624

URL tag should support not escaping ampersand

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Duplicate
    • Affects Version/s: None
    • Fix Version/s: 2.0.10, 2.1.0
    • Component/s: Plugin - Tags
    • Labels:
      None

      Description

      See: http://jira.opensymphony.com/browse/WW-1403

      URL tag should support not escaping ampersand as discussed at [1]

      [1] - http://forums.opensymphony.com/thread.jspa?threadID=46866&tstart=0

        Issue Links

          Activity

          Jeff Turner made changes -
          Reporter Phil Barnes [ pluppens ] Philip Luppens [ pluppens ]
          Jeff Turner made changes -
          Project Import Mon Feb 01 01:17:42 UTC 2010 [ 1264987062082 ]
          Antonio Petrelli made changes -
          Workflow Struts - editable closed status (temporary) [ 47149 ] Struts - editable closed status [ 50426 ]
          Antonio Petrelli made changes -
          Workflow Struts - editable closed status [ 42353 ] Struts - editable closed status (temporary) [ 47149 ]
          Jeff Turner made changes -
          Workflow Struts [ 40723 ] Struts - editable closed status [ 42353 ]
          James Holmes made changes -
          Fix Version/s 2.0.10 [ 21850 ]
          Resolution Duplicate [ 3 ]
          Status Open [ 1 ] Closed [ 6 ]
          Hide
          James Holmes added a comment -

          Closing this as a duplicate of WW-1938.

          Show
          James Holmes added a comment - Closing this as a duplicate of WW-1938 .
          Hide
          James Holmes added a comment -

          Even though this ticket was opened first, I'm going to mark it as a duplicate of WW-1938 because that is where the active discussion is.

          Show
          James Holmes added a comment - Even though this ticket was opened first, I'm going to mark it as a duplicate of WW-1938 because that is where the active discussion is.
          James Holmes made changes -
          Link This issue duplicates WW-1938 [ WW-1938 ]
          Hide
          Don Brown added a comment -

          This needs to evaluated in light of the url handling changes we have contemplated as part of the portlet removal. The poor buildUrl() method would have 10 parameters, which raises some red flags for me.

          Show
          Don Brown added a comment - This needs to evaluated in light of the url handling changes we have contemplated as part of the portlet removal. The poor buildUrl() method would have 10 parameters, which raises some red flags for me.
          Hide
          Knut Erik Borgen added a comment -

          As this was solved in Webworks I have ported it from webworks to struts + included a unittest. Here is the diff:

          $ svn diff
          Index: core/src/test/java/org/apache/struts2/views/jsp/URLTagTest.java
          ===================================================================
          — core/src/test/java/org/apache/struts2/views/jsp/URLTagTest.java (revision 543088)
          +++ core/src/test/java/org/apache/struts2/views/jsp/URLTagTest.java (working copy)
          @@ -23,6 +23,7 @@
          import java.util.HashMap;
          import java.util.Map;

          +import javax.servlet.jsp.JspException;
          import javax.servlet.jsp.JspWriter;

          import org.apache.struts2.components.URL;
          @@ -185,7 +186,7 @@

          // request parameter map should not have any effect, as includeParams
          // default to GET, which get its param from request.getQueryString()

          • Map tmp = new HashMap();
            + Map<String,String> tmp = new HashMap<String,String>();
            tmp.put("one", "aaa");
            tmp.put("two", "bbb");
            tmp.put("three", "ccc");
            @@ -348,6 +349,19 @@
            assertEquals("/team.action?section=team&company=acme+inc", writer.toString());
            }

          + public void testRequestURIActionIncludeGetDoNotQuoteAmp() throws JspException

          { + request.setRequestURI("/public/about"); + request.setQueryString("section=team&company=acme inc"); + + tag.setAction("team"); + tag.setIncludeParams("get"); + tag.setEscapeAmp("false"); + tag.doStartTag(); + tag.doEndTag(); + + assertEquals("/team.action?section=team&company=acme+inc", writer.toString()); + }

          +
          public void testRequestURINoActionIncludeNone() throws Exception {
          request.setRequestURI("/public/about");
          request.setQueryString("section=team&company=acme inc");
          Index: core/src/main/java/org/apache/struts2/components/Component.java
          ===================================================================
          — core/src/main/java/org/apache/struts2/components/Component.java (revision 543088)
          +++ core/src/main/java/org/apache/struts2/components/Component.java (working copy)
          @@ -334,16 +334,17 @@

          • @param scheme http or https
          • @param includeContext should the context path be included or not
          • @param encodeResult should the url be encoded
            + * @param escapeXml If true, it will escape &.
          • @return the action url.
            */
            protected String determineActionURL(String action, String namespace, String method,
            HttpServletRequest req, HttpServletResponse res, Map parameters, String scheme,
          • boolean includeContext, boolean encodeResult) {
            + boolean includeContext, boolean encodeResult, boolean escapeXml) { String finalAction = findString(action); String finalNamespace = determineNamespace(namespace, getStack(), req); ActionMapping mapping = new ActionMapping(finalAction, finalNamespace, method, parameters); String uri = actionMapper.getUriFromActionMapping(mapping); - return UrlHelper.buildUrl(uri, req, res, parameters, scheme, includeContext, encodeResult); + return UrlHelper.buildUrl(uri, req, res, parameters, scheme, includeContext, encodeResult, false, escapeXml); }

          /**
          Index: core/src/main/java/org/apache/struts2/components/URL.java
          ===================================================================
          — core/src/main/java/org/apache/struts2/components/URL.java (revision 543088)
          +++ core/src/main/java/org/apache/struts2/components/URL.java (working copy)
          @@ -141,7 +141,8 @@
          protected String anchor;
          protected String urlIncludeParams;
          protected ExtraParameterProvider extraParameterProvider;
          -
          + protected boolean escapeAmp = true;
          +
          public URL(ValueStack stack, HttpServletRequest req, HttpServletResponse res)

          { super(stack); this.req = req; @@ -240,7 +241,7 @@ result = PortletUrlHelper.buildUrl(action, namespace, parameters, portletUrlType, portletMode, windowSt ate); }

          else

          { - result = determineActionURL(action, namespace, method, req, res, parameters, scheme, includeContext, en code); + result = determineActionURL(action, namespace, method, req, res, parameters, scheme, includeContext, en code, escapeAmp); }

          } else {
          if(Dispatcher.getInstance().isPortletSupportActive() && PortletActionContext.isPortletRequest()) {
          @@ -337,8 +338,12 @@
          public void setAnchor(String anchor)

          { this.anchor = anchor; }
          +
          + @StrutsTagAttribute(description="Whether to escape ampersand (&) to (&) or not, default to true.")
          + public void setEscapeAmp(boolean escapeAmp) { + this.escapeAmp = escapeAmp; + }

          -
          /**
          * Merge request parameters into current parameters. If a parameter is
          * already present, than the request parameter in the current request and value atrribute
          Index: core/src/main/java/org/apache/struts2/views/jsp/URLTag.java
          ===================================================================
          — core/src/main/java/org/apache/struts2/views/jsp/URLTag.java (revision 543088)
          +++ core/src/main/java/org/apache/struts2/views/jsp/URLTag.java (working copy)
          @@ -48,6 +48,7 @@
          protected String windowState;
          protected String portletUrlType;
          protected String anchor;
          + protected String escapeAmp;

          public Component getBean(ValueStack stack, HttpServletRequest req, HttpServletResponse res) {
          return new URL(stack, req, res);
          @@ -74,6 +75,9 @@
          if (includeContext != null) { url.setIncludeContext(Boolean.valueOf(includeContext).booleanValue()); }
          + if (escapeAmp != null) { + url.setEscapeAmp(Boolean.valueOf(escapeAmp).booleanValue()); + }
          }

          public void setEncode(String encode) {
          @@ -120,4 +124,8 @@
          public void setAnchor(String anchor) { this.anchor = anchor; }

          +
          + public void setEscapeAmp(String escapeAmp)

          { + this.escapeAmp = escapeAmp; + }

          }
          Index: core/src/main/java/org/apache/struts2/views/util/UrlHelper.java
          ===================================================================
          — core/src/main/java/org/apache/struts2/views/util/UrlHelper.java (revision 543088)
          +++ core/src/main/java/org/apache/struts2/views/util/UrlHelper.java (working copy)
          @@ -91,6 +91,10 @@
          }

          public static String buildUrl(String action, HttpServletRequest request, HttpServletResponse response, Map params,
          String scheme, boolean includeContext, boolean encodeResult, boolean forceAddSchemeHostAndPort)

          { + return buildUrl(action, request, response, params, scheme, includeContext, encodeResult, forceAddSchemeHostAndPo rt, true); + }

          +
          + public static String buildUrl(String action, HttpServletRequest request, HttpServletResponse response, Map params,
          String scheme, boolean includeContext, boolean encodeResult, boolean forceAddSchemeHostAndPort, boolean escapeAmp)

          { StringBuffer link = new StringBuffer(); boolean changedScheme = false; @@ -169,7 +173,12 @@ }

          //if the action was not explicitly set grab the params from the request

          • buildParametersString(params, link);
            + if (escapeAmp) { + buildParametersString(params, link); + }

            + else

            { + buildParametersString(params, link, "&"); + }

          String result;

          Show
          Knut Erik Borgen added a comment - As this was solved in Webworks I have ported it from webworks to struts + included a unittest. Here is the diff: $ svn diff Index: core/src/test/java/org/apache/struts2/views/jsp/URLTagTest.java =================================================================== — core/src/test/java/org/apache/struts2/views/jsp/URLTagTest.java (revision 543088) +++ core/src/test/java/org/apache/struts2/views/jsp/URLTagTest.java (working copy) @@ -23,6 +23,7 @@ import java.util.HashMap; import java.util.Map; +import javax.servlet.jsp.JspException; import javax.servlet.jsp.JspWriter; import org.apache.struts2.components.URL; @@ -185,7 +186,7 @@ // request parameter map should not have any effect, as includeParams // default to GET, which get its param from request.getQueryString() Map tmp = new HashMap(); + Map<String,String> tmp = new HashMap<String,String>(); tmp.put("one", "aaa"); tmp.put("two", "bbb"); tmp.put("three", "ccc"); @@ -348,6 +349,19 @@ assertEquals("/team.action?section=team&company=acme+inc", writer.toString()); } + public void testRequestURIActionIncludeGetDoNotQuoteAmp() throws JspException { + request.setRequestURI("/public/about"); + request.setQueryString("section=team&company=acme inc"); + + tag.setAction("team"); + tag.setIncludeParams("get"); + tag.setEscapeAmp("false"); + tag.doStartTag(); + tag.doEndTag(); + + assertEquals("/team.action?section=team&company=acme+inc", writer.toString()); + } + public void testRequestURINoActionIncludeNone() throws Exception { request.setRequestURI("/public/about"); request.setQueryString("section=team&company=acme inc"); Index: core/src/main/java/org/apache/struts2/components/Component.java =================================================================== — core/src/main/java/org/apache/struts2/components/Component.java (revision 543088) +++ core/src/main/java/org/apache/struts2/components/Component.java (working copy) @@ -334,16 +334,17 @@ @param scheme http or https @param includeContext should the context path be included or not @param encodeResult should the url be encoded + * @param escapeXml If true, it will escape &. @return the action url. */ protected String determineActionURL(String action, String namespace, String method, HttpServletRequest req, HttpServletResponse res, Map parameters, String scheme, boolean includeContext, boolean encodeResult) { + boolean includeContext, boolean encodeResult, boolean escapeXml) { String finalAction = findString(action); String finalNamespace = determineNamespace(namespace, getStack(), req); ActionMapping mapping = new ActionMapping(finalAction, finalNamespace, method, parameters); String uri = actionMapper.getUriFromActionMapping(mapping); - return UrlHelper.buildUrl(uri, req, res, parameters, scheme, includeContext, encodeResult); + return UrlHelper.buildUrl(uri, req, res, parameters, scheme, includeContext, encodeResult, false, escapeXml); } /** Index: core/src/main/java/org/apache/struts2/components/URL.java =================================================================== — core/src/main/java/org/apache/struts2/components/URL.java (revision 543088) +++ core/src/main/java/org/apache/struts2/components/URL.java (working copy) @@ -141,7 +141,8 @@ protected String anchor; protected String urlIncludeParams; protected ExtraParameterProvider extraParameterProvider; - + protected boolean escapeAmp = true; + public URL(ValueStack stack, HttpServletRequest req, HttpServletResponse res) { super(stack); this.req = req; @@ -240,7 +241,7 @@ result = PortletUrlHelper.buildUrl(action, namespace, parameters, portletUrlType, portletMode, windowSt ate); } else { - result = determineActionURL(action, namespace, method, req, res, parameters, scheme, includeContext, en code); + result = determineActionURL(action, namespace, method, req, res, parameters, scheme, includeContext, en code, escapeAmp); } } else { if(Dispatcher.getInstance().isPortletSupportActive() && PortletActionContext.isPortletRequest()) { @@ -337,8 +338,12 @@ public void setAnchor(String anchor) { this.anchor = anchor; } + + @StrutsTagAttribute(description="Whether to escape ampersand (&) to (&) or not, default to true.") + public void setEscapeAmp(boolean escapeAmp) { + this.escapeAmp = escapeAmp; + } - /** * Merge request parameters into current parameters. If a parameter is * already present, than the request parameter in the current request and value atrribute Index: core/src/main/java/org/apache/struts2/views/jsp/URLTag.java =================================================================== — core/src/main/java/org/apache/struts2/views/jsp/URLTag.java (revision 543088) +++ core/src/main/java/org/apache/struts2/views/jsp/URLTag.java (working copy) @@ -48,6 +48,7 @@ protected String windowState; protected String portletUrlType; protected String anchor; + protected String escapeAmp; public Component getBean(ValueStack stack, HttpServletRequest req, HttpServletResponse res) { return new URL(stack, req, res); @@ -74,6 +75,9 @@ if (includeContext != null) { url.setIncludeContext(Boolean.valueOf(includeContext).booleanValue()); } + if (escapeAmp != null) { + url.setEscapeAmp(Boolean.valueOf(escapeAmp).booleanValue()); + } } public void setEncode(String encode) { @@ -120,4 +124,8 @@ public void setAnchor(String anchor) { this.anchor = anchor; } + + public void setEscapeAmp(String escapeAmp) { + this.escapeAmp = escapeAmp; + } } Index: core/src/main/java/org/apache/struts2/views/util/UrlHelper.java =================================================================== — core/src/main/java/org/apache/struts2/views/util/UrlHelper.java (revision 543088) +++ core/src/main/java/org/apache/struts2/views/util/UrlHelper.java (working copy) @@ -91,6 +91,10 @@ } public static String buildUrl(String action, HttpServletRequest request, HttpServletResponse response, Map params, String scheme, boolean includeContext, boolean encodeResult, boolean forceAddSchemeHostAndPort) { + return buildUrl(action, request, response, params, scheme, includeContext, encodeResult, forceAddSchemeHostAndPo rt, true); + } + + public static String buildUrl(String action, HttpServletRequest request, HttpServletResponse response, Map params, String scheme, boolean includeContext, boolean encodeResult, boolean forceAddSchemeHostAndPort, boolean escapeAmp) { StringBuffer link = new StringBuffer(); boolean changedScheme = false; @@ -169,7 +173,12 @@ } //if the action was not explicitly set grab the params from the request buildParametersString(params, link); + if (escapeAmp) { + buildParametersString(params, link); + } + else { + buildParametersString(params, link, "&"); + } String result;
          Don Brown made changes -
          Fix Version/s 2.0.8 [ 21801 ]
          Fix Version/s 2.1.0 [ 21794 ]
          Ted Husted made changes -
          Fix Version/s 2.0.7 [ 21796 ]
          Fix Version/s 2.0.8 [ 21801 ]
          Ted Husted made changes -
          Fix Version/s 2.0.7 [ 21796 ]
          Fix Version/s 2.0.x [ 21742 ]
          Priority Major [ 3 ] Minor [ 4 ]
          Hide
          Ted Husted added a comment -

          Bumping to 2.0.7 so as to remove 2.0.x version.

          Show
          Ted Husted added a comment - Bumping to 2.0.7 so as to remove 2.0.x version.
          Ted Husted made changes -
          Field Original Value New Value
          Fix Version/s 2.0.x [ 21742 ]
          Philip Luppens created issue -

            People

            • Assignee:
              Unassigned
              Reporter:
              Philip Luppens
            • Votes:
              2 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development