Uploaded image for project: 'Struts 2'
  1. Struts 2
  2. WW-4448

Parameters are not encoded by ServletRedirectAction before checking for valid URI

    Details

      Description

      WW-4187 changed ServletRedirectResult to use java.net.URI to check whether a redirect URL is actually a path. However, it does not encode parameters first, which will often result in a URL being deemed invalid (eg if one of the parameters contains spaces) and thus being treated as a path.

      Where I work, we actually don't want parameters to be appended to our absolute redirects at all, but I can't see a way to disable this...DefaultResultFactory doesn't seem to be configurable.

        Issue Links

          Activity

          Hide
          thrawnca Mitth'raw'nuruodo added a comment - - edited

          I've found a workaround, at least, using a 'httpheader' result type, which does not append parameters, instead of ServletRedirectAction.

                  <result name="success" type="httpheader">
                      <param name="status">302</param>
                      <param name="headers.Location">${redirectUrl}</param>
                  </result>
          
          Show
          thrawnca Mitth'raw'nuruodo added a comment - - edited I've found a workaround, at least, using a 'httpheader' result type, which does not append parameters, instead of ServletRedirectAction. <result name= "success" type= "httpheader" > <param name= "status" > 302 </param> <param name= "headers.Location" > ${redirectUrl} </param> </result>
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          DefaultResultFactory isn't configurable but you can easily defined your own, here are all the extension points [1]

          [1] http://struts.apache.org/docs/plugins.html#Plugins-ExtensionPoints

          Show
          lukaszlenart Lukasz Lenart added a comment - DefaultResultFactory isn't configurable but you can easily defined your own, here are all the extension points [1] [1] http://struts.apache.org/docs/plugins.html#Plugins-ExtensionPoints
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          So the solution is to add encoding of input parameter in protected boolean isPathUrl(String url), right?

          Show
          lukaszlenart Lukasz Lenart added a comment - So the solution is to add encoding of input parameter in protected boolean isPathUrl(String url) , right?
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          Encoding ruins the whole url, dropping parameters solves the problem:

                      String rawUrl = url;
                      if (url.contains("?")) {
                          rawUrl = url.substring(0, url.indexOf("?"));
                      }
                      URI uri = URI.create(rawUrl.replaceAll(" ", "%20"));
          
          Show
          lukaszlenart Lukasz Lenart added a comment - Encoding ruins the whole url, dropping parameters solves the problem: String rawUrl = url; if (url.contains( "?" )) { rawUrl = url.substring(0, url.indexOf( "?" )); } URI uri = URI.create(rawUrl.replaceAll( " " , "%20" ));
          Hide
          thrawnca Mitth'raw'nuruodo added a comment - - edited

          That should work, but why do you need to encode spaces?

          Yes, we could provide a custom result factory, but the header solution is probably more future-proof for us.

          Thanks for jumping on this . Any idea when 2.3.22 is coming?

          Show
          thrawnca Mitth'raw'nuruodo added a comment - - edited That should work, but why do you need to encode spaces? Yes, we could provide a custom result factory, but the header solution is probably more future-proof for us. Thanks for jumping on this . Any idea when 2.3.22 is coming?
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          I must encode only spaces as URI.create will blow up if there are spaces in path

          No concrete plans for 2.3.22 but probably in 2-3 weeks

          Show
          lukaszlenart Lukasz Lenart added a comment - I must encode only spaces as URI.create will blow up if there are spaces in path No concrete plans for 2.3.22 but probably in 2-3 weeks
          Hide
          thrawnca Mitth'raw'nuruodo added a comment - - edited

          Hmm...fair enough, but if URI.create fails, then the default behavior is to treat it as a path, right? So I would think that that would work fine.

          Unless there's a performance issue (which shouldn't be a problem when we're issuing a redirect anyway), I would think that encoding is only necessary if there might be spaces in an otherwise valid absolute URL

          Show
          thrawnca Mitth'raw'nuruodo added a comment - - edited Hmm...fair enough, but if URI.create fails, then the default behavior is to treat it as a path, right? So I would think that that would work fine. Unless there's a performance issue (which shouldn't be a problem when we're issuing a redirect anyway), I would think that encoding is only necessary if there might be spaces in an otherwise valid absolute URL
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 0f44e11cd1f3cff51ed4a2a10dec593d8822ade2 in struts's branch refs/heads/develop from Lukasz Lenart
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=0f44e11 ]

          WW-4448 Strips params and replaces spaces

          Show
          jira-bot ASF subversion and git services added a comment - Commit 0f44e11cd1f3cff51ed4a2a10dec593d8822ade2 in struts's branch refs/heads/develop from Lukasz Lenart [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=0f44e11 ] WW-4448 Strips params and replaces spaces
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          Done, thanks for reporting!

          Show
          lukaszlenart Lukasz Lenart added a comment - Done, thanks for reporting!
          Hide
          hudson Hudson added a comment -

          UNSTABLE: Integrated in Struts-JDK6-develop #124 (See https://builds.apache.org/job/Struts-JDK6-develop/124/)
          WW-4448 Strips params and replaces spaces (lukaszlenart: rev 0f44e11cd1f3cff51ed4a2a10dec593d8822ade2)

          • core/src/test/java/org/apache/struts2/dispatcher/ServletRedirectResultTest.java
          • core/src/main/java/org/apache/struts2/dispatcher/ServletRedirectResult.java
          Show
          hudson Hudson added a comment - UNSTABLE: Integrated in Struts-JDK6-develop #124 (See https://builds.apache.org/job/Struts-JDK6-develop/124/ ) WW-4448 Strips params and replaces spaces (lukaszlenart: rev 0f44e11cd1f3cff51ed4a2a10dec593d8822ade2) core/src/test/java/org/apache/struts2/dispatcher/ServletRedirectResultTest.java core/src/main/java/org/apache/struts2/dispatcher/ServletRedirectResult.java
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit acef492390863e73f97714f082b214046b46c9c2 in struts's branch refs/heads/develop from Lukasz Lenart
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=acef492 ]

          WW-4448 Replaces spaces with encoded value

          Show
          jira-bot ASF subversion and git services added a comment - Commit acef492390863e73f97714f082b214046b46c9c2 in struts's branch refs/heads/develop from Lukasz Lenart [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=acef492 ] WW-4448 Replaces spaces with encoded value
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Struts-JDK6-develop #125 (See https://builds.apache.org/job/Struts-JDK6-develop/125/)
          WW-4448 Replaces spaces with encoded value (lukaszlenart: rev acef492390863e73f97714f082b214046b46c9c2)

          • core/src/main/java/org/apache/struts2/dispatcher/ServletRedirectResult.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Struts-JDK6-develop #125 (See https://builds.apache.org/job/Struts-JDK6-develop/125/ ) WW-4448 Replaces spaces with encoded value (lukaszlenart: rev acef492390863e73f97714f082b214046b46c9c2) core/src/main/java/org/apache/struts2/dispatcher/ServletRedirectResult.java
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Struts-JDK6-master #904 (See https://builds.apache.org/job/Struts-JDK6-master/904/)
          WW-4448 Strips params and replaces spaces (lukaszlenart: rev 0f44e11cd1f3cff51ed4a2a10dec593d8822ade2)

          • core/src/test/java/org/apache/struts2/dispatcher/ServletRedirectResultTest.java
          • core/src/main/java/org/apache/struts2/dispatcher/ServletRedirectResult.java
            WW-4448 Replaces spaces with encoded value (lukaszlenart: rev acef492390863e73f97714f082b214046b46c9c2)
          • core/src/main/java/org/apache/struts2/dispatcher/ServletRedirectResult.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Struts-JDK6-master #904 (See https://builds.apache.org/job/Struts-JDK6-master/904/ ) WW-4448 Strips params and replaces spaces (lukaszlenart: rev 0f44e11cd1f3cff51ed4a2a10dec593d8822ade2) core/src/test/java/org/apache/struts2/dispatcher/ServletRedirectResultTest.java core/src/main/java/org/apache/struts2/dispatcher/ServletRedirectResult.java WW-4448 Replaces spaces with encoded value (lukaszlenart: rev acef492390863e73f97714f082b214046b46c9c2) core/src/main/java/org/apache/struts2/dispatcher/ServletRedirectResult.java
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Struts-JDK7-master #371 (See https://builds.apache.org/job/Struts-JDK7-master/371/)
          WW-4448 Strips params and replaces spaces (lukaszlenart: rev 0f44e11cd1f3cff51ed4a2a10dec593d8822ade2)

          • core/src/test/java/org/apache/struts2/dispatcher/ServletRedirectResultTest.java
          • core/src/main/java/org/apache/struts2/dispatcher/ServletRedirectResult.java
            WW-4448 Replaces spaces with encoded value (lukaszlenart: rev acef492390863e73f97714f082b214046b46c9c2)
          • core/src/main/java/org/apache/struts2/dispatcher/ServletRedirectResult.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Struts-JDK7-master #371 (See https://builds.apache.org/job/Struts-JDK7-master/371/ ) WW-4448 Strips params and replaces spaces (lukaszlenart: rev 0f44e11cd1f3cff51ed4a2a10dec593d8822ade2) core/src/test/java/org/apache/struts2/dispatcher/ServletRedirectResultTest.java core/src/main/java/org/apache/struts2/dispatcher/ServletRedirectResult.java WW-4448 Replaces spaces with encoded value (lukaszlenart: rev acef492390863e73f97714f082b214046b46c9c2) core/src/main/java/org/apache/struts2/dispatcher/ServletRedirectResult.java

            People

            • Assignee:
              lukaszlenart Lukasz Lenart
              Reporter:
              thrawnca Mitth'raw'nuruodo
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development