Uploaded image for project: 'Stripes'
  1. STS-894

Clean URL decoding bug - double decoding of path

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects versions: Release 1.5.4, Release 1.5.5, Release 1.5.6, Release 1.5.7
    • Fix versions: Release 1.5.8, Release 1.6
    • Labels:
      None
    • Environment:
      Tomcat 6.0.20
    • Sprint:

      Description

      STS-743 Closed , while very well meaning, introduced a very subtle bug in the clean URL handling code.

      The following URL, which is perfectly valid, will not be parsed correctly by the HttpUtil class.

      @UrlBinding( value = "/action/group/

      {key}

      " )
      key = "upgrades+configuration"

      The Stripes tag libraries generated this URL (correctly): http://mysite.com/action/group/upgrades%2Bconfiguration

      The HttpUtil class receives this from the HttpServletRequest correctly parsed:
      servletPath = "/action"
      pathInfo = "/group/upgrades+configuration"

      So far, so good. But then the path is sent to StringUtil.urlDecode(). This turns the path in to "/action/group/upgrades configuration". This is incorrect.

      The bug appears to be running a path through the URLDecoder class. The URLDecoder class mangles the correct path. The path should be returned as-is and not decoded twice.

      ================
      Technical details - the boring stuff...
      ================

      It took me a while to research this, but I finally found the cause:

      Different parts of the URL (URI) are encoded differently. The '+' sign, for example, is handled differently depending on where it appears. According to RFC 2396 (http://www.ietf.org/rfc/rfc2396.txt), a plus sign is legal in the path, but converts to a space in the query/parameters area. For example:

      @UrlBinding( value = "/action/dosomething/

      {param1}

      ")

      http://mywebsite.com/action/dosomething/first+value?param2=other+value

      path area parsing query area parsing
      param1 = 'first+value' param2 = 'other value'

      In STS-743 Closed , the reporter mentions that they have encoded the parameter values using URLEncoding. URLEncoding/URLDecoding can only parse URI query parameters, and does not work on URI paths. So the encoding that was reported as "broken" was actually working correctly for the first time.

      Both of these are correctly handled by the parsing code in the Servlet engine. The path is parsed correctly, and using the parameterMap the values are also parsed correctly. No further action would be required by stripes to handle this. (In fact, the URI class always encodes a "+" sign in to the path as %23, but will accept either form for parsing and handles it correctly.)

      =================
      Suggested solutions
      =================

      Two possible fixes. First, change the getRequestedPath() method. I've attached code below. Second, it might be worthwhile to provide a startup parameter that re-enables the use of URLDecoding for parameters passed in the path. (And just the parameters, not the rest of the path.) If you would like this option let me know, and I'll see what I can do.

      public static String getRequestedPath(HttpServletRequest request) {
      String servletPath, pathInfo;

      // Check to see if the request is processing an include, and pull the path
      // information from the appropriate source.
      servletPath = (String) request.getAttribute(StripesConstants.REQ_ATTR_INCLUDE_PATH);
      if (servletPath != null)

      { pathInfo = (String) request.getAttribute(StripesConstants.REQ_ATTR_INCLUDE_PATH_INFO); }

      else

      { servletPath = request.getServletPath(); pathInfo = request.getPathInfo(); }

      String finalPath = (servletPath != null ? servletPath : "") + (pathInfo != null ? pathInfo : "" );
      return finalPath;
      }

        Attachments

          Issue links

            Activity

              People

              • Assignee:
                vankeisb R
                Reporter:
                jonebaker Jonathan Baker
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: