diff --git a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java index 242975e2b1..fff02d56f8 100644 --- a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java +++ b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java @@ -203,15 +203,6 @@ public final class DirectoryBrowserSupport implements HttpResponse { } private void serveFile(StaplerRequest2 req, StaplerResponse2 rsp, VirtualFile root, String icon, boolean serveDirIndex) throws IOException, ServletException, InterruptedException { - // handle form submission - String pattern = req.getParameter("pattern"); - if (pattern == null) - pattern = req.getParameter("path"); // compatibility with Hudson<1.129 - if (pattern != null && Util.isSafeToRedirectTo(pattern)) { // avoid open redirect - rsp.sendRedirect2(pattern); - return; - } - String path = getPath(req); if (path.replace('\\', '/').contains("/../")) { // don't serve anything other than files in the artifacts dir diff --git a/core/src/main/resources/hudson/model/DirectoryBrowserSupport/dir.jelly b/core/src/main/resources/hudson/model/DirectoryBrowserSupport/dir.jelly index a50dd9b46c..508c0b8887 100644 --- a/core/src/main/resources/hudson/model/DirectoryBrowserSupport/dir.jelly +++ b/core/src/main/resources/hudson/model/DirectoryBrowserSupport/dir.jelly @@ -33,17 +33,18 @@ THE SOFTWARE.
-
+ ${it.owner.name} / ${p.title} / - -
+
diff --git a/core/src/main/resources/hudson/model/DirectoryBrowserSupport/pattern.js b/core/src/main/resources/hudson/model/DirectoryBrowserSupport/pattern.js new file mode 100644 index 0000000000..47ea80e647 --- /dev/null +++ b/core/src/main/resources/hudson/model/DirectoryBrowserSupport/pattern.js @@ -0,0 +1,18 @@ +document.addEventListener("DOMContentLoaded", function () { + document + .getElementById("pattern-submit") + .addEventListener("click", function (ev) { + ev.preventDefault(); + + let input = document.getElementById("pattern"); + let pattern = input.value; + let back = input.dataset.backpath; + + let baseurl = back; + if (!baseurl.endsWith("/")) { + baseurl = baseurl + "/"; + } + baseurl = baseurl + pattern; + document.location.href = baseurl; + }); +}); diff --git a/test/src/test/java/jenkins/security/Security3501Test.java b/test/src/test/java/jenkins/security/Security3501Test.java index f266b2f4c1..2128ad3074 100644 --- a/test/src/test/java/jenkins/security/Security3501Test.java +++ b/test/src/test/java/jenkins/security/Security3501Test.java @@ -4,29 +4,56 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import java.util.List; +import jenkins.security.security3501Test.Security3501RootAction; import org.htmlunit.FailingHttpStatusCodeException; import org.junit.Assert; import org.junit.Rule; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.RealJenkinsRule; +@RunWith(Parameterized.class) public class Security3501Test { + // Workaround for https://github.com/jenkinsci/jenkins-test-harness/issues/933 + private final String contextPath; + @Rule - public RealJenkinsRule jj = new RealJenkinsRule(); + public RealJenkinsRule jj = new RealJenkinsRule().addSyntheticPlugin(new RealJenkinsRule.SyntheticPlugin(Security3501RootAction.class.getPackage()).shortName("Security3501RootAction")); + + @Parameterized.Parameters + public static List contexts() { + return List.of("/jenkins", ""); + } + + public Security3501Test(String contextPath) { + jj.withPrefix(contextPath); + this.contextPath = contextPath; + } @Test public void testRedirects() throws Throwable { - jj.then(Security3501Test::_testRedirects); + jj.then(new TestRedirectsStep(contextPath)); } - public static void _testRedirects(JenkinsRule j) throws Exception { - List prohibitedPaths = List.of("%5C%5Cexample.org", "%5C/example.org", "/%5Cexample.org", "//example.org", "https://example.org", "\\example.org"); - for (String path : prohibitedPaths) { - try (JenkinsRule.WebClient wc = j.createWebClient().withRedirectEnabled(false)) { - final FailingHttpStatusCodeException fhsce = Assert.assertThrows(FailingHttpStatusCodeException.class, () -> wc.goTo("userContent?path=" + path)); - assertThat(fhsce.getStatusCode(), is(302)); - assertThat(fhsce.getResponse().getResponseHeaderValue("Location"), is(j.getURL().toExternalForm() + "userContent/")); + private record TestRedirectsStep(String context) implements RealJenkinsRule.Step { + public void run(JenkinsRule j) throws Exception { + List prohibitedPaths = List.of("%5C%5Cexample.org", "%5C/example.org", "/%5Cexample.org", "//example.org", "https://example.org", "\\example.org"); + for (String path : prohibitedPaths) { + try (JenkinsRule.WebClient wc = j.createWebClient().withRedirectEnabled(false)) { + final FailingHttpStatusCodeException fhsce = Assert.assertThrows(FailingHttpStatusCodeException.class, () -> wc.goTo("redirects/content?path=" + path)); + assertThat(fhsce.getStatusCode(), is(404)); + } + } + + List allowedPaths = List.of("foo", "foo/bar"); + for (String path : allowedPaths) { + try (JenkinsRule.WebClient wc = j.createWebClient().withRedirectEnabled(false)) { + final FailingHttpStatusCodeException fhsce = Assert.assertThrows(FailingHttpStatusCodeException.class, () -> wc.goTo("redirects/content?path=" + path)); + assertThat(fhsce.getStatusCode(), is(302)); + assertThat(fhsce.getResponse().getResponseHeaderValue("Location"), is(context + "/redirects/" + path)); + } } } } diff --git a/test/src/test/java/jenkins/security/security3501Test/Security3501RootAction.java b/test/src/test/java/jenkins/security/security3501Test/Security3501RootAction.java new file mode 100644 index 0000000000..2cdc1e3c20 --- /dev/null +++ b/test/src/test/java/jenkins/security/security3501Test/Security3501RootAction.java @@ -0,0 +1,26 @@ +package jenkins.security.security3501Test; + +import hudson.Util; +import hudson.model.InvisibleAction; +import hudson.model.RootAction; +import java.io.IOException; +import org.jenkinsci.plugins.variant.OptionalExtension; +import org.kohsuke.stapler.StaplerRequest2; +import org.kohsuke.stapler.StaplerResponse2; + +@OptionalExtension +public class Security3501RootAction extends InvisibleAction implements RootAction { + @Override + public String getUrlName() { + return "redirects"; + } + + public void doContent(StaplerRequest2 req, StaplerResponse2 rsp) throws IOException { + final String path = req.getParameter("path"); + if (path != null && Util.isSafeToRedirectTo(path)) { + rsp.sendRedirect2(path); + return; + } + rsp.setStatus(404); + } +} diff --git a/test/src/test/java/jenkins/security/security3501Test/package-info.java b/test/src/test/java/jenkins/security/security3501Test/package-info.java new file mode 100644 index 0000000000..c2e75076c8 --- /dev/null +++ b/test/src/test/java/jenkins/security/security3501Test/package-info.java @@ -0,0 +1,4 @@ +@OptionalPackage(requirePlugins = "Security3501RootAction") +package jenkins.security.security3501Test; + +import org.jenkinsci.plugins.variant.OptionalPackage;