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

Jetty 12 test failure DirectoryBrowserSupportTest

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

      On the prototype branch of Jenkins core, DirectoryBrowserSupportTest fails 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/model/DirectoryBrowserSupportTest.java b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java
      index 7b1bbdb7c9..e919223137 100644
      --- a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java
      +++ b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java
      @@ -89,6 +89,7 @@ import org.htmlunit.UnexpectedPage;
       import org.htmlunit.html.HtmlPage;
       import org.htmlunit.util.NameValuePair;
       import org.junit.Assume;
      +import org.junit.Ignore;
       import org.junit.Rule;
       import org.junit.Test;
       import org.jvnet.hudson.test.Email;
      @@ -150,7 +151,12 @@ public class DirectoryBrowserSupportTest {
               j.buildAndAssertSuccess(p);
       
               // can we see it?
      -        j.createWebClient().goTo("job/" + p.getName() + "/ws/abc%5Cdef.bin", "application/octet-stream");
      +        // TODO Why is this behavior changing?
      +        JenkinsRule.WebClient wc = j.createWebClient();
      +        wc.setThrowExceptionOnFailingStatusCode(false);
      +        HtmlPage page = wc.goTo("job/" + p.getName() + "/ws/abc%5Cdef.bin");
      +        assertEquals(400, page.getWebResponse().getStatusCode());
      +        assertEquals("Error 400 Suspicious Path Character", page.getTitleText());
           }
       
           @Test
      @@ -1108,13 +1114,16 @@ public class DirectoryBrowserSupportTest {
               String content = "random data provided as fixed value";
               Files.writeString(targetTmpPath, content, StandardCharsets.UTF_8);
       
      +        // TODO Why is this behavior changing?
               JenkinsRule.WebClient wc = j.createWebClient().withThrowExceptionOnFailingStatusCode(false);
      -        Page page = wc.goTo("userContent/" + targetTmpPath.toAbsolutePath() + "/*view*", null);
      +        HtmlPage page = wc.goTo("userContent/" + targetTmpPath.toAbsolutePath() + "/*view*");
       
      -        MatcherAssert.assertThat(page.getWebResponse().getStatusCode(), equalTo(404));
      +        assertEquals(400, page.getWebResponse().getStatusCode());
      +        assertEquals("Error 400 Suspicious Path Character", page.getTitleText());
           }
       
           @Test
      +    @Ignore("TODO Escape hatch no longer works on Jetty 12")
           @Issue("SECURITY-2481")
           public void windows_canViewAbsolutePath_withEscapeHatch() throws Exception {
               Assume.assumeTrue("can only be tested this on Windows", Functions.isWindows());
      

      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.model.DirectoryBrowserSupportTest passes on both Linux and Windows. If we decide this is a desirable change and it obviates the escape hatch, then we should just remove the escape hatch in a PR to the default branch of Jenkins core.

          [JENKINS-73129] Jetty 12 test failure DirectoryBrowserSupportTest

          Basil Crow created issue -
          Basil Crow made changes -
          Priority Original: Minor [ 4 ] New: Major [ 3 ]
          Basil Crow made changes -
          Description Original: On the {{prototype}} branch of Jenkins core, {{DirectoryBrowserSupportTest}} fails 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/model/DirectoryBrowserSupportTest.java b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java
          index 7b1bbdb7c9..e919223137 100644
          --- a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java
          +++ b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java
          @@ -89,6 +89,7 @@ import org.htmlunit.UnexpectedPage;
           import org.htmlunit.html.HtmlPage;
           import org.htmlunit.util.NameValuePair;
           import org.junit.Assume;
          +import org.junit.Ignore;
           import org.junit.Rule;
           import org.junit.Test;
           import org.jvnet.hudson.test.Email;
          @@ -150,7 +151,12 @@ public class DirectoryBrowserSupportTest {
                   j.buildAndAssertSuccess(p);
           
                   // can we see it?
          - j.createWebClient().goTo("job/" + p.getName() + "/ws/abc%5Cdef.bin", "application/octet-stream");
          + // TODO Why is this behavior changing?
          + JenkinsRule.WebClient wc = j.createWebClient();
          + wc.setThrowExceptionOnFailingStatusCode(false);
          + HtmlPage page = wc.goTo("job/" + p.getName() + "/ws/abc%5Cdef.bin");
          + assertEquals(400, page.getWebResponse().getStatusCode());
          + assertEquals("Error 400 Suspicious Path Character", page.getTitleText());
               }
           
               @Test
          @@ -1108,13 +1114,16 @@ public class DirectoryBrowserSupportTest {
                   String content = "random data provided as fixed value";
                   Files.writeString(targetTmpPath, content, StandardCharsets.UTF_8);
           
          + // TODO Why is this behavior changing?
                   JenkinsRule.WebClient wc = j.createWebClient().withThrowExceptionOnFailingStatusCode(false);
          - Page page = wc.goTo("userContent/" + targetTmpPath.toAbsolutePath() + "/*view*", null);
          + HtmlPage page = wc.goTo("userContent/" + targetTmpPath.toAbsolutePath() + "/*view*");
           
          - MatcherAssert.assertThat(page.getWebResponse().getStatusCode(), equalTo(404));
          + assertEquals(400, page.getWebResponse().getStatusCode());
          + assertEquals("Error 400 Suspicious Path Character", page.getTitleText());
               }
           
               @Test
          + @Ignore("TODO Escape hatch no longer works on Jetty 12")
               @Issue("SECURITY-2481")
               public void windows_canViewAbsolutePath_withEscapeHatch() throws Exception {
                   Assume.assumeTrue("can only be tested this on Windows", Functions.isWindows());
          {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.model.DirectoryBrowserSupportTest}} passes on both Linux and Windows. If we decide this is a desirable change and it obviates the escape hatch, then we should just remove the escape hatch in a PR to the default branch of Jenkins core.
          New: On the {{prototype}} branch of Jenkins core, {{DirectoryBrowserSupportTest}} fails 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/model/DirectoryBrowserSupportTest.java b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java
          index 7b1bbdb7c9..e919223137 100644
          --- a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java
          +++ b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java
          @@ -89,6 +89,7 @@ import org.htmlunit.UnexpectedPage;
           import org.htmlunit.html.HtmlPage;
           import org.htmlunit.util.NameValuePair;
           import org.junit.Assume;
          +import org.junit.Ignore;
           import org.junit.Rule;
           import org.junit.Test;
           import org.jvnet.hudson.test.Email;
          @@ -150,7 +151,12 @@ public class DirectoryBrowserSupportTest {
                   j.buildAndAssertSuccess(p);
           
                   // can we see it?
          - j.createWebClient().goTo("job/" + p.getName() + "/ws/abc%5Cdef.bin", "application/octet-stream");
          + // TODO Why is this behavior changing?
          + JenkinsRule.WebClient wc = j.createWebClient();
          + wc.setThrowExceptionOnFailingStatusCode(false);
          + HtmlPage page = wc.goTo("job/" + p.getName() + "/ws/abc%5Cdef.bin");
          + assertEquals(400, page.getWebResponse().getStatusCode());
          + assertEquals("Error 400 Suspicious Path Character", page.getTitleText());
               }
           
               @Test
          @@ -1108,13 +1114,16 @@ public class DirectoryBrowserSupportTest {
                   String content = "random data provided as fixed value";
                   Files.writeString(targetTmpPath, content, StandardCharsets.UTF_8);
           
          + // TODO Why is this behavior changing?
                   JenkinsRule.WebClient wc = j.createWebClient().withThrowExceptionOnFailingStatusCode(false);
          - Page page = wc.goTo("userContent/" + targetTmpPath.toAbsolutePath() + "/*view*", null);
          + HtmlPage page = wc.goTo("userContent/" + targetTmpPath.toAbsolutePath() + "/*view*");
           
          - MatcherAssert.assertThat(page.getWebResponse().getStatusCode(), equalTo(404));
          + assertEquals(400, page.getWebResponse().getStatusCode());
          + assertEquals("Error 400 Suspicious Path Character", page.getTitleText());
               }
           
               @Test
          + @Ignore("TODO Escape hatch no longer works on Jetty 12")
               @Issue("SECURITY-2481")
               public void windows_canViewAbsolutePath_withEscapeHatch() throws Exception {
                   Assume.assumeTrue("can only be tested this on Windows", Functions.isWindows());
          {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.model.DirectoryBrowserSupportTest}} passes on both Linux and Windows. If we decide this is a desirable change and it obviates the escape hatch, then we should just remove the escape hatch in a PR to the default branch of Jenkins core.
          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 "Jetty 12.0.8 and later implement the suspicious path character check from Servlet API 6.0 (Web Link)" [ 29687 ]

          Mark Waite added a comment - - edited

          There is a rare behavioral difference for Jenkins that is a result of the Jakarta Servlet Spec 6.0 implementation of the suspicious path character check.

          Jenkins 2.461 with Jetty 10

          A label can be assigned to a node with the text "g\h". The label is displayed in the agent page as a valid label. When the label is clicked from the agent page, it reports page not found / HTTP 404 for the page with URL suffix '/label/g\h'.

          If the URL is converted from the literal "g\h" sequence to the URL escaped "g%5ch" sequence, then the label is shown on the web page for the page with URL suffix '/label/g%5ch/'.

          Jenkins 2.461-rc with Jetty 12

          A label can be assigned to a node with the text "g\h". The label is displayed in the agent page as a valid label. When the label is clicked from the agent page, it reports page not found / HTTP 404 for the page with URL suffix '/label/g\h'.

          If the URL is converted from the literal "g\h" sequence to the URL escaped "g%5ch" sequence, then the page reports HTTP 400 Suspicious Path Character for the page with URL suffix '/label/g%5ch/'.

          Mark Waite added a comment - - edited There is a rare behavioral difference for Jenkins that is a result of the Jakarta Servlet Spec 6.0 implementation of the suspicious path character check. Jenkins 2.461 with Jetty 10 A label can be assigned to a node with the text "g\h". The label is displayed in the agent page as a valid label. When the label is clicked from the agent page, it reports page not found / HTTP 404 for the page with URL suffix '/label/g\h'. If the URL is converted from the literal "g\h" sequence to the URL escaped "g%5ch" sequence, then the label is shown on the web page for the page with URL suffix '/label/g%5ch/'. Jenkins 2.461-rc with Jetty 12 A label can be assigned to a node with the text "g\h". The label is displayed in the agent page as a valid label. When the label is clicked from the agent page, it reports page not found / HTTP 404 for the page with URL suffix '/label/g\h'. If the URL is converted from the literal "g\h" sequence to the URL escaped "g%5ch" sequence, then the page reports HTTP 400 Suspicious Path Character for the page with URL suffix '/label/g%5ch/'.

          Mark Waite added a comment -

          I believe that the escape hatch will need an implementation that is aware of the Jetty 12 suspicious path character implementation. The servlet spec 6.0 allows that containers may modify the suspicious path character implementation, though I do not yet know how to perform that modification.

          Mark Waite added a comment - I believe that the escape hatch will need an implementation that is aware of the Jetty 12 suspicious path character implementation. The servlet spec 6.0 allows that containers may modify the suspicious path character implementation, though I do not yet know how to perform that modification.

          Mark Waite added a comment - - edited

          basil I think that the two comments added to my copy of the DirectoryBrowserSupportTest are ready for review. Changes in that file are not complete because the escape hatch still needs to be implemented for Jetty 12, but I believe the comments are ready for your review. I haven't changed this issue to "In review" because I would like to finish the escape hatch implementation and testing before the issue is put into review.

          I'm not sure how to handle the behavioral difference that is described in the earlier comment. The initial click of the label link reports HTTP 404 for both Jetty 10 and Jetty 12, but the URL encoded label link behaves differently with Jetty 10 (HTTP 200) than with Jetty 12 (HTTP 400). Do you have a recommendation for that case?

          It seems like a bug report for the HTTP 404 is appropriate but I'm not sure which is the more appropriate behavior for the URL encoded label link. Jetty 12 complies with servlet spec 6 but I don't think that users care much about servlet spec compliance.

          Mark Waite added a comment - - edited basil I think that the two comments added to my copy of the DirectoryBrowserSupportTest are ready for review. Changes in that file are not complete because the escape hatch still needs to be implemented for Jetty 12, but I believe the comments are ready for your review. I haven't changed this issue to "In review" because I would like to finish the escape hatch implementation and testing before the issue is put into review. I'm not sure how to handle the behavioral difference that is described in the earlier comment. The initial click of the label link reports HTTP 404 for both Jetty 10 and Jetty 12, but the URL encoded label link behaves differently with Jetty 10 (HTTP 200) than with Jetty 12 (HTTP 400). Do you have a recommendation for that case? It seems like a bug report for the HTTP 404 is appropriate but I'm not sure which is the more appropriate behavior for the URL encoded label link. Jetty 12 complies with servlet spec 6 but I don't think that users care much about servlet spec compliance.

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

              Created:
              Updated:
              Resolved: