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

Struts 2.5.1 gives errors on unexpected action names

    Details

      Description

      As of Struts 2.5.1 (specifically, commit 27ca165ddbf81c84bafbd083b99a18d89cc49ca7), URLs containing unexpected characters are rejected, instead of cleaned up. This breaks the interaction of one of our clients, who unfortunately is using braces in their URL (matched at our end by a wildcard).

      We want to keep specifying a strict list of allowed characters, for cleanup purposes, but we can't do that if it will break interactions with customers.

      What was the purpose of changing this behavior? I can't find anything about it in the changelog.

        Activity

        Hide
        lukaszlenart Lukasz Lenart added a comment -

        I have updated the docs to add note about how redefine action names' RegEx https://cwiki.apache.org/confluence/display/WW/Action+Configuration#ActionConfiguration-Allowedactionnames

        Show
        lukaszlenart Lukasz Lenart added a comment - I have updated the docs to add note about how redefine action names' RegEx https://cwiki.apache.org/confluence/display/WW/Action+Configuration#ActionConfiguration-Allowedactionnames
        Hide
        jira-bot ASF subversion and git services added a comment -

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

        WW-4669 Reverts back to default RegEx

        Show
        jira-bot ASF subversion and git services added a comment - Commit c6d07d3ab66083f32ead69a9e2ff709d4c29dcd3 in struts's branch refs/heads/master from Lukasz Lenart [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=c6d07d3 ] WW-4669 Reverts back to default RegEx
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Struts-JDK7-master #498 (See https://builds.apache.org/job/Struts-JDK7-master/498/)
        WW-4669 Reverts back to default RegEx (lukaszlenart: rev c6d07d3ab66083f32ead69a9e2ff709d4c29dcd3)

        • core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Struts-JDK7-master #498 (See https://builds.apache.org/job/Struts-JDK7-master/498/ ) WW-4669 Reverts back to default RegEx (lukaszlenart: rev c6d07d3ab66083f32ead69a9e2ff709d4c29dcd3) core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java
        Hide
        thrawnca Mitth'raw'nuruodo added a comment - - edited

        We already know how to adjust the regex. The point is, previously the regex resulted in a warning and an attempt to clean up the URL, whereas now it results in an exception. We don't want this change, it wasn't in the changelog (which resulted in a release breaking a production system until we rolled it back) nor even associated with a JIRA issue, and what is the justification for it?

        We can't control which URLs clients send us; we can only control how we react to them. And reacting with an exception is not graceful or robust. We already have a catch-all action that can gracefully handle unexpected URLs, but it can't work when those URLs are rejected up front.

        Show
        thrawnca Mitth'raw'nuruodo added a comment - - edited We already know how to adjust the regex. The point is, previously the regex resulted in a warning and an attempt to clean up the URL, whereas now it results in an exception. We don't want this change, it wasn't in the changelog (which resulted in a release breaking a production system until we rolled it back) nor even associated with a JIRA issue, and what is the justification for it? We can't control which URLs clients send us; we can only control how we react to them. And reacting with an exception is not graceful or robust. We already have a catch-all action that can gracefully handle unexpected URLs, but it can't work when those URLs are rejected up front.
        Hide
        lukaszlenart Lukasz Lenart added a comment -

        It was mentioned here http://struts.apache.org/docs/s2-035.html as a potenially vulnerable solution - you can always write your own ActionMapper based on DefaultActionMapper and override just cleanupActionName. Right now I'm wondering if instead of throwing exception it would be better to return a default action name ...

        https://struts.apache.org/docs/actionmapper.html#ActionMapper-CustomActionMapper
        https://github.com/apache/struts/blob/master/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java#L385-L391

        Show
        lukaszlenart Lukasz Lenart added a comment - It was mentioned here http://struts.apache.org/docs/s2-035.html as a potenially vulnerable solution - you can always write your own ActionMapper based on DefaultActionMapper and override just cleanupActionName . Right now I'm wondering if instead of throwing exception it would be better to return a default action name ... https://struts.apache.org/docs/actionmapper.html#ActionMapper-CustomActionMapper https://github.com/apache/struts/blob/master/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java#L385-L391
        Hide
        jira-bot ASF subversion and git services added a comment -

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

        WW-4669 Returns default action/method instead of throwing exception

        Show
        jira-bot ASF subversion and git services added a comment - Commit 5acc71f7c30e16807912bf99b4c32cc4b25f586e in struts's branch refs/heads/master from Lukasz Lenart [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=5acc71f ] WW-4669 Returns default action/method instead of throwing exception
        Hide
        lukaszlenart Lukasz Lenart added a comment -

        Default action/method name support was implemented

        Show
        lukaszlenart Lukasz Lenart added a comment - Default action/method name support was implemented
        Hide
        lukaszlenart Lukasz Lenart added a comment -

        Waiting on others' opinion

        Show
        lukaszlenart Lukasz Lenart added a comment - Waiting on others' opinion
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Struts-JDK7-master #502 (See https://builds.apache.org/job/Struts-JDK7-master/502/)
        WW-4669 Returns default action/method instead of throwing exception (lukaszlenart: rev 5acc71f7c30e16807912bf99b4c32cc4b25f586e)

        • core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java
        • plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java
        • core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java
        • core/src/main/java/org/apache/struts2/StrutsConstants.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Struts-JDK7-master #502 (See https://builds.apache.org/job/Struts-JDK7-master/502/ ) WW-4669 Returns default action/method instead of throwing exception (lukaszlenart: rev 5acc71f7c30e16807912bf99b4c32cc4b25f586e) core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java core/src/main/java/org/apache/struts2/StrutsConstants.java
        Show
        lukaszlenart Lukasz Lenart added a comment - Docs updated https://cwiki.apache.org/confluence/display/WW/ActionMapper#ActionMapper-AllowedactionnameRegEx
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 23e0181328bd095f50f80d4a1bf47c1620f992a0 in struts's branch refs/heads/support-2-3 from Lukasz Lenart
        [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=23e0181 ]

        WW-4669 Returns default action/method instead of throwing exception

        Show
        jira-bot ASF subversion and git services added a comment - Commit 23e0181328bd095f50f80d4a1bf47c1620f992a0 in struts's branch refs/heads/support-2-3 from Lukasz Lenart [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=23e0181 ] WW-4669 Returns default action/method instead of throwing exception
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Struts-JDK6-support-2.3 #1045 (See https://builds.apache.org/job/Struts-JDK6-support-2.3/1045/)
        WW-4669 Returns default action/method instead of throwing exception (lukaszlenart: rev 23e0181328bd095f50f80d4a1bf47c1620f992a0)

        • (edit) plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java
        • (edit) core/src/main/java/org/apache/struts2/StrutsConstants.java
        • (edit) core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java
        • (edit) core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Struts-JDK6-support-2.3 #1045 (See https://builds.apache.org/job/Struts-JDK6-support-2.3/1045/ ) WW-4669 Returns default action/method instead of throwing exception (lukaszlenart: rev 23e0181328bd095f50f80d4a1bf47c1620f992a0) (edit) plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java (edit) core/src/main/java/org/apache/struts2/StrutsConstants.java (edit) core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java (edit) core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.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