Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-73128

Jetty 12 test failure in PluginTest on Windows

    • Icon: Task Task
    • Resolution: Fixed
    • Icon: Major Major
    • core
    • None

      On the prototype branch of Jenkins core, PluginTest fails on Windows (but not on Linux) because the code under test now returns a 400 rather than 404. I have temporarily worked around the issue with the following hack:

      diff --git a/test/src/test/java/hudson/PluginTest.java b/test/src/test/java/hudson/PluginTest.java
      index 0e4192ae4b..477770d641 100644
      --- a/test/src/test/java/hudson/PluginTest.java
      +++ b/test/src/test/java/hudson/PluginTest.java
      @@ -54,7 +54,8 @@ public class PluginTest {
               r.createWebClient().assertFails("plugin/matrix-auth/images/%2e%2e%2fWEB-INF/licenses.xml", HttpServletResponse.SC_BAD_REQUEST);
               r.createWebClient().assertFails("plugin/matrix-auth/images/%2e.%2fWEB-INF/licenses.xml", HttpServletResponse.SC_BAD_REQUEST);
               r.createWebClient().assertFails("plugin/matrix-auth/images/..%2f..%2f..%2f" + r.jenkins.getRootDir().getName() + "%2fsecrets%2fmaster.key", HttpServletResponse.SC_BAD_REQUEST);
      -        r.createWebClient().assertFails("plugin/matrix-auth/" + r.jenkins.getRootDir() + "/secrets/master.key", /* ./ prepended anyway */ HttpServletResponse.SC_NOT_FOUND);
      +        // TODO Why is this behavior changing?
      +        r.createWebClient().assertFails("plugin/matrix-auth/" + r.jenkins.getRootDir() + "/secrets/master.key", /* ./ prepended anyway */ Functions.isWindows() ? HttpServletResponse.SC_BAD_REQUEST : HttpServletResponse.SC_NOT_FOU
      ND);
               // SECURITY-155:
               r.createWebClient().assertFails("plugin/matrix-auth/WEB-INF/licenses.xml", HttpServletResponse.SC_BAD_REQUEST);
               r.createWebClient().assertFails("plugin/matrix-auth/META-INF/MANIFEST.MF", HttpServletResponse.SC_BAD_REQUEST);
      

      The root cause of this change in behavior should be understood. It may actually be a desirable Jetty security enhancement. If the behavior is expected, the test should be adapted (at the very least, to remove the TODO comment). If the behavior is unexpected, the bug should be fixed and the test change reverted. Before submitting the PR to the prototype branch to fix this ticket, ensure that mvn clean verify -Dtest=hudson.PluginTest passes on both Linux and Windows.

          [JENKINS-73128] Jetty 12 test failure in PluginTest on Windows

          Basil Crow created issue -
          Basil Crow made changes -
          Description Original: On the {{prototype}} branch of Jenkins core, {{PluginTest}} fails on Windows (but not on Linux) because the code under test now returns a 400 rather than 404. I have temporarily worked around the issue with the following hack:

          {code:java}
          diff --git a/test/src/test/java/hudson/PluginTest.java b/test/src/test/java/hudson/PluginTest.java
          index 0e4192ae4b..477770d641 100644
          --- a/test/src/test/java/hudson/PluginTest.java
          +++ b/test/src/test/java/hudson/PluginTest.java
          @@ -54,7 +54,8 @@ public class PluginTest {
                   r.createWebClient().assertFails("plugin/matrix-auth/images/%2e%2e%2fWEB-INF/licenses.xml", HttpServletResponse.SC_BAD_REQUEST);
                   r.createWebClient().assertFails("plugin/matrix-auth/images/%2e.%2fWEB-INF/licenses.xml", HttpServletResponse.SC_BAD_REQUEST);
                   r.createWebClient().assertFails("plugin/matrix-auth/images/..%2f..%2f..%2f" + r.jenkins.getRootDir().getName() + "%2fsecrets%2fmaster.key", HttpServletResponse.SC_BAD_REQUEST);
          - r.createWebClient().assertFails("plugin/matrix-auth/" + r.jenkins.getRootDir() + "/secrets/master.key", /* ./ prepended anyway */ HttpServletResponse.SC_NOT_FOUND);
          + // TODO Why is this behavior changing?
          + r.createWebClient().assertFails("plugin/matrix-auth/" + r.jenkins.getRootDir() + "/secrets/master.key", /* ./ prepended anyway */ Functions.isWindows() ? HttpServletResponse.SC_BAD_REQUEST : HttpServletResponse.SC_NOT_FOU
          ND);
                   // SECURITY-155:
                   r.createWebClient().assertFails("plugin/matrix-auth/WEB-INF/licenses.xml", HttpServletResponse.SC_BAD_REQUEST);
                   r.createWebClient().assertFails("plugin/matrix-auth/META-INF/MANIFEST.MF", HttpServletResponse.SC_BAD_REQUEST);
          {code}

          The root cause of this change in behavior should be understood. It may actually be a desirable Jetty security enhancement. If the behavior is expected, the test should be adapted (at the very least, to remove the TODO comment). If the behavior is unexpected, the bug should be fixed and the test change reverted. Before submitting the PR to fix this ticket, ensure that {{mvn clean verify -Dtest=hudson.PluginTest}} passes on both Linux and Windows.
          New: On the {{prototype}} branch of Jenkins core, {{PluginTest}} fails on Windows (but not on Linux) because the code under test now returns a 400 rather than 404. I have temporarily worked around the issue with the following hack:
          {code:java}
          diff --git a/test/src/test/java/hudson/PluginTest.java b/test/src/test/java/hudson/PluginTest.java
          index 0e4192ae4b..477770d641 100644
          --- a/test/src/test/java/hudson/PluginTest.java
          +++ b/test/src/test/java/hudson/PluginTest.java
          @@ -54,7 +54,8 @@ public class PluginTest {
                   r.createWebClient().assertFails("plugin/matrix-auth/images/%2e%2e%2fWEB-INF/licenses.xml", HttpServletResponse.SC_BAD_REQUEST);
                   r.createWebClient().assertFails("plugin/matrix-auth/images/%2e.%2fWEB-INF/licenses.xml", HttpServletResponse.SC_BAD_REQUEST);
                   r.createWebClient().assertFails("plugin/matrix-auth/images/..%2f..%2f..%2f" + r.jenkins.getRootDir().getName() + "%2fsecrets%2fmaster.key", HttpServletResponse.SC_BAD_REQUEST);
          - r.createWebClient().assertFails("plugin/matrix-auth/" + r.jenkins.getRootDir() + "/secrets/master.key", /* ./ prepended anyway */ HttpServletResponse.SC_NOT_FOUND);
          + // TODO Why is this behavior changing?
          + r.createWebClient().assertFails("plugin/matrix-auth/" + r.jenkins.getRootDir() + "/secrets/master.key", /* ./ prepended anyway */ Functions.isWindows() ? HttpServletResponse.SC_BAD_REQUEST : HttpServletResponse.SC_NOT_FOU
          ND);
                   // SECURITY-155:
                   r.createWebClient().assertFails("plugin/matrix-auth/WEB-INF/licenses.xml", HttpServletResponse.SC_BAD_REQUEST);
                   r.createWebClient().assertFails("plugin/matrix-auth/META-INF/MANIFEST.MF", HttpServletResponse.SC_BAD_REQUEST);
          {code}
          The root cause of this change in behavior should be understood. It may actually be a desirable Jetty security enhancement. If the behavior is expected, the test should be adapted (at the very least, to remove the TODO comment). If the behavior is unexpected, the bug should be fixed and the test change reverted. Before submitting the PR to the {{prototype}} branch to fix this ticket, ensure that {{mvn clean verify -Dtest=hudson.PluginTest}} passes on both Linux and Windows.
          Basil Crow made changes -
          Priority Original: Minor [ 4 ] New: Major [ 3 ]
          Basil Crow made changes -
          Link New: This issue is blocking JENKINS-73130 [ JENKINS-73130 ]
          Mark Waite made changes -
          Assignee New: Mark Waite [ markewaite ]
          Mark Waite made changes -
          Status Original: Open [ 1 ] New: In Progress [ 3 ]
          Mark Waite made changes -
          Remote Link New: This issue links to "Commits that explain the HTTP 400 due to suspicious characters in the URL (Web Link)" [ 29688 ]

          Mark Waite added a comment -

          basil I believe that the comment added to my copy of PluginTest.java is ready for your review. Let me know if I've missed something in the investigation or if the comment needs further improvements before it is included in the prototype branch of Jenkins core.

          Mark Waite added a comment - basil  I believe that the comment added to my copy of PluginTest.java is ready for your review. Let me know if I've missed something in the investigation or if the comment needs further improvements before it is included in the prototype branch of Jenkins core.
          Mark Waite made changes -
          Status Original: In Progress [ 3 ] New: In Review [ 10005 ]

          Basil Crow added a comment -

          Thanks very much markewaite; those references to Servlet 6 and the Jetty issues are a great starting point for understanding the reasoning behind the change in behavior. The only thing I still wonder is whether the reasoning behind the original Jenkins security issues still applies in the context of the new Servlet 6. The tests mention SECURITY-705 and SECURITY-2481 directly (SECURITY-131 and SECURITY-155 indirectly) so I think it would be worth reading through those and confirming that the intention behind the original fixes remains in effect (even if its particular implementation may have changed in Servlet 6).

          Basil Crow added a comment - Thanks very much markewaite ; those references to Servlet 6 and the Jetty issues are a great starting point for understanding the reasoning behind the change in behavior. The only thing I still wonder is whether the reasoning behind the original Jenkins security issues still applies in the context of the new Servlet 6. The tests mention SECURITY-705 and SECURITY-2481 directly (SECURITY-131 and SECURITY-155 indirectly) so I think it would be worth reading through those and confirming that the intention behind the original fixes remains in effect (even if its particular implementation may have changed in Servlet 6).

            markewaite Mark Waite
            basil Basil Crow
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved: