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

Struts2 Rest plugin doesn't handle JSESSIONID with DMI

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.3.24
    • Fix Version/s: 2.3.28, 2.5
    • Component/s: Plugin - REST
    • Labels:
      None
    • Environment:

      tomcat

      Description

      If a url with DMI and with a JSESSIONID is sent to a Struts2 action, a failure will result

      see this modified unit test from the plugin unit test:

          public void testGetJsessionIdSemicolonMappingWithMethod() throws Exception {
              req.setRequestURI("/myapp/animals/dog/fido!update;jsessionid=29fefpv23do1g");
              req.setServletPath("/animals/dog/fido");
              req.setMethod("GET");
      
              mapper.setAllowDynamicMethodCalls("true");
      
              ActionMapping mapping = mapper.getMapping(req, configManager);
      
              assertEquals("/animals", mapping.getNamespace());
              assertEquals("dog", mapping.getName());
              assertEquals("fido", ((String[]) mapping.getParams().get("id"))[0]);
              assertEquals("update", mapping.getMethod());
          }
      

        Activity

        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Struts-JDK7-master #495 (See https://builds.apache.org/job/Struts-JDK7-master/495/)
        WW-4585 Struts2 Rest plugin doesn't handle JSESSIONID with DMI (lukaszlenart: rev 5eaef08e52fbb46e1659b46435172d8e52b8b090)

        • plugins/rest/src/test/java/org/apache/struts2/rest/RestActionMapperTest.java
        • plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Struts-JDK7-master #495 (See https://builds.apache.org/job/Struts-JDK7-master/495/ ) WW-4585 Struts2 Rest plugin doesn't handle JSESSIONID with DMI (lukaszlenart: rev 5eaef08e52fbb46e1659b46435172d8e52b8b090) plugins/rest/src/test/java/org/apache/struts2/rest/RestActionMapperTest.java plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java
        Hide
        rpii Rich Peters added a comment -

        Ive done that. Its issue WW-4589. Don't know how to do a Pull Request in the GitHub yet, will look at that.

        Show
        rpii Rich Peters added a comment - Ive done that. Its issue WW-4589 . Don't know how to do a Pull Request in the GitHub yet, will look at that.
        Hide
        aleksandr-m Aleksandr Mashchenko added a comment -

        Rich Peters Yes, open a new issue and explain in it what do you expect to get. Right now test results show `Expected :dog` but there is no such assert in the test method you've provided.
        If you have a solution base it on master branch, please. If you need this in 2.3.x branch then we can cherry pick it from master. Additionally, instead of providing patches you can open a Pull Request in the GitHub.

        Show
        aleksandr-m Aleksandr Mashchenko added a comment - Rich Peters Yes, open a new issue and explain in it what do you expect to get. Right now test results show `Expected :dog` but there is no such assert in the test method you've provided. If you have a solution base it on master branch, please. If you need this in 2.3.x branch then we can cherry pick it from master. Additionally, instead of providing patches you can open a Pull Request in the GitHub .
        Hide
        rpii Rich Peters added a comment - - edited

        Aleksander, thanks for the fix. I tested it, and it showed another issue which I had addressed in the code I provided:
        Here is a test of it. would you like me to file a separate issue for this?

        The code I provided actually fixed a second issue with a complex id:

            public void testMappingWithMethodAndId() throws Exception {
                req.setRequestURI("/myapp/animals/dog/fido/test/some-url!update;jsessionid=29fefpv23do1g");
                req.setServletPath("/animals/dog/fido/test/some-url");
                req.setMethod("GET");
                mapper.setAllowDynamicMethodCalls("true");
                ActionMapping mapping = mapper.getMapping(req, configManager);
        
                assertEquals("/animals", mapping.getNamespace());
                assertEquals("dog/fido/test", mapping.getName());
                assertEquals("some-url", ((String[]) mapping.getParams().get("id"))[0]);
                assertEquals("update", mapping.getMethod());
            }
        

        Here is the test:

        junit.framework.ComparisonFailure: null 
        Expected :dog
        Actual   :dog/fido
          <Click to see difference>
        
        Show
        rpii Rich Peters added a comment - - edited Aleksander, thanks for the fix. I tested it, and it showed another issue which I had addressed in the code I provided: Here is a test of it. would you like me to file a separate issue for this? The code I provided actually fixed a second issue with a complex id: public void testMappingWithMethodAndId() throws Exception { req.setRequestURI( "/myapp/animals/dog/fido/test/some-url!update;jsessionid=29fefpv23do1g" ); req.setServletPath( "/animals/dog/fido/test/some-url" ); req.setMethod( "GET" ); mapper.setAllowDynamicMethodCalls( " true " ); ActionMapping mapping = mapper.getMapping(req, configManager); assertEquals( "/animals" , mapping.getNamespace()); assertEquals( "dog/fido/test" , mapping.getName()); assertEquals( "some-url" , (( String []) mapping.getParams().get( "id" ))[0]); assertEquals( "update" , mapping.getMethod()); } Here is the test: junit.framework.ComparisonFailure: null Expected :dog Actual :dog/fido <Click to see difference>
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Struts-JDK6-support-2.3 #958 (See https://builds.apache.org/job/Struts-JDK6-support-2.3/958/)
        WW-4585 Struts2 Rest plugin doesn't handle JSESSIONID with DMI (amashchenko: rev ff4cdd967031ed21740ee555d9e0c58b2033aa0c)

        • plugins/rest/src/test/java/org/apache/struts2/rest/RestActionMapperTest.java
        • plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Struts-JDK6-support-2.3 #958 (See https://builds.apache.org/job/Struts-JDK6-support-2.3/958/ ) WW-4585 Struts2 Rest plugin doesn't handle JSESSIONID with DMI (amashchenko: rev ff4cdd967031ed21740ee555d9e0c58b2033aa0c) plugins/rest/src/test/java/org/apache/struts2/rest/RestActionMapperTest.java plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java
        Hide
        rpii Rich Peters added a comment -

        Sorry about the formatting, Intellij reformatted it automatically. I have ignore white space differences on in my diff tool. Ill be more careful in the future.

        Show
        rpii Rich Peters added a comment - Sorry about the formatting, Intellij reformatted it automatically. I have ignore white space differences on in my diff tool. Ill be more careful in the future.
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Struts-JDK7-master #407 (See https://builds.apache.org/job/Struts-JDK7-master/407/)
        WW-4585 Struts2 Rest plugin doesn't handle JSESSIONID with DMI (amashchenko: rev e0003f0471f98d5986844cd799555618aae88fce)

        • plugins/rest/src/test/java/org/apache/struts2/rest/RestActionMapperTest.java
        • plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Struts-JDK7-master #407 (See https://builds.apache.org/job/Struts-JDK7-master/407/ ) WW-4585 Struts2 Rest plugin doesn't handle JSESSIONID with DMI (amashchenko: rev e0003f0471f98d5986844cd799555618aae88fce) plugins/rest/src/test/java/org/apache/struts2/rest/RestActionMapperTest.java plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java
        Hide
        aleksandr-m Aleksandr Mashchenko added a comment - - edited

        Rich Peters Fixed it in master branch also which has newer version than 2.3.24.1, that's why fix is different from yours. Next time try to preserve original source formatting when submitting patches/PR-s. Thank you for reporting.

        Show
        aleksandr-m Aleksandr Mashchenko added a comment - - edited Rich Peters Fixed it in master branch also which has newer version than 2.3.24.1, that's why fix is different from yours. Next time try to preserve original source formatting when submitting patches/PR-s. Thank you for reporting.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit ff4cdd967031ed21740ee555d9e0c58b2033aa0c in struts's branch refs/heads/support-2-3 from Aleksandr Mashchenko
        [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=ff4cdd9 ]

        WW-4585 Struts2 Rest plugin doesn't handle JSESSIONID with DMI

        Show
        jira-bot ASF subversion and git services added a comment - Commit ff4cdd967031ed21740ee555d9e0c58b2033aa0c in struts's branch refs/heads/support-2-3 from Aleksandr Mashchenko [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=ff4cdd9 ] WW-4585 Struts2 Rest plugin doesn't handle JSESSIONID with DMI
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit e0003f0471f98d5986844cd799555618aae88fce in struts's branch refs/heads/master from Aleksandr Mashchenko
        [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=e0003f0 ]

        WW-4585 Struts2 Rest plugin doesn't handle JSESSIONID with DMI

        Show
        jira-bot ASF subversion and git services added a comment - Commit e0003f0471f98d5986844cd799555618aae88fce in struts's branch refs/heads/master from Aleksandr Mashchenko [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=e0003f0 ] WW-4585 Struts2 Rest plugin doesn't handle JSESSIONID with DMI
        Hide
        rpii Rich Peters added a comment -

        FWIW, attached is a version of this file based upon 2.3.34.1 which implements this change. processing the method name correctly required the other changes in the source.

        Show
        rpii Rich Peters added a comment - FWIW, attached is a version of this file based upon 2.3.34.1 which implements this change. processing the method name correctly required the other changes in the source.
        Hide
        rpii Rich Peters added a comment - - edited

        It was actually turned on earlier in the test suite. here is the example with it turned on in the test:

            public void testGetJsessionIdSemicolonMappingWithMethod() throws Exception {
                req.setRequestURI("/myapp/animals/dog/fido!update;jsessionid=29fefpv23do1g");
                req.setServletPath("/animals/dog/fido");
                req.setMethod("GET");
                mapper.setAllowDynamicMethodCalls("true");
                ActionMapping mapping = mapper.getMapping(req, configManager);
        
                assertEquals("/animals", mapping.getNamespace());
                assertEquals("dog", mapping.getName());
                assertEquals("fido", ((String[]) mapping.getParams().get("id"))[0]);
                assertEquals("update", mapping.getMethod());
            }
        

        results:

        junit.framework.ComparisonFailure: null 
        Expected :update
        Actual   :update;jsessionid=29fefpv23do1g
        
        Show
        rpii Rich Peters added a comment - - edited It was actually turned on earlier in the test suite. here is the example with it turned on in the test: public void testGetJsessionIdSemicolonMappingWithMethod() throws Exception { req.setRequestURI( "/myapp/animals/dog/fido!update;jsessionid=29fefpv23do1g" ); req.setServletPath( "/animals/dog/fido" ); req.setMethod( "GET" ); mapper.setAllowDynamicMethodCalls( " true " ); ActionMapping mapping = mapper.getMapping(req, configManager); assertEquals( "/animals" , mapping.getNamespace()); assertEquals( "dog" , mapping.getName()); assertEquals( "fido" , (( String []) mapping.getParams().get( "id" ))[0]); assertEquals( "update" , mapping.getMethod()); } results: junit.framework.ComparisonFailure: null Expected :update Actual :update;jsessionid=29fefpv23do1g
        Hide
        lukaszlenart Lukasz Lenart added a comment -

        DMI is disabled by default, you must enabled it to work

        Show
        lukaszlenart Lukasz Lenart added a comment - DMI is disabled by default, you must enabled it to work
        Hide
        rpii Rich Peters added a comment - - edited

        my mistake... try this one

            public void testGetJsessionIdSemicolonMappingWithMethod() throws Exception {
                req.setRequestURI("/myapp/animals/dog/fido!update;jsessionid=29fefpv23do1g");
                req.setServletPath("/animals/dog/fido");
                req.setMethod("GET");
        
                ActionMapping mapping = mapper.getMapping(req, configManager);
        
                assertEquals("/animals", mapping.getNamespace());
                assertEquals("dog", mapping.getName());
                assertEquals("fido", ((String[]) mapping.getParams().get("id"))[0]);
                assertEquals("update", mapping.getMethod());
        

        gives

        junit.framework.ComparisonFailure: null 
        Expected :update
        Actual   :show
          <Click to see difference>
        
        
        
        	at junit.framework.Assert.assertEquals(Assert.java:81)
        	at junit.framework.Assert.assertEquals(Assert.java:87)
        	at org.apache.struts2.rest.RestActionMapperTest.testGetJsessionIdSemicolonMappingWithMethod(RestActionMapperTest.java:192)
        	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        	at java.lang.reflect.Method.invoke(Method.java:497)
        	at junit.framework.TestCase.runTest(TestCase.java:168)
        	at junit.framework.TestCase.runBare(TestCase.java:134)
        	at junit.framework.TestResult$1.protect(TestResult.java:110)
        	at junit.framework.TestResult.runProtected(TestResult.java:128)
        	at junit.framework.TestResult.run(TestResult.java:113)
        	at junit.framework.TestCase.run(TestCase.java:124)
        	at junit.framework.TestSuite.runTest(TestSuite.java:243)
        	at junit.framework.TestSuite.run(TestSuite.java:238)
        	at org.junit.internal.runners.JUnit38ClassRunner.run(JUnit38ClassRunner.java:83)
        	at org.junit.runner.JUnitCore.run(JUnitCore.java:157)
        	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:69)
        	at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:234)
        	at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:74)
        	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        	at java.lang.reflect.Method.invoke(Method.java:497)
        	at com.intellij.rt.execution.application.AppMain.main(AppMain.java:144)
            }
        
        Show
        rpii Rich Peters added a comment - - edited my mistake... try this one public void testGetJsessionIdSemicolonMappingWithMethod() throws Exception { req.setRequestURI( "/myapp/animals/dog/fido!update;jsessionid=29fefpv23do1g" ); req.setServletPath( "/animals/dog/fido" ); req.setMethod( "GET" ); ActionMapping mapping = mapper.getMapping(req, configManager); assertEquals( "/animals" , mapping.getNamespace()); assertEquals( "dog" , mapping.getName()); assertEquals( "fido" , (( String []) mapping.getParams().get( "id" ))[0]); assertEquals( "update" , mapping.getMethod()); gives junit.framework.ComparisonFailure: null Expected :update Actual :show <Click to see difference> at junit.framework.Assert.assertEquals(Assert.java:81) at junit.framework.Assert.assertEquals(Assert.java:87) at org.apache.struts2. rest .RestActionMapperTest.testGetJsessionIdSemicolonMappingWithMethod(RestActionMapperTest.java:192) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:497) at junit.framework.TestCase.runTest(TestCase.java:168) at junit.framework.TestCase.runBare(TestCase.java:134) at junit.framework.TestResult$1.protect(TestResult.java:110) at junit.framework.TestResult.runProtected(TestResult.java:128) at junit.framework.TestResult.run(TestResult.java:113) at junit.framework.TestCase.run(TestCase.java:124) at junit.framework.TestSuite.runTest(TestSuite.java:243) at junit.framework.TestSuite.run(TestSuite.java:238) at org.junit.internal.runners.JUnit38ClassRunner.run(JUnit38ClassRunner.java:83) at org.junit.runner.JUnitCore.run(JUnitCore.java:157) at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:69) at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:234) at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:74) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:497) at com.intellij.rt.execution.application.AppMain.main(AppMain.java:144) }
        Hide
        aleksandr-m Aleksandr Mashchenko added a comment - - edited

        What did you change in it? Looks the same and runs fine.

        Show
        aleksandr-m Aleksandr Mashchenko added a comment - - edited What did you change in it? Looks the same and runs fine.

          People

          • Assignee:
            aleksandr-m Aleksandr Mashchenko
            Reporter:
            rpii Rich Peters
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development