From 82dbd2fdd379baa56d25f3b242ad0873166393f9 Mon Sep 17 00:00:00 2001 From: "Simeon L." Date: Mon, 7 Oct 2024 00:29:57 +0200 Subject: [PATCH 01/21] feat: JENKINS-73837 - Adjusted DirScanner to allow empty directories in the TarArchiver --- .../src/main/java/hudson/util/DirScanner.java | 7 ++++ .../java/hudson/util/io/TarArchiverTest.java | 34 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/core/src/main/java/hudson/util/DirScanner.java b/core/src/main/java/hudson/util/DirScanner.java index a1c79c8c1a9b..90aeac773c0c 100644 --- a/core/src/main/java/hudson/util/DirScanner.java +++ b/core/src/main/java/hudson/util/DirScanner.java @@ -9,6 +9,7 @@ import java.io.IOException; import java.io.Serializable; import java.nio.file.OpenOption; +import java.util.Arrays; import java.util.HashSet; import java.util.Set; import org.apache.tools.ant.BuildException; @@ -146,6 +147,12 @@ public void scan(File dir, FileVisitor visitor) throws IOException { File file = new File(dir, f); scanSingle(file, f, visitor); } + for (String d : ds.getIncludedDirectories()) { + if (!d.isEmpty()) { + File file = new File(dir, d); + scanSingle(file, d, visitor); + } + } } } diff --git a/core/src/test/java/hudson/util/io/TarArchiverTest.java b/core/src/test/java/hudson/util/io/TarArchiverTest.java index c0f41981c27f..7522915fc71c 100644 --- a/core/src/test/java/hudson/util/io/TarArchiverTest.java +++ b/core/src/test/java/hudson/util/io/TarArchiverTest.java @@ -36,9 +36,18 @@ import hudson.util.StreamTaskListener; import java.io.File; import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.nio.file.Path; import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +import org.apache.tools.tar.TarEntry; +import org.apache.tools.tar.TarInputStream; +import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -141,6 +150,31 @@ private static void run(FilePath dir, String... cmds) throws InterruptedExceptio t1.join(); } + @Ignore("Empty Directory Test") + @Issue("JENKINS-73837") + @Test + public void emptyDirectory() throws Exception { + Path tar = tmp.newFile("test.tar").toPath(); + Path root = tmp.newFolder().toPath(); + Files.createDirectory(root.resolve("a")); + Files.createDirectory(root.resolve("b")); + + Files.writeString(root.resolve("a/file.txt"), "empty_dir_test"); + + try (OutputStream outputStream = Files.newOutputStream(tar)) { + new FilePath(root.toFile()).tar(outputStream, "**"); + } + + Set names = new HashSet<>(); + try (TarInputStream tarInputStream = new TarInputStream(Files.newInputStream(tar), StandardCharsets.UTF_8.name())) { + TarEntry tarEntry; + while ((tarEntry = tarInputStream.getNextEntry()) != null) { + names.add(tarEntry.getName()); + } + } + assertEquals(Set.of("a/", "b/", "a/file.txt"), names); + } + private static class GrowingFileRunnable implements Runnable { private boolean finish = false; private Exception ex = null; From 4f3652e6ce11b6283ff81744d57c6c258072d3c5 Mon Sep 17 00:00:00 2001 From: "Simeon L." Date: Mon, 7 Oct 2024 00:45:05 +0200 Subject: [PATCH 02/21] fix: Fixed spotless format violations --- core/src/main/java/hudson/util/DirScanner.java | 1 - core/src/test/java/hudson/util/io/TarArchiverTest.java | 4 ---- 2 files changed, 5 deletions(-) diff --git a/core/src/main/java/hudson/util/DirScanner.java b/core/src/main/java/hudson/util/DirScanner.java index 90aeac773c0c..4ca35ecb14e9 100644 --- a/core/src/main/java/hudson/util/DirScanner.java +++ b/core/src/main/java/hudson/util/DirScanner.java @@ -9,7 +9,6 @@ import java.io.IOException; import java.io.Serializable; import java.nio.file.OpenOption; -import java.util.Arrays; import java.util.HashSet; import java.util.Set; import org.apache.tools.ant.BuildException; diff --git a/core/src/test/java/hudson/util/io/TarArchiverTest.java b/core/src/test/java/hudson/util/io/TarArchiverTest.java index 7522915fc71c..55360e1cdf2f 100644 --- a/core/src/test/java/hudson/util/io/TarArchiverTest.java +++ b/core/src/test/java/hudson/util/io/TarArchiverTest.java @@ -36,7 +36,6 @@ import hudson.util.StreamTaskListener; import java.io.File; import java.io.IOException; -import java.io.InputStream; import java.io.OutputStream; import java.nio.charset.StandardCharsets; import java.nio.file.Files; @@ -44,10 +43,8 @@ import java.util.Arrays; import java.util.HashSet; import java.util.Set; - import org.apache.tools.tar.TarEntry; import org.apache.tools.tar.TarInputStream; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -150,7 +147,6 @@ private static void run(FilePath dir, String... cmds) throws InterruptedExceptio t1.join(); } - @Ignore("Empty Directory Test") @Issue("JENKINS-73837") @Test public void emptyDirectory() throws Exception { From 4328df6956f2af2be617fb267a29c49b4034f00a Mon Sep 17 00:00:00 2001 From: "Simeon L." Date: Mon, 7 Oct 2024 23:42:31 +0200 Subject: [PATCH 03/21] fix: Fixed most VirtualFileTests --- .../test/java/jenkins/util/VirtualFileTest.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/core/src/test/java/jenkins/util/VirtualFileTest.java b/core/src/test/java/jenkins/util/VirtualFileTest.java index af27d3a07ac4..35acd0db2500 100644 --- a/core/src/test/java/jenkins/util/VirtualFileTest.java +++ b/core/src/test/java/jenkins/util/VirtualFileTest.java @@ -27,6 +27,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.containsInRelativeOrder; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.greaterThan; @@ -237,7 +238,7 @@ public void list_Glob_NoFollowLinks_FileVF() throws Exception { File root = tmp.getRoot(); VirtualFile virtualRoot = VirtualFile.forFile(root); Collection children = virtualRoot.list("**", null, true, LinkOption.NOFOLLOW_LINKS); - assertThat(children, containsInAnyOrder( + assertThat(children, containsInRelativeOrder( "a/aa/aa.txt", "a/ab/ab.txt", "b/ba/ba.txt" @@ -281,7 +282,8 @@ public void zip_NoFollowLinks_FilePathVF() throws Exception { assertTrue(unzipPath.isDirectory()); assertTrue(unzipPath.child("a").child("aa").child("aa.txt").exists()); assertTrue(unzipPath.child("a").child("ab").child("ab.txt").exists()); - assertFalse(unzipPath.child("a").child("aa").child("aaa").exists()); + // JENKINS-73837 - Empty directories are included in the zip file + assertTrue(unzipPath.child("a").child("aa").child("aaa").exists()); assertFalse(unzipPath.child("a").child("_b").exists()); assertTrue(unzipPath.child("b").child("ba").child("ba.txt").exists()); assertFalse(unzipPath.child("b").child("_a").exists()); @@ -311,7 +313,8 @@ public void zip_NoFollowLinks_FilePathVF_withPrefix() throws Exception { assertTrue(unzipPath.child(prefix).isDirectory()); assertTrue(unzipPath.child(prefix).child("a").child("aa").child("aa.txt").exists()); assertTrue(unzipPath.child(prefix).child("a").child("ab").child("ab.txt").exists()); - assertFalse(unzipPath.child(prefix).child("a").child("aa").child("aaa").exists()); + // JENKINS-73837 - Empty directories are included in the zip file + assertTrue(unzipPath.child(prefix).child("a").child("aa").child("aaa").exists()); assertFalse(unzipPath.child(prefix).child("a").child("_b").exists()); assertTrue(unzipPath.child(prefix).child("b").child("ba").child("ba.txt").exists()); assertFalse(unzipPath.child(prefix).child("b").child("_a").exists()); @@ -339,7 +342,8 @@ public void zip_NoFollowLinks_FileVF() throws Exception { assertTrue(unzipPath.isDirectory()); assertTrue(unzipPath.child("a").child("aa").child("aa.txt").exists()); assertTrue(unzipPath.child("a").child("ab").child("ab.txt").exists()); - assertFalse(unzipPath.child("a").child("aa").child("aaa").exists()); + // JENKINS-73837 - Empty directories are included in the zip file + assertTrue(unzipPath.child("a").child("aa").child("aaa").exists()); assertFalse(unzipPath.child("a").child("_b").exists()); assertTrue(unzipPath.child("b").child("ba").child("ba.txt").exists()); assertFalse(unzipPath.child("b").child("_a").exists()); @@ -369,7 +373,8 @@ public void zip_NoFollowLinks_FileVF_withPrefix() throws Exception { assertTrue(unzipPath.child(prefix).isDirectory()); assertTrue(unzipPath.child(prefix).child("a").child("aa").child("aa.txt").exists()); assertTrue(unzipPath.child(prefix).child("a").child("ab").child("ab.txt").exists()); - assertFalse(unzipPath.child(prefix).child("a").child("aa").child("aaa").exists()); + // JENKINS-73837 - Empty directories are included in the zip file + assertTrue(unzipPath.child(prefix).child("a").child("aa").child("aaa").exists()); assertFalse(unzipPath.child(prefix).child("a").child("_b").exists()); assertTrue(unzipPath.child(prefix).child("b").child("ba").child("ba.txt").exists()); assertFalse(unzipPath.child(prefix).child("b").child("_a").exists()); From 4cea26a1277d1c1d81086ade1798af76cb89b309 Mon Sep 17 00:00:00 2001 From: "Simeon L." Date: Wed, 9 Oct 2024 01:16:24 +0200 Subject: [PATCH 04/21] feat: JENKINS-73837 - Updated the testEOFbrokenFlush Test --- core/src/test/java/hudson/FilePathTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/test/java/hudson/FilePathTest.java b/core/src/test/java/hudson/FilePathTest.java index 54d37c2b3dcd..b287d7d84998 100644 --- a/core/src/test/java/hudson/FilePathTest.java +++ b/core/src/test/java/hudson/FilePathTest.java @@ -806,7 +806,7 @@ public void testEOFbrokenFlush() throws IOException, InterruptedException { // Compress archive final FilePath tmpDirPath = new FilePath(srcFolder); int tarred = tmpDirPath.tar(Files.newOutputStream(archive.toPath()), "**"); - assertEquals("One file should have been compressed", 3, tarred); + assertEquals("One file should have been compressed", 4, tarred); // Decompress final File dstFolder = temp.newFolder("dst"); From 20bf90da7a56230a59fd6f15b5e66c4d54b1da64 Mon Sep 17 00:00:00 2001 From: "Simeon L." Date: Wed, 9 Oct 2024 01:35:48 +0200 Subject: [PATCH 05/21] fix: JENKINS-73837 - Fixed most of the DirectoryBrowserSupportTests --- .../model/DirectoryBrowserSupportTest.java | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java index fbd1173909df..53eed2aef228 100644 --- a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java +++ b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java @@ -31,6 +31,7 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertEquals; @@ -915,7 +916,8 @@ public void junctionAndSymlink_outsideWorkspace_areNotAllowed_windowsJunction() assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, containsInAnyOrder( + // JENKINS-73837 - Using hasItems instead of containsInAnyOrder to allow unmatched directories to be ignored + assertThat(entryNames, hasItems( p.getName() + "/intermediateFolder/public2.key", p.getName() + "/public1.key" )); @@ -925,7 +927,8 @@ public void junctionAndSymlink_outsideWorkspace_areNotAllowed_windowsJunction() assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, contains("intermediateFolder/public2.key")); + // JENKINS-73837 - Using hasItems instead of containsInAnyOrder to allow unmatched directories to be ignored + assertThat(entryNames, hasItems("intermediateFolder/public2.key")); } // Explicitly delete everything including junctions, which TemporaryDirectoryAllocator.dispose may have trouble with: new Launcher.LocalLauncher(StreamTaskListener.fromStderr()).launch().cmds("cmd", "/c", "rmdir", "/s", "/q", j.jenkins.getRootDir().getAbsolutePath()).start().join(); @@ -987,7 +990,8 @@ public void directSymlink_forTestingZip() throws Exception { assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, hasSize(0)); + // JENKINS-73837 - Allows empty folders. + assertThat(entryNames, hasSize(6)); } { Page zipPage = wc.goTo(p.getUrl() + "ws/a1/*zip*/a1.zip", null); @@ -1270,8 +1274,10 @@ public boolean perform(AbstractBuild build, Launcher launcher, BuildListen assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, hasSize(2)); - assertThat(entryNames, containsInAnyOrder( + // JENKINS-73837 - Allows empty folders. + assertThat(entryNames, hasSize(4)); + // JENKINS-73837 - Using hasItems instead of containsInAnyOrder to allow unmatched directories to be ignored + assertThat(entryNames, hasItems( "test0/anotherDir/one.txt", "test0/anotherDir/insideDir/two.txt" )); @@ -1280,8 +1286,10 @@ public boolean perform(AbstractBuild build, Launcher launcher, BuildListen assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, hasSize(2)); - assertThat(entryNames, containsInAnyOrder( + // JENKINS-73837 - Allows empty folders. + assertThat(entryNames, hasSize(3)); + // JENKINS-73837 - Using hasItems instead of containsInAnyOrder to allow unmatched directories to be ignored + assertThat(entryNames, hasItems( "anotherDir/one.txt", "anotherDir/insideDir/two.txt" )); From 14c51694a0994909c555abe79ac401fb066d95ee Mon Sep 17 00:00:00 2001 From: Simeon L Date: Wed, 9 Oct 2024 13:19:19 +0200 Subject: [PATCH 06/21] fix: Updated VirtualFileTests and DirectoryBrowserSupportTests to allow empty folders --- core/src/test/java/jenkins/util/VirtualFileTest.java | 10 +++++++--- .../hudson/model/DirectoryBrowserSupportTest.java | 12 ++++++++---- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/core/src/test/java/jenkins/util/VirtualFileTest.java b/core/src/test/java/jenkins/util/VirtualFileTest.java index 35acd0db2500..3898e9397a46 100644 --- a/core/src/test/java/jenkins/util/VirtualFileTest.java +++ b/core/src/test/java/jenkins/util/VirtualFileTest.java @@ -31,6 +31,7 @@ import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.lessThan; @@ -254,7 +255,8 @@ public void list_Glob_NoFollowLinks_FilePathVF() throws Exception { File root = tmp.getRoot(); VirtualFile virtualRoot = VirtualFile.forFilePath(new FilePath(root)); Collection children = virtualRoot.list("**", null, true, LinkOption.NOFOLLOW_LINKS); - assertThat(children, containsInAnyOrder( + // JENKINS-73837 - Since we now allow empty directories, this had to be changed to hasItems + assertThat(children, hasItems( "a/aa/aa.txt", "a/ab/ab.txt", "b/ba/ba.txt" @@ -509,7 +511,8 @@ public void list_Glob_NoFollowLinks_ExternalSymlink_FilePathVF() throws Exceptio VirtualFile symlinkVirtualPath = VirtualFile.forFilePath(symlinkPath); VirtualFile symlinkChildVirtualPath = symlinkVirtualPath.child("aa"); Collection children = symlinkChildVirtualPath.list("**", null, true, LinkOption.NOFOLLOW_LINKS); - assertThat(children, contains("aa.txt")); + // JENKINS-73837 - Since we now allow empty directories, this had to be changed to hasItems + assertThat(children, hasItems("aa.txt")); } @Test @@ -524,7 +527,8 @@ public void list_Glob_NoFollowLinks_ExternalSymlink_FileVF() throws Exception { VirtualFile symlinkVirtualFile = VirtualFile.forFile(symlinkFile); VirtualFile symlinkChildVirtualFile = symlinkVirtualFile.child("aa"); Collection children = symlinkChildVirtualFile.list("**", null, true, LinkOption.NOFOLLOW_LINKS); - assertThat(children, contains("aa.txt")); + // JENKINS-73837 - Since we now allow empty directories, this had to be changed to hasItems + assertThat(children, hasItems("aa.txt")); } @Test diff --git a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java index 53eed2aef228..e7fb019b1cd8 100644 --- a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java +++ b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java @@ -686,7 +686,8 @@ public void symlink_outsideWorkspace_areNotAllowed() throws Exception { assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, containsInAnyOrder( + // JENKINS-73837 - Since we now allow empty directories, this had to be changed to hasItems + assertThat(entryNames, hasItems( p.getName() + "/intermediateFolder/public2.key", p.getName() + "/public1.key" )); @@ -696,7 +697,8 @@ public void symlink_outsideWorkspace_areNotAllowed() throws Exception { assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, containsInAnyOrder( + // JENKINS-73837 - Since we now allow empty directories, this had to be changed to hasItems + assertThat(entryNames, hasItems( "intermediateFolder/public2.key", "public1.key" )); @@ -706,14 +708,16 @@ public void symlink_outsideWorkspace_areNotAllowed() throws Exception { assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, contains("intermediateFolder/public2.key")); + // JENKINS-73837 - Since we now allow empty directories, this had to be changed to hasItems + assertThat(entryNames, hasItems("intermediateFolder/public2.key")); } { // workaround for JENKINS-19947 is still supported, i.e. no parent folder, even inside a sub-folder Page zipPage = wc.goTo(p.getUrl() + "ws/intermediateFolder/**/*zip*/intermediateFolder.zip", null); assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, contains("public2.key")); + // JENKINS-73837 - Since we now allow empty directories, this had to be changed to hasItems + assertThat(entryNames, hasItems("public2.key")); } } From 879bbdabaa6293b8aa03f615f162e5899d494dab Mon Sep 17 00:00:00 2001 From: Simeon L Date: Wed, 9 Oct 2024 15:03:10 +0200 Subject: [PATCH 07/21] fix: JENKINS-73837 - removed unused import to fix spotbugs check --- test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java index e7fb019b1cd8..63b11d98c4ef 100644 --- a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java +++ b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java @@ -26,7 +26,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.allOf; -import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; From 3f887f43266d1c090e731c8508eb9ae188692216 Mon Sep 17 00:00:00 2001 From: "Simeon L." Date: Sun, 13 Oct 2024 15:43:01 +0200 Subject: [PATCH 08/21] fix: Updated VirtualFile#collectFiles to fix VirtualFileTest#list --- .../main/java/jenkins/util/VirtualFile.java | 718 ++++++++++-------- .../java/jenkins/util/VirtualFileTest.java | 14 +- 2 files changed, 395 insertions(+), 337 deletions(-) diff --git a/core/src/main/java/jenkins/util/VirtualFile.java b/core/src/main/java/jenkins/util/VirtualFile.java index 6c754a50bbc4..57374d2fd86f 100644 --- a/core/src/main/java/jenkins/util/VirtualFile.java +++ b/core/src/main/java/jenkins/util/VirtualFile.java @@ -38,6 +38,7 @@ import hudson.util.IOUtils; import hudson.util.io.Archiver; import hudson.util.io.ArchiverFactory; + import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; @@ -60,6 +61,7 @@ import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; + import jenkins.MasterToSlaveFileCallable; import jenkins.model.ArtifactManager; import jenkins.security.MasterToSlaveCallable; @@ -87,7 +89,7 @@ * {@link File}s somewhere. This makes VirtualFile more abstract. * *

Opening files from other machines

- * + *

* While {@link VirtualFile} is marked {@link Serializable}, * it is not safe in general to transfer over a Remoting channel. * (For example, an implementation from {@link #forFilePath} could be sent on the same channel, @@ -114,8 +116,9 @@ public abstract class VirtualFile implements Comparable, Serializab /** * Gets the base name, meaning just the last portion of the path name without any * directories. - * + *

* For a “root directory” this may be the empty string. + * * @return a simple name (no slashes) */ public abstract @NonNull String getName(); @@ -125,6 +128,7 @@ public abstract class VirtualFile implements Comparable, Serializab * Should at least uniquely identify this virtual file within its root, but not necessarily globally. *

When {@link #toExternalURL} is implemented, that same value could be used here, * unless some sort of authentication is also embedded. + * * @return a URI (need not be absolute) */ public abstract @NonNull URI toURI(); @@ -132,12 +136,14 @@ public abstract class VirtualFile implements Comparable, Serializab /** * Gets the parent file. * Need only operate within the originally given root. + * * @return the parent */ public abstract VirtualFile getParent(); /** * Checks whether this file exists and is a directory. + * * @return true if it is a directory, false if a file or nonexistent * @throws IOException in case checking status failed */ @@ -145,6 +151,7 @@ public abstract class VirtualFile implements Comparable, Serializab /** * Checks whether this file exists and is a plain file. + * * @return true if it is a file, false if a directory or nonexistent * @throws IOException in case checking status failed */ @@ -154,6 +161,7 @@ public abstract class VirtualFile implements Comparable, Serializab * If this file is a symlink, returns the link target. *

The default implementation always returns null. * Some implementations may not support symlinks under any conditions. + * * @return a target (typically a relative path in some format), or null if this is not a link * @throws IOException if reading the link, or even determining whether this file is a link, failed * @since 2.118 @@ -165,6 +173,7 @@ public abstract class VirtualFile implements Comparable, Serializab /** * Checks whether this file exists. * The behavior is undefined for symlinks; if in doubt, check {@link #readLink} first. + * * @return true if it is a plain file or directory, false if nonexistent * @throws IOException in case checking status failed */ @@ -180,12 +189,13 @@ public abstract class VirtualFile implements Comparable, Serializab /** * Lists children of this directory. Only one level deep. - * + *

* This is intended to allow the caller to provide {@link java.nio.file.LinkOption#NOFOLLOW_LINKS} to ignore * symlinks. However, this cannot be enforced. The base implementation here in VirtualFile ignores the openOptions. * Some VirtualFile subclasses may not be able to provide * an implementation in which NOFOLLOW_LINKS is used or makes sense. Implementations are free * to ignore openOptions. Some subclasses of VirtualFile may not have a concept of symlinks. + * * @param openOptions the options to apply when opening. * @return a list of children (files and subdirectories); empty for a file or nonexistent directory * @throws IOException if it could not be opened @@ -205,8 +215,9 @@ public boolean supportsQuickRecursiveListing() { * A recognized symlink can be the file itself or any containing directory between * it and the optional root directory. If there is no provided root directory then * only the file itself is considered. - * + *

* This base implementation ignores the existence of symlinks. + * * @param openOptions the various open options to apply to the operation. * @return True if the file is a symlink or is referenced within a containing symlink. * directory before reaching the root directory. @@ -248,9 +259,10 @@ public boolean hasSymlink(OpenOption... openOptions) throws IOException { * Lists recursive files of this directory with pattern matching. *

The default implementation calls {@link #list()} recursively inside {@link #run} and applies filtering to the result. * Implementations may wish to override this more efficiently. - * @param includes comma-separated Ant-style globs as per {@link Util#createFileSet(File, String, String)} using {@code /} as a path separator; - * the empty string means no matches (use {@link SelectorUtils#DEEP_TREE_MATCH} if you want to match everything except some excludes) - * @param excludes optional excludes in similar format to {@code includes} + * + * @param includes comma-separated Ant-style globs as per {@link Util#createFileSet(File, String, String)} using {@code /} as a path separator; + * the empty string means no matches (use {@link SelectorUtils#DEEP_TREE_MATCH} if you want to match everything except some excludes) + * @param excludes optional excludes in similar format to {@code includes} * @param useDefaultExcludes as per {@link AbstractFileSet#setDefaultexcludes} * @return a list of {@code /}-separated relative names of children (files directly inside or in subdirectories) * @throws IOException if this is not a directory, or listing was not possible for some other reason @@ -265,15 +277,16 @@ public boolean hasSymlink(OpenOption... openOptions) throws IOException { * *

The default implementation calls {@link #list()} recursively inside {@link #run} and applies filtering to the result. * Implementations may wish to override this more efficiently. - + *

* This method allows the user to specify that symlinks should not be followed by passing * LinkOption.NOFOLLOW_LINKS as true. However, some implementations may not be able to reliably * prevent link following. The base implementation here in VirtualFile ignores this parameter. - * @param includes comma-separated Ant-style globs as per {@link Util#createFileSet(File, String, String)} using {@code /} as a path separator; - * the empty string means no matches (use {@link SelectorUtils#DEEP_TREE_MATCH} if you want to match everything except some excludes) - * @param excludes optional excludes in similar format to {@code includes} + * + * @param includes comma-separated Ant-style globs as per {@link Util#createFileSet(File, String, String)} using {@code /} as a path separator; + * the empty string means no matches (use {@link SelectorUtils#DEEP_TREE_MATCH} if you want to match everything except some excludes) + * @param excludes optional excludes in similar format to {@code includes} * @param useDefaultExcludes as per {@link AbstractFileSet#setDefaultexcludes} - * @param openOptions the options to apply when opening. + * @param openOptions the options to apply when opening. * @return a list of {@code /}-separated relative names of children (files directly inside or in subdirectories) * @throws IOException if this is not a directory, or listing was not possible for some other reason * @since 2.275 and 2.263.2 @@ -327,11 +340,9 @@ public Collection call() throws IOException { private static void collectFiles(VirtualFile d, Collection names, String prefix) throws IOException { for (VirtualFile child : d.list()) { - if (child.isFile()) { - names.add(prefix + child.getName()); - } else if (child.isDirectory()) { - collectFiles(child, names, prefix + child.getName() + "/"); - } + // JENKINS-73837 - Empty Directories are now included in the list + names.add(prefix + child.getName()); + collectFiles(child, names, prefix + child.getName() + "/"); } } } @@ -354,13 +365,13 @@ private List patterns(String patts) { * *

The default implementation calls other existing methods to list the folders/files, then retrieve them and zip them all. * - * @param includes comma-separated Ant-style globs as per {@link Util#createFileSet(File, String, String)} using {@code /} as a path separator; - * the empty string means no matches (use {@link SelectorUtils#DEEP_TREE_MATCH} if you want to match everything except some excludes) - * @param excludes optional excludes in similar format to {@code includes} + * @param includes comma-separated Ant-style globs as per {@link Util#createFileSet(File, String, String)} using {@code /} as a path separator; + * the empty string means no matches (use {@link SelectorUtils#DEEP_TREE_MATCH} if you want to match everything except some excludes) + * @param excludes optional excludes in similar format to {@code includes} * @param useDefaultExcludes as per {@link AbstractFileSet#setDefaultexcludes} - * @param prefix the partial path that will be added before each entry inside the archive. - * If non-empty, a trailing slash will be enforced. - * @param openOptions the options to apply when opening. + * @param prefix the partial path that will be added before each entry inside the archive. + * If non-empty, a trailing slash will be enforced. + * @param openOptions the options to apply when opening. * @return the number of files inside the archive (not the folders) * @throws IOException if this is not a directory, or listing was not possible for some other reason * @since 2.275 and 2.263.2 @@ -400,14 +411,14 @@ private void sendOneZipEntry(ZipOutputStream zos, VirtualFile vf, String relativ try (InputStream in = vf.open(openOptions)) { // hudson.util.IOUtils is already present org.apache.commons.io.IOUtils.copy(in, zos); - } - finally { + } finally { zos.closeEntry(); } } /** * Obtains a child file. + * * @param name a relative path, possibly including {@code /} (but not {@code ..}) * @return a representation of that child, whether it actually exists or not */ @@ -415,6 +426,7 @@ private void sendOneZipEntry(ZipOutputStream zos, VirtualFile vf, String relativ /** * Gets the file length. + * * @return a length, or 0 if inapplicable (e.g. a directory) * @throws IOException if checking the length failed */ @@ -422,6 +434,7 @@ private void sendOneZipEntry(ZipOutputStream zos, VirtualFile vf, String relativ /** * Gets the file timestamp. + * * @return a length, or 0 if inapplicable * @throws IOException if checking the timestamp failed */ @@ -430,6 +443,7 @@ private void sendOneZipEntry(ZipOutputStream zos, VirtualFile vf, String relativ /** * Gets the file’s Unix mode, if meaningful. * If the file is symlink (see {@link #readLink}), the mode is that of the link target, not the link itself. + * * @return for example, 0644 ~ {@code rw-r--r--}; -1 by default, meaning unknown or inapplicable * @throws IOException if checking the mode failed * @since 2.118 @@ -440,6 +454,7 @@ public int mode() throws IOException { /** * Checks whether this file can be read. + * * @return true normally * @throws IOException if checking status failed */ @@ -447,6 +462,7 @@ public int mode() throws IOException { /** * Opens an input stream on the file so its contents can be read. + * * @return an open stream * @throws IOException if it could not be opened */ @@ -468,7 +484,8 @@ public InputStream open(OpenOption... openOptions) throws IOException { * Does case-insensitive comparison. * {@inheritDoc} */ - @Override public final int compareTo(VirtualFile o) { + @Override + public final int compareTo(VirtualFile o) { return getName().compareToIgnoreCase(o.getName()); } @@ -476,7 +493,8 @@ public InputStream open(OpenOption... openOptions) throws IOException { * Compares according to {@link #toURI}. * {@inheritDoc} */ - @Override public final boolean equals(Object obj) { + @Override + public final boolean equals(Object obj) { return obj instanceof VirtualFile && toURI().equals(((VirtualFile) obj).toURI()); } @@ -484,7 +502,8 @@ public InputStream open(OpenOption... openOptions) throws IOException { * Hashes according to {@link #toURI}. * {@inheritDoc} */ - @Override public final int hashCode() { + @Override + public final int hashCode() { return toURI().hashCode(); } @@ -492,7 +511,8 @@ public InputStream open(OpenOption... openOptions) throws IOException { * Displays {@link #toURI}. * {@inheritDoc} */ - @Override public final String toString() { + @Override + public final String toString() { return toURI().toString(); } @@ -500,7 +520,8 @@ public InputStream open(OpenOption... openOptions) throws IOException { * Does some calculations in batch. * For a remote file, this can be much faster than doing the corresponding operations one by one as separate requests. * The default implementation just calls the block directly. - * @param a value type + * + * @param a value type * @param callable something to run all at once (only helpful if any mentioned files are on the same system) * @return the callable result * @throws IOException if remote communication failed @@ -523,9 +544,10 @@ public V run(Callable callable) throws IOException { * Authentication should be limited to download, not upload or any other modifications. *

The URL might be valid for only a limited amount of time or even only a single use; * this method should be called anew every time an external URL is required. + * * @return an externally usable URL like {@code https://gist.githubusercontent.com/ACCT/GISTID/raw/COMMITHASH/FILE}, or null if there is no such support - * @since 2.118 * @see #toURI + * @since 2.118 */ public @CheckForNull URL toExternalURL() throws IOException { return null; @@ -533,7 +555,7 @@ public V run(Callable callable) throws IOException { /** * Determine if the implementation supports the {@link #isDescendant(String)} method - * + *

* TODO un-restrict it in a weekly after the patch */ @Restricted(NoExternalUse.class) @@ -544,7 +566,7 @@ public boolean supportIsDescendant() { /** * Check if the relative path is really a descendant of this folder, following the symbolic links. * Meant to be used in coordination with {@link #child(String)}. - * + *

* TODO un-restrict it in a weekly after the patch */ @Restricted(NoExternalUse.class) @@ -560,6 +582,7 @@ String joinWithForwardSlashes(Collection relativePath) { /** * Creates a virtual file wrapper for a local file. + * * @param f a disk file (need not exist) * @return a wrapper */ @@ -577,180 +600,196 @@ private static final class FileVF extends VirtualFile { this.root = root; } - @Override public String getName() { - return f.getName(); - } + @Override + public String getName() { + return f.getName(); + } - @Override public URI toURI() { - return f.toURI(); - } + @Override + public URI toURI() { + return f.toURI(); + } - @Override public VirtualFile getParent() { - return new FileVF(f.getParentFile(), root); - } + @Override + public VirtualFile getParent() { + return new FileVF(f.getParentFile(), root); + } - @Override public boolean isDirectory() throws IOException { - if (isIllegalSymlink()) { - return false; - } - return f.isDirectory(); + @Override + public boolean isDirectory() throws IOException { + if (isIllegalSymlink()) { + return false; } + return f.isDirectory(); + } - @Override public boolean isFile() throws IOException { - if (isIllegalSymlink()) { - return false; - } - return f.isFile(); + @Override + public boolean isFile() throws IOException { + if (isIllegalSymlink()) { + return false; } + return f.isFile(); + } - @Override public boolean exists() throws IOException { - if (isIllegalSymlink()) { - return false; - } - return f.exists(); + @Override + public boolean exists() throws IOException { + if (isIllegalSymlink()) { + return false; } + return f.exists(); + } - @Override public String readLink() throws IOException { - if (isIllegalSymlink()) { - return null; // best to just ignore link -> ../whatever - } - return Util.resolveSymlink(f); + @Override + public String readLink() throws IOException { + if (isIllegalSymlink()) { + return null; // best to just ignore link -> ../whatever } + return Util.resolveSymlink(f); + } - @Override public VirtualFile[] list() throws IOException { - if (isIllegalSymlink()) { - return new VirtualFile[0]; - } - File[] kids = f.listFiles(); - if (kids == null) { - return new VirtualFile[0]; - } - VirtualFile[] vfs = new VirtualFile[kids.length]; - for (int i = 0; i < kids.length; i++) { - vfs[i] = new FileVF(kids[i], root); - } - return vfs; + @Override + public VirtualFile[] list() throws IOException { + if (isIllegalSymlink()) { + return new VirtualFile[0]; } - - @NonNull - @Override - public VirtualFile[] list(OpenOption... openOptions) throws IOException { - String rootPath = determineRootPath(); - File[] kids = f.listFiles(); - List contents = new ArrayList<>(kids.length); - for (File child : kids) { - if (!FilePath.isSymlink(child, rootPath, openOptions) && !FilePath.isTmpDir(child, rootPath, openOptions)) { - contents.add(new FileVF(child, root)); - } - } - return contents.toArray(new VirtualFile[0]); + File[] kids = f.listFiles(); + if (kids == null) { + return new VirtualFile[0]; } - - @Override public boolean supportsQuickRecursiveListing() { - return true; + VirtualFile[] vfs = new VirtualFile[kids.length]; + for (int i = 0; i < kids.length; i++) { + vfs[i] = new FileVF(kids[i], root); } + return vfs; + } - @Override public @NonNull List listOnlyDescendants() throws IOException { - if (isIllegalSymlink()) { - return Collections.emptyList(); - } - File[] children = f.listFiles(); - if (children == null) { - return Collections.emptyList(); - } - List legalChildren = new ArrayList<>(children.length); - for (File child : children) { - if (isDescendant(child.getName())) { - FileVF legalChild = new FileVF(child, root); - legalChild.cacheDescendant = true; - legalChildren.add(legalChild); - } + @NonNull + @Override + public VirtualFile[] list(OpenOption... openOptions) throws IOException { + String rootPath = determineRootPath(); + File[] kids = f.listFiles(); + List contents = new ArrayList<>(kids.length); + for (File child : kids) { + if (!FilePath.isSymlink(child, rootPath, openOptions) && !FilePath.isTmpDir(child, rootPath, openOptions)) { + contents.add(new FileVF(child, root)); } - return legalChildren; } + return contents.toArray(new VirtualFile[0]); + } - @Override - public Collection list(String includes, String excludes, boolean useDefaultExcludes) throws IOException { - if (isIllegalSymlink()) { - return Collections.emptySet(); - } - return new Scanner(includes, excludes, useDefaultExcludes).invoke(f, null); - } + @Override + public boolean supportsQuickRecursiveListing() { + return true; + } - @Override - public Collection list(String includes, String excludes, boolean useDefaultExcludes, - OpenOption... openOptions) throws IOException { - String rootPath = determineRootPath(); - return new Scanner(includes, excludes, useDefaultExcludes, rootPath, openOptions).invoke(f, null); + @Override + public @NonNull List listOnlyDescendants() throws IOException { + if (isIllegalSymlink()) { + return Collections.emptyList(); } - - @Override - public int zip(OutputStream outputStream, String includes, String excludes, boolean useDefaultExcludes, - String prefix, OpenOption... openOptions) throws IOException { - String rootPath = determineRootPath(); - DirScanner.Glob globScanner = new DirScanner.Glob(includes, excludes, useDefaultExcludes, openOptions); - ArchiverFactory archiverFactory = prefix == null ? ArchiverFactory.ZIP : ArchiverFactory.createZipWithPrefix(prefix, openOptions); - try (Archiver archiver = archiverFactory.create(outputStream)) { - globScanner.scan(f, FilePath.ignoringTmpDirs(FilePath.ignoringSymlinks(archiver, rootPath, openOptions), rootPath, openOptions)); - return archiver.countEntries(); + File[] children = f.listFiles(); + if (children == null) { + return Collections.emptyList(); + } + List legalChildren = new ArrayList<>(children.length); + for (File child : children) { + if (isDescendant(child.getName())) { + FileVF legalChild = new FileVF(child, root); + legalChild.cacheDescendant = true; + legalChildren.add(legalChild); } } + return legalChildren; + } - @Override - public boolean hasSymlink(OpenOption... openOptions) throws IOException { - String rootPath = determineRootPath(); - return FilePath.isSymlink(f, rootPath, openOptions); + @Override + public Collection list(String includes, String excludes, boolean useDefaultExcludes) throws IOException { + if (isIllegalSymlink()) { + return Collections.emptySet(); } + return new Scanner(includes, excludes, useDefaultExcludes).invoke(f, null); + } - @Override public VirtualFile child(String name) { - return new FileVF(new File(f, name), root); - } + @Override + public Collection list(String includes, String excludes, boolean useDefaultExcludes, + OpenOption... openOptions) throws IOException { + String rootPath = determineRootPath(); + return new Scanner(includes, excludes, useDefaultExcludes, rootPath, openOptions).invoke(f, null); + } - @Override public long length() throws IOException { - if (isIllegalSymlink()) { - return 0; - } - return f.length(); + @Override + public int zip(OutputStream outputStream, String includes, String excludes, boolean useDefaultExcludes, + String prefix, OpenOption... openOptions) throws IOException { + String rootPath = determineRootPath(); + DirScanner.Glob globScanner = new DirScanner.Glob(includes, excludes, useDefaultExcludes, openOptions); + ArchiverFactory archiverFactory = prefix == null ? ArchiverFactory.ZIP : ArchiverFactory.createZipWithPrefix(prefix, openOptions); + try (Archiver archiver = archiverFactory.create(outputStream)) { + globScanner.scan(f, FilePath.ignoringTmpDirs(FilePath.ignoringSymlinks(archiver, rootPath, openOptions), rootPath, openOptions)); + return archiver.countEntries(); } + } - @Override public int mode() throws IOException { - if (isIllegalSymlink()) { - return -1; - } - return IOUtils.mode(f); + @Override + public boolean hasSymlink(OpenOption... openOptions) throws IOException { + String rootPath = determineRootPath(); + return FilePath.isSymlink(f, rootPath, openOptions); + } + + @Override + public VirtualFile child(String name) { + return new FileVF(new File(f, name), root); + } + + @Override + public long length() throws IOException { + if (isIllegalSymlink()) { + return 0; } + return f.length(); + } - @Override public long lastModified() throws IOException { - if (isIllegalSymlink()) { - return 0; - } - return f.lastModified(); + @Override + public int mode() throws IOException { + if (isIllegalSymlink()) { + return -1; } + return IOUtils.mode(f); + } - @Override public boolean canRead() throws IOException { - if (isIllegalSymlink()) { - return false; - } - return f.canRead(); + @Override + public long lastModified() throws IOException { + if (isIllegalSymlink()) { + return 0; } + return f.lastModified(); + } - @Override public InputStream open() throws IOException { - if (isIllegalSymlink()) { - throw new FileNotFoundException(f.getPath()); - } - try { - return Files.newInputStream(f.toPath()); - } catch (InvalidPathException e) { - throw new IOException(e); - } + @Override + public boolean canRead() throws IOException { + if (isIllegalSymlink()) { + return false; } + return f.canRead(); + } - @Override - public InputStream open(OpenOption... openOptions) throws IOException { - String rootPath = determineRootPath(); - InputStream inputStream = FilePath.newInputStreamDenyingSymlinkAsNeeded(f, rootPath, openOptions); - return inputStream; + @Override + public InputStream open() throws IOException { + if (isIllegalSymlink()) { + throw new FileNotFoundException(f.getPath()); + } + try { + return Files.newInputStream(f.toPath()); + } catch (InvalidPathException e) { + throw new IOException(e); } + } + + @Override + public InputStream open(OpenOption... openOptions) throws IOException { + String rootPath = determineRootPath(); + InputStream inputStream = FilePath.newInputStreamDenyingSymlinkAsNeeded(f, rootPath, openOptions); + return inputStream; + } @Override public boolean containsSymLinkChild(OpenOption... openOptions) { @@ -829,8 +868,7 @@ public boolean isDescendant(String potentialChildRelativePath) throws IOExceptio cacheDescendant = true; } return isDescendant; - } - catch (InterruptedException e) { + } catch (InterruptedException e) { return false; } } @@ -856,6 +894,7 @@ private String computeRelativePathToRoot() { /** * Creates a virtual file wrapper for a remotable file. + * * @param f a local or remote file (need not exist) * @return a wrapper */ @@ -873,78 +912,86 @@ private static final class FilePathVF extends VirtualFile { this.root = root; } - @Override public String getName() { - return f.getName(); - } + @Override + public String getName() { + return f.getName(); + } - @Override public URI toURI() { - try { - return f.toURI(); - } catch (Exception x) { - return URI.create(f.getRemote()); - } + @Override + public URI toURI() { + try { + return f.toURI(); + } catch (Exception x) { + return URI.create(f.getRemote()); } + } - @Override public VirtualFile getParent() { - return f.getParent().toVirtualFile(); - } + @Override + public VirtualFile getParent() { + return f.getParent().toVirtualFile(); + } - @Override public boolean isDirectory() throws IOException { - try { - return f.isDirectory(); - } catch (InterruptedException x) { - throw new IOException(x); - } + @Override + public boolean isDirectory() throws IOException { + try { + return f.isDirectory(); + } catch (InterruptedException x) { + throw new IOException(x); } + } - @Override public boolean isFile() throws IOException { - // TODO should probably introduce a method for this purpose - return exists() && !isDirectory(); - } + @Override + public boolean isFile() throws IOException { + // TODO should probably introduce a method for this purpose + return exists() && !isDirectory(); + } - @Override public boolean exists() throws IOException { - try { - return f.exists(); - } catch (InterruptedException x) { - throw new IOException(x); - } + @Override + public boolean exists() throws IOException { + try { + return f.exists(); + } catch (InterruptedException x) { + throw new IOException(x); } + } - @Override public String readLink() throws IOException { - try { - return f.readLink(); - } catch (InterruptedException x) { - throw new IOException(x); - } + @Override + public String readLink() throws IOException { + try { + return f.readLink(); + } catch (InterruptedException x) { + throw new IOException(x); } + } - @Override public VirtualFile[] list() throws IOException { - try { - List kids = f.list(); - return convertChildrenToVirtualFile(kids); - } catch (InterruptedException x) { - throw new IOException(x); - } + @Override + public VirtualFile[] list() throws IOException { + try { + List kids = f.list(); + return convertChildrenToVirtualFile(kids); + } catch (InterruptedException x) { + throw new IOException(x); } + } - private VirtualFile[] convertChildrenToVirtualFile(List kids) { - VirtualFile[] vfs = new VirtualFile[kids.size()]; - for (int i = 0; i < vfs.length; i++) { - vfs[i] = new FilePathVF(kids.get(i), this.root); - } - return vfs; + private VirtualFile[] convertChildrenToVirtualFile(List kids) { + VirtualFile[] vfs = new VirtualFile[kids.size()]; + for (int i = 0; i < vfs.length; i++) { + vfs[i] = new FilePathVF(kids.get(i), this.root); } + return vfs; + } - @NonNull - @Override - public VirtualFile[] list(OpenOption... openOptions) throws IOException { - try { - List kids = f.list(root, openOptions); - return convertChildrenToVirtualFile(kids); - } catch (InterruptedException x) { - throw new IOException(x); - } + @NonNull + @Override + public VirtualFile[] list(OpenOption... openOptions) throws IOException { + try { + List kids = f.list(root, openOptions); + return convertChildrenToVirtualFile(kids); + } catch (InterruptedException x) { + throw new IOException(x); } + } @Override public boolean containsSymLinkChild(OpenOption... openOptions) throws IOException { @@ -956,130 +1003,141 @@ public boolean containsSymLinkChild(OpenOption... openOptions) throws IOExceptio } @Override - public boolean hasSymlink(OpenOption... openOptions) throws IOException { - try { - return f.hasSymlink(root, openOptions); - } catch (InterruptedException x) { - throw new IOException(x); - } + public boolean hasSymlink(OpenOption... openOptions) throws IOException { + try { + return f.hasSymlink(root, openOptions); + } catch (InterruptedException x) { + throw new IOException(x); } + } - @Override public boolean supportsQuickRecursiveListing() { - return this.f.getChannel() == FilePath.localChannel; - } + @Override + public boolean supportsQuickRecursiveListing() { + return this.f.getChannel() == FilePath.localChannel; + } - @Override public @NonNull List listOnlyDescendants() throws IOException { - try { - if (!isDescendant("")) { - return Collections.emptyList(); - } + @Override + public @NonNull List listOnlyDescendants() throws IOException { + try { + if (!isDescendant("")) { + return Collections.emptyList(); + } - List children = f.list(); - List legalChildren = new ArrayList<>(children.size()); - for (FilePath child : children) { - if (isDescendant(child.getName())) { - FilePathVF legalChild = new FilePathVF(child, this.root); - legalChild.cacheDescendant = true; - legalChildren.add(legalChild); - } + List children = f.list(); + List legalChildren = new ArrayList<>(children.size()); + for (FilePath child : children) { + if (isDescendant(child.getName())) { + FilePathVF legalChild = new FilePathVF(child, this.root); + legalChild.cacheDescendant = true; + legalChildren.add(legalChild); } - - return legalChildren; - } catch (InterruptedException x) { - throw new IOException(x); } - } - @Override public Collection list(String includes, String excludes, boolean useDefaultExcludes) throws IOException { - try { - return f.act(new Scanner(includes, excludes, useDefaultExcludes)); - } catch (InterruptedException x) { - throw new IOException(x); - } + return legalChildren; + } catch (InterruptedException x) { + throw new IOException(x); } + } - @Override - public Collection list(String includes, String excludes, boolean useDefaultExcludes, - OpenOption... openOptions) throws IOException { - try { - String rootPath = root == null ? null : root.getRemote(); - return f.act(new Scanner(includes, excludes, useDefaultExcludes, rootPath, openOptions)); - } catch (InterruptedException x) { - throw new IOException(x); - } + @Override + public Collection list(String includes, String excludes, boolean useDefaultExcludes) throws IOException { + try { + return f.act(new Scanner(includes, excludes, useDefaultExcludes)); + } catch (InterruptedException x) { + throw new IOException(x); } + } - @Override - public int zip(OutputStream outputStream, String includes, String excludes, boolean useDefaultExcludes, - String prefix, OpenOption... openOptions) throws IOException { - try { - String rootPath = root == null ? null : root.getRemote(); - DirScanner.Glob globScanner = new DirScanner.Glob(includes, excludes, useDefaultExcludes, openOptions); - return f.zip(outputStream, globScanner, rootPath, prefix, openOptions); - } catch (InterruptedException x) { - throw new IOException(x); - } + @Override + public Collection list(String includes, String excludes, boolean useDefaultExcludes, + OpenOption... openOptions) throws IOException { + try { + String rootPath = root == null ? null : root.getRemote(); + return f.act(new Scanner(includes, excludes, useDefaultExcludes, rootPath, openOptions)); + } catch (InterruptedException x) { + throw new IOException(x); } + } - @Override public VirtualFile child(String name) { - return new FilePathVF(f.child(name), this.root); + @Override + public int zip(OutputStream outputStream, String includes, String excludes, boolean useDefaultExcludes, + String prefix, OpenOption... openOptions) throws IOException { + try { + String rootPath = root == null ? null : root.getRemote(); + DirScanner.Glob globScanner = new DirScanner.Glob(includes, excludes, useDefaultExcludes, openOptions); + return f.zip(outputStream, globScanner, rootPath, prefix, openOptions); + } catch (InterruptedException x) { + throw new IOException(x); } + } - @Override public long length() throws IOException { - try { - return f.length(); - } catch (InterruptedException x) { - throw new IOException(x); - } + @Override + public VirtualFile child(String name) { + return new FilePathVF(f.child(name), this.root); + } + + @Override + public long length() throws IOException { + try { + return f.length(); + } catch (InterruptedException x) { + throw new IOException(x); } + } - @Override public int mode() throws IOException { - try { - return f.mode(); - } catch (InterruptedException x) { - throw new IOException(x); - } + @Override + public int mode() throws IOException { + try { + return f.mode(); + } catch (InterruptedException x) { + throw new IOException(x); } + } - @Override public long lastModified() throws IOException { - try { - return f.lastModified(); - } catch (InterruptedException x) { - throw new IOException(x); - } + @Override + public long lastModified() throws IOException { + try { + return f.lastModified(); + } catch (InterruptedException x) { + throw new IOException(x); } + } - @Override public boolean canRead() throws IOException { - try { - return f.act(new Readable()); - } catch (InterruptedException x) { - throw new IOException(x); - } + @Override + public boolean canRead() throws IOException { + try { + return f.act(new Readable()); + } catch (InterruptedException x) { + throw new IOException(x); } + } - @Override public InputStream open() throws IOException { - try { - return f.read(); - } catch (InterruptedException x) { - throw new IOException(x); - } + @Override + public InputStream open() throws IOException { + try { + return f.read(); + } catch (InterruptedException x) { + throw new IOException(x); } + } - @Override public InputStream open(OpenOption... openOptions) throws IOException { - try { - return f.read(root, openOptions); - } catch (InterruptedException x) { - throw new IOException(x); - } + @Override + public InputStream open(OpenOption... openOptions) throws IOException { + try { + return f.read(root, openOptions); + } catch (InterruptedException x) { + throw new IOException(x); } + } - @Override public V run(Callable callable) throws IOException { - try { - return f.act(callable); - } catch (InterruptedException x) { - throw new IOException(x); - } + @Override + public V run(Callable callable) throws IOException { + try { + return f.act(callable); + } catch (InterruptedException x) { + throw new IOException(x); } + } /** * TODO un-restrict it in a weekly after the patch @@ -1116,8 +1174,7 @@ public boolean isDescendant(String potentialChildRelativePath) throws IOExceptio } // not a return false because you can be a non-descendant of your parent but still // inside the root directory - } - catch (InterruptedException e) { + } catch (InterruptedException e) { return false; } } @@ -1126,8 +1183,7 @@ public boolean isDescendant(String potentialChildRelativePath) throws IOExceptio try { return this.root.isDescendant(relativePath + potentialChildRelativePath); - } - catch (InterruptedException e) { + } catch (InterruptedException e) { return false; } } @@ -1169,7 +1225,8 @@ private static final class Scanner extends MasterToSlaveFileCallable invoke(File f, VirtualChannel channel) throws IOException { + @Override + public List invoke(File f, VirtualChannel channel) throws IOException { if (includes.isEmpty()) { // see Glob class Javadoc, and list(String, String, boolean) note return Collections.emptyList(); } @@ -1188,7 +1245,8 @@ public void visit(File f, String relativePath) { } private static final class Readable extends MasterToSlaveFileCallable { - @Override public Boolean invoke(File f, VirtualChannel channel) throws IOException, InterruptedException { + @Override + public Boolean invoke(File f, VirtualChannel channel) throws IOException, InterruptedException { return f.canRead(); } } diff --git a/core/src/test/java/jenkins/util/VirtualFileTest.java b/core/src/test/java/jenkins/util/VirtualFileTest.java index 3898e9397a46..e3950bc78686 100644 --- a/core/src/test/java/jenkins/util/VirtualFileTest.java +++ b/core/src/test/java/jenkins/util/VirtualFileTest.java @@ -134,16 +134,16 @@ private static String modeString(int mode) throws IOException { System.err.println("testing " + vf.getClass().getName()); assertEquals("[.hg/config.txt, sub/mid.txt, sub/subsub/lowest.txt, top.txt]", new TreeSet<>(vf.list("**/*.txt", null, false)).toString()); assertEquals("[sub/mid.txt, sub/subsub/lowest.txt, top.txt]", new TreeSet<>(vf.list("**/*.txt", null, true)).toString()); - assertEquals("[.hg/config.txt, sub/mid.txt, sub/subsub/lowest.txt, top.txt, very/deep/path/here]", new TreeSet<>(vf.list("**", null, false)).toString()); + assertEquals("[.hg, .hg/config.txt, sub, sub/mid.txt, sub/subsub, sub/subsub/lowest.txt, top.txt, very, very/deep, very/deep/path, very/deep/path/here]", new TreeSet<>(vf.list("**", null, false)).toString()); assertEquals("[]", new TreeSet<>(vf.list("", null, false)).toString()); - assertEquals("[sub/mid.txt, sub/subsub/lowest.txt]", new TreeSet<>(vf.list("sub/", null, false)).toString()); - assertEquals("[sub/mid.txt]", new TreeSet<>(vf.list("sub/", "sub/subsub/", false)).toString()); - assertEquals("[sub/mid.txt]", new TreeSet<>(vf.list("sub/", "sub/subsub/**", false)).toString()); - assertEquals("[sub/mid.txt]", new TreeSet<>(vf.list("sub/", "**/subsub/", false)).toString()); + assertEquals("[sub, sub/mid.txt, sub/subsub, sub/subsub/lowest.txt]", new TreeSet<>(vf.list("sub/", null, false)).toString()); + assertEquals("[sub, sub/mid.txt]", new TreeSet<>(vf.list("sub/", "sub/subsub/", false)).toString()); + assertEquals("[sub, sub/mid.txt]", new TreeSet<>(vf.list("sub/", "sub/subsub/**", false)).toString()); + assertEquals("[sub, sub/mid.txt]", new TreeSet<>(vf.list("sub/", "**/subsub/", false)).toString()); assertEquals("[.hg/config.txt, sub/mid.txt]", new TreeSet<>(vf.list("**/mid*,**/conf*", null, false)).toString()); - assertEquals("[sub/mid.txt, sub/subsub/lowest.txt]", new TreeSet<>(vf.list("sub/", "**/notthere/", false)).toString()); + assertEquals("[sub, sub/mid.txt, sub/subsub, sub/subsub/lowest.txt]", new TreeSet<>(vf.list("sub/", "**/notthere/", false)).toString()); assertEquals("[top.txt]", new TreeSet<>(vf.list("*.txt", null, false)).toString()); - assertEquals("[sub/subsub/lowest.txt, top.txt, very/deep/path/here]", new TreeSet<>(vf.list("**", "**/mid*,**/conf*", false)).toString()); + assertEquals("[.hg, sub, sub/subsub, sub/subsub/lowest.txt, top.txt, very, very/deep, very/deep/path, very/deep/path/here]", new TreeSet<>(vf.list("**", "**/mid*,**/conf*", false)).toString()); } } /** Roughly analogous to {@code org.jenkinsci.plugins.compress_artifacts.ZipStorage}. */ From b78d5d60fb5671c147eb91c5d91ca47d78879728 Mon Sep 17 00:00:00 2001 From: "Simeon L." Date: Sun, 13 Oct 2024 15:53:56 +0200 Subject: [PATCH 09/21] fix: removed duplicated test, fixed Spotless check --- .../main/java/jenkins/util/VirtualFile.java | 2 -- .../java/hudson/util/io/TarArchiverTest.java | 25 ------------------- 2 files changed, 27 deletions(-) diff --git a/core/src/main/java/jenkins/util/VirtualFile.java b/core/src/main/java/jenkins/util/VirtualFile.java index 57374d2fd86f..6d0f74fdbb44 100644 --- a/core/src/main/java/jenkins/util/VirtualFile.java +++ b/core/src/main/java/jenkins/util/VirtualFile.java @@ -38,7 +38,6 @@ import hudson.util.IOUtils; import hudson.util.io.Archiver; import hudson.util.io.ArchiverFactory; - import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; @@ -61,7 +60,6 @@ import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; - import jenkins.MasterToSlaveFileCallable; import jenkins.model.ArtifactManager; import jenkins.security.MasterToSlaveCallable; diff --git a/core/src/test/java/hudson/util/io/TarArchiverTest.java b/core/src/test/java/hudson/util/io/TarArchiverTest.java index 63567ea89d8e..372735184614 100644 --- a/core/src/test/java/hudson/util/io/TarArchiverTest.java +++ b/core/src/test/java/hudson/util/io/TarArchiverTest.java @@ -36,7 +36,6 @@ import hudson.util.StreamTaskListener; import java.io.File; import java.io.IOException; -import java.io.InputStream; import java.io.OutputStream; import java.nio.charset.StandardCharsets; import java.nio.file.Files; @@ -46,7 +45,6 @@ import java.util.Set; import org.apache.tools.tar.TarEntry; import org.apache.tools.tar.TarInputStream; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -129,29 +127,6 @@ private static void run(FilePath dir, String... cmds) throws InterruptedExceptio } } - @Ignore("TODO fails to add empty directories to archive") - @Issue("JENKINS-73837") - @Test - public void emptyDirectory() throws Exception { - Path tar = tmp.newFile("test.tar").toPath(); - Path root = tmp.newFolder().toPath(); - Files.createDirectory(root.resolve("foo")); - Files.createDirectory(root.resolve("bar")); - Files.writeString(root.resolve("bar/file.txt"), "foobar", StandardCharsets.UTF_8); - try (OutputStream out = Files.newOutputStream(tar)) { - new FilePath(root.toFile()).tar(out, "**"); - } - Set names = new HashSet<>(); - try (InputStream is = Files.newInputStream(tar); - TarInputStream tis = new TarInputStream(is, StandardCharsets.UTF_8.name())) { - TarEntry te; - while ((te = tis.getNextEntry()) != null) { - names.add(te.getName()); - } - } - assertEquals(Set.of("foo/", "bar/", "bar/file.txt"), names); - } - /** * Test backing up an open file */ From 0dc6d7db6fd3fe63e2ef3278987bf96fc121135e Mon Sep 17 00:00:00 2001 From: "Simeon L." Date: Mon, 14 Oct 2024 03:16:19 +0200 Subject: [PATCH 10/21] feat: added zip test to also close JENKINS-49296 --- .../java/hudson/util/io/ZipArchiverTest.java | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/core/src/test/java/hudson/util/io/ZipArchiverTest.java b/core/src/test/java/hudson/util/io/ZipArchiverTest.java index a54fad193ecf..7e2caad5f958 100644 --- a/core/src/test/java/hudson/util/io/ZipArchiverTest.java +++ b/core/src/test/java/hudson/util/io/ZipArchiverTest.java @@ -81,26 +81,25 @@ public void huge64bitFile() throws IOException { } } - @Ignore("TODO fails to add empty directories to archive") @Issue("JENKINS-49296") @Test public void emptyDirectory() throws Exception { Path zip = tmp.newFile("test.zip").toPath(); Path root = tmp.newFolder().toPath(); - Files.createDirectory(root.resolve("foo")); - Files.createDirectory(root.resolve("bar")); - Files.writeString(root.resolve("bar/file.txt"), "foobar", StandardCharsets.UTF_8); + Files.createDirectory(root.resolve("a")); + Files.createDirectory(root.resolve("b")); + Files.writeString(root.resolve("a/file.txt"), "empty_dir_zip_test"); try (OutputStream out = Files.newOutputStream(zip)) { new FilePath(root.toFile()).zip(out, "**"); } - Set names = new HashSet<>(); - try (InputStream is = Files.newInputStream(zip); - ZipInputStream zis = new ZipInputStream(is, StandardCharsets.UTF_8)) { - ZipEntry ze; - while ((ze = zis.getNextEntry()) != null) { - names.add(ze.getName()); + Set actual = new HashSet<>(); + Set expected = Set.of("a/", "b/", "a/file.txt"); + try (ZipInputStream zipInputStream = new ZipInputStream(Files.newInputStream(zip), StandardCharsets.UTF_8)) { + ZipEntry zipEntry; + while ((zipEntry = zipInputStream.getNextEntry()) != null) { + actual.add(zipEntry.getName()); } } - assertEquals(Set.of("foo/", "bar/", "bar/file.txt"), names); + assertEquals(expected, actual); } } From 67713a6f3f6b56bd8b06dbecd465a8c58cb2919b Mon Sep 17 00:00:00 2001 From: "Simeon L." Date: Mon, 14 Oct 2024 03:19:18 +0200 Subject: [PATCH 11/21] chore: removed unused imports --- core/src/test/java/hudson/util/io/ZipArchiverTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/core/src/test/java/hudson/util/io/ZipArchiverTest.java b/core/src/test/java/hudson/util/io/ZipArchiverTest.java index 7e2caad5f958..2f941613113f 100644 --- a/core/src/test/java/hudson/util/io/ZipArchiverTest.java +++ b/core/src/test/java/hudson/util/io/ZipArchiverTest.java @@ -4,7 +4,6 @@ import hudson.FilePath; import java.io.IOException; -import java.io.InputStream; import java.io.OutputStream; import java.io.RandomAccessFile; import java.nio.charset.StandardCharsets; @@ -16,7 +15,6 @@ import java.util.zip.ZipFile; import java.util.zip.ZipInputStream; import org.junit.Assume; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; From 53f875e0da0ca6028cf8d73011af2084f8a41c52 Mon Sep 17 00:00:00 2001 From: "Simeon L." Date: Tue, 15 Oct 2024 03:07:43 +0200 Subject: [PATCH 12/21] fix: Reverted VirtualFile formatting --- .../main/java/jenkins/util/VirtualFile.java | 708 ++++++++---------- 1 file changed, 325 insertions(+), 383 deletions(-) diff --git a/core/src/main/java/jenkins/util/VirtualFile.java b/core/src/main/java/jenkins/util/VirtualFile.java index 6d0f74fdbb44..6e50789d330a 100644 --- a/core/src/main/java/jenkins/util/VirtualFile.java +++ b/core/src/main/java/jenkins/util/VirtualFile.java @@ -87,7 +87,7 @@ * {@link File}s somewhere. This makes VirtualFile more abstract. * *

Opening files from other machines

- *

+ * * While {@link VirtualFile} is marked {@link Serializable}, * it is not safe in general to transfer over a Remoting channel. * (For example, an implementation from {@link #forFilePath} could be sent on the same channel, @@ -114,9 +114,8 @@ public abstract class VirtualFile implements Comparable, Serializab /** * Gets the base name, meaning just the last portion of the path name without any * directories. - *

- * For a “root directory” this may be the empty string. * + * For a “root directory” this may be the empty string. * @return a simple name (no slashes) */ public abstract @NonNull String getName(); @@ -126,7 +125,6 @@ public abstract class VirtualFile implements Comparable, Serializab * Should at least uniquely identify this virtual file within its root, but not necessarily globally. *

When {@link #toExternalURL} is implemented, that same value could be used here, * unless some sort of authentication is also embedded. - * * @return a URI (need not be absolute) */ public abstract @NonNull URI toURI(); @@ -134,14 +132,12 @@ public abstract class VirtualFile implements Comparable, Serializab /** * Gets the parent file. * Need only operate within the originally given root. - * * @return the parent */ public abstract VirtualFile getParent(); /** * Checks whether this file exists and is a directory. - * * @return true if it is a directory, false if a file or nonexistent * @throws IOException in case checking status failed */ @@ -149,7 +145,6 @@ public abstract class VirtualFile implements Comparable, Serializab /** * Checks whether this file exists and is a plain file. - * * @return true if it is a file, false if a directory or nonexistent * @throws IOException in case checking status failed */ @@ -159,7 +154,6 @@ public abstract class VirtualFile implements Comparable, Serializab * If this file is a symlink, returns the link target. *

The default implementation always returns null. * Some implementations may not support symlinks under any conditions. - * * @return a target (typically a relative path in some format), or null if this is not a link * @throws IOException if reading the link, or even determining whether this file is a link, failed * @since 2.118 @@ -171,7 +165,6 @@ public abstract class VirtualFile implements Comparable, Serializab /** * Checks whether this file exists. * The behavior is undefined for symlinks; if in doubt, check {@link #readLink} first. - * * @return true if it is a plain file or directory, false if nonexistent * @throws IOException in case checking status failed */ @@ -187,13 +180,12 @@ public abstract class VirtualFile implements Comparable, Serializab /** * Lists children of this directory. Only one level deep. - *

+ * * This is intended to allow the caller to provide {@link java.nio.file.LinkOption#NOFOLLOW_LINKS} to ignore * symlinks. However, this cannot be enforced. The base implementation here in VirtualFile ignores the openOptions. * Some VirtualFile subclasses may not be able to provide * an implementation in which NOFOLLOW_LINKS is used or makes sense. Implementations are free * to ignore openOptions. Some subclasses of VirtualFile may not have a concept of symlinks. - * * @param openOptions the options to apply when opening. * @return a list of children (files and subdirectories); empty for a file or nonexistent directory * @throws IOException if it could not be opened @@ -213,9 +205,8 @@ public boolean supportsQuickRecursiveListing() { * A recognized symlink can be the file itself or any containing directory between * it and the optional root directory. If there is no provided root directory then * only the file itself is considered. - *

- * This base implementation ignores the existence of symlinks. * + * This base implementation ignores the existence of symlinks. * @param openOptions the various open options to apply to the operation. * @return True if the file is a symlink or is referenced within a containing symlink. * directory before reaching the root directory. @@ -257,10 +248,9 @@ public boolean hasSymlink(OpenOption... openOptions) throws IOException { * Lists recursive files of this directory with pattern matching. *

The default implementation calls {@link #list()} recursively inside {@link #run} and applies filtering to the result. * Implementations may wish to override this more efficiently. - * - * @param includes comma-separated Ant-style globs as per {@link Util#createFileSet(File, String, String)} using {@code /} as a path separator; - * the empty string means no matches (use {@link SelectorUtils#DEEP_TREE_MATCH} if you want to match everything except some excludes) - * @param excludes optional excludes in similar format to {@code includes} + * @param includes comma-separated Ant-style globs as per {@link Util#createFileSet(File, String, String)} using {@code /} as a path separator; + * the empty string means no matches (use {@link SelectorUtils#DEEP_TREE_MATCH} if you want to match everything except some excludes) + * @param excludes optional excludes in similar format to {@code includes} * @param useDefaultExcludes as per {@link AbstractFileSet#setDefaultexcludes} * @return a list of {@code /}-separated relative names of children (files directly inside or in subdirectories) * @throws IOException if this is not a directory, or listing was not possible for some other reason @@ -275,16 +265,15 @@ public boolean hasSymlink(OpenOption... openOptions) throws IOException { * *

The default implementation calls {@link #list()} recursively inside {@link #run} and applies filtering to the result. * Implementations may wish to override this more efficiently. - *

+ * This method allows the user to specify that symlinks should not be followed by passing * LinkOption.NOFOLLOW_LINKS as true. However, some implementations may not be able to reliably * prevent link following. The base implementation here in VirtualFile ignores this parameter. - * - * @param includes comma-separated Ant-style globs as per {@link Util#createFileSet(File, String, String)} using {@code /} as a path separator; - * the empty string means no matches (use {@link SelectorUtils#DEEP_TREE_MATCH} if you want to match everything except some excludes) - * @param excludes optional excludes in similar format to {@code includes} + * @param includes comma-separated Ant-style globs as per {@link Util#createFileSet(File, String, String)} using {@code /} as a path separator; + * the empty string means no matches (use {@link SelectorUtils#DEEP_TREE_MATCH} if you want to match everything except some excludes) + * @param excludes optional excludes in similar format to {@code includes} * @param useDefaultExcludes as per {@link AbstractFileSet#setDefaultexcludes} - * @param openOptions the options to apply when opening. + * @param openOptions the options to apply when opening. * @return a list of {@code /}-separated relative names of children (files directly inside or in subdirectories) * @throws IOException if this is not a directory, or listing was not possible for some other reason * @since 2.275 and 2.263.2 @@ -363,13 +352,13 @@ private List patterns(String patts) { * *

The default implementation calls other existing methods to list the folders/files, then retrieve them and zip them all. * - * @param includes comma-separated Ant-style globs as per {@link Util#createFileSet(File, String, String)} using {@code /} as a path separator; - * the empty string means no matches (use {@link SelectorUtils#DEEP_TREE_MATCH} if you want to match everything except some excludes) - * @param excludes optional excludes in similar format to {@code includes} + * @param includes comma-separated Ant-style globs as per {@link Util#createFileSet(File, String, String)} using {@code /} as a path separator; + * the empty string means no matches (use {@link SelectorUtils#DEEP_TREE_MATCH} if you want to match everything except some excludes) + * @param excludes optional excludes in similar format to {@code includes} * @param useDefaultExcludes as per {@link AbstractFileSet#setDefaultexcludes} - * @param prefix the partial path that will be added before each entry inside the archive. - * If non-empty, a trailing slash will be enforced. - * @param openOptions the options to apply when opening. + * @param prefix the partial path that will be added before each entry inside the archive. + * If non-empty, a trailing slash will be enforced. + * @param openOptions the options to apply when opening. * @return the number of files inside the archive (not the folders) * @throws IOException if this is not a directory, or listing was not possible for some other reason * @since 2.275 and 2.263.2 @@ -409,14 +398,14 @@ private void sendOneZipEntry(ZipOutputStream zos, VirtualFile vf, String relativ try (InputStream in = vf.open(openOptions)) { // hudson.util.IOUtils is already present org.apache.commons.io.IOUtils.copy(in, zos); - } finally { + } + finally { zos.closeEntry(); } } /** * Obtains a child file. - * * @param name a relative path, possibly including {@code /} (but not {@code ..}) * @return a representation of that child, whether it actually exists or not */ @@ -424,7 +413,6 @@ private void sendOneZipEntry(ZipOutputStream zos, VirtualFile vf, String relativ /** * Gets the file length. - * * @return a length, or 0 if inapplicable (e.g. a directory) * @throws IOException if checking the length failed */ @@ -432,7 +420,6 @@ private void sendOneZipEntry(ZipOutputStream zos, VirtualFile vf, String relativ /** * Gets the file timestamp. - * * @return a length, or 0 if inapplicable * @throws IOException if checking the timestamp failed */ @@ -441,7 +428,6 @@ private void sendOneZipEntry(ZipOutputStream zos, VirtualFile vf, String relativ /** * Gets the file’s Unix mode, if meaningful. * If the file is symlink (see {@link #readLink}), the mode is that of the link target, not the link itself. - * * @return for example, 0644 ~ {@code rw-r--r--}; -1 by default, meaning unknown or inapplicable * @throws IOException if checking the mode failed * @since 2.118 @@ -452,7 +438,6 @@ public int mode() throws IOException { /** * Checks whether this file can be read. - * * @return true normally * @throws IOException if checking status failed */ @@ -460,7 +445,6 @@ public int mode() throws IOException { /** * Opens an input stream on the file so its contents can be read. - * * @return an open stream * @throws IOException if it could not be opened */ @@ -482,8 +466,7 @@ public InputStream open(OpenOption... openOptions) throws IOException { * Does case-insensitive comparison. * {@inheritDoc} */ - @Override - public final int compareTo(VirtualFile o) { + @Override public final int compareTo(VirtualFile o) { return getName().compareToIgnoreCase(o.getName()); } @@ -491,8 +474,7 @@ public final int compareTo(VirtualFile o) { * Compares according to {@link #toURI}. * {@inheritDoc} */ - @Override - public final boolean equals(Object obj) { + @Override public final boolean equals(Object obj) { return obj instanceof VirtualFile && toURI().equals(((VirtualFile) obj).toURI()); } @@ -500,8 +482,7 @@ public final boolean equals(Object obj) { * Hashes according to {@link #toURI}. * {@inheritDoc} */ - @Override - public final int hashCode() { + @Override public final int hashCode() { return toURI().hashCode(); } @@ -509,8 +490,7 @@ public final int hashCode() { * Displays {@link #toURI}. * {@inheritDoc} */ - @Override - public final String toString() { + @Override public final String toString() { return toURI().toString(); } @@ -518,8 +498,7 @@ public final String toString() { * Does some calculations in batch. * For a remote file, this can be much faster than doing the corresponding operations one by one as separate requests. * The default implementation just calls the block directly. - * - * @param a value type + * @param a value type * @param callable something to run all at once (only helpful if any mentioned files are on the same system) * @return the callable result * @throws IOException if remote communication failed @@ -542,10 +521,9 @@ public V run(Callable callable) throws IOException { * Authentication should be limited to download, not upload or any other modifications. *

The URL might be valid for only a limited amount of time or even only a single use; * this method should be called anew every time an external URL is required. - * * @return an externally usable URL like {@code https://gist.githubusercontent.com/ACCT/GISTID/raw/COMMITHASH/FILE}, or null if there is no such support - * @see #toURI * @since 2.118 + * @see #toURI */ public @CheckForNull URL toExternalURL() throws IOException { return null; @@ -553,7 +531,7 @@ public V run(Callable callable) throws IOException { /** * Determine if the implementation supports the {@link #isDescendant(String)} method - *

+ * * TODO un-restrict it in a weekly after the patch */ @Restricted(NoExternalUse.class) @@ -564,7 +542,7 @@ public boolean supportIsDescendant() { /** * Check if the relative path is really a descendant of this folder, following the symbolic links. * Meant to be used in coordination with {@link #child(String)}. - *

+ * * TODO un-restrict it in a weekly after the patch */ @Restricted(NoExternalUse.class) @@ -580,7 +558,6 @@ String joinWithForwardSlashes(Collection relativePath) { /** * Creates a virtual file wrapper for a local file. - * * @param f a disk file (need not exist) * @return a wrapper */ @@ -598,196 +575,180 @@ private static final class FileVF extends VirtualFile { this.root = root; } - @Override - public String getName() { - return f.getName(); - } - - @Override - public URI toURI() { - return f.toURI(); - } - - @Override - public VirtualFile getParent() { - return new FileVF(f.getParentFile(), root); - } - - @Override - public boolean isDirectory() throws IOException { - if (isIllegalSymlink()) { - return false; + @Override public String getName() { + return f.getName(); } - return f.isDirectory(); - } - @Override - public boolean isFile() throws IOException { - if (isIllegalSymlink()) { - return false; + @Override public URI toURI() { + return f.toURI(); } - return f.isFile(); - } - @Override - public boolean exists() throws IOException { - if (isIllegalSymlink()) { - return false; + @Override public VirtualFile getParent() { + return new FileVF(f.getParentFile(), root); } - return f.exists(); - } - @Override - public String readLink() throws IOException { - if (isIllegalSymlink()) { - return null; // best to just ignore link -> ../whatever + @Override public boolean isDirectory() throws IOException { + if (isIllegalSymlink()) { + return false; + } + return f.isDirectory(); } - return Util.resolveSymlink(f); - } - @Override - public VirtualFile[] list() throws IOException { - if (isIllegalSymlink()) { - return new VirtualFile[0]; - } - File[] kids = f.listFiles(); - if (kids == null) { - return new VirtualFile[0]; + @Override public boolean isFile() throws IOException { + if (isIllegalSymlink()) { + return false; + } + return f.isFile(); } - VirtualFile[] vfs = new VirtualFile[kids.length]; - for (int i = 0; i < kids.length; i++) { - vfs[i] = new FileVF(kids[i], root); + + @Override public boolean exists() throws IOException { + if (isIllegalSymlink()) { + return false; + } + return f.exists(); } - return vfs; - } - @NonNull - @Override - public VirtualFile[] list(OpenOption... openOptions) throws IOException { - String rootPath = determineRootPath(); - File[] kids = f.listFiles(); - List contents = new ArrayList<>(kids.length); - for (File child : kids) { - if (!FilePath.isSymlink(child, rootPath, openOptions) && !FilePath.isTmpDir(child, rootPath, openOptions)) { - contents.add(new FileVF(child, root)); + @Override public String readLink() throws IOException { + if (isIllegalSymlink()) { + return null; // best to just ignore link -> ../whatever } + return Util.resolveSymlink(f); } - return contents.toArray(new VirtualFile[0]); - } - @Override - public boolean supportsQuickRecursiveListing() { - return true; - } + @Override public VirtualFile[] list() throws IOException { + if (isIllegalSymlink()) { + return new VirtualFile[0]; + } + File[] kids = f.listFiles(); + if (kids == null) { + return new VirtualFile[0]; + } + VirtualFile[] vfs = new VirtualFile[kids.length]; + for (int i = 0; i < kids.length; i++) { + vfs[i] = new FileVF(kids[i], root); + } + return vfs; + } - @Override - public @NonNull List listOnlyDescendants() throws IOException { - if (isIllegalSymlink()) { - return Collections.emptyList(); + @NonNull + @Override + public VirtualFile[] list(OpenOption... openOptions) throws IOException { + String rootPath = determineRootPath(); + File[] kids = f.listFiles(); + List contents = new ArrayList<>(kids.length); + for (File child : kids) { + if (!FilePath.isSymlink(child, rootPath, openOptions) && !FilePath.isTmpDir(child, rootPath, openOptions)) { + contents.add(new FileVF(child, root)); + } + } + return contents.toArray(new VirtualFile[0]); } - File[] children = f.listFiles(); - if (children == null) { - return Collections.emptyList(); + + @Override public boolean supportsQuickRecursiveListing() { + return true; } - List legalChildren = new ArrayList<>(children.length); - for (File child : children) { - if (isDescendant(child.getName())) { - FileVF legalChild = new FileVF(child, root); - legalChild.cacheDescendant = true; - legalChildren.add(legalChild); + + @Override public @NonNull List listOnlyDescendants() throws IOException { + if (isIllegalSymlink()) { + return Collections.emptyList(); + } + File[] children = f.listFiles(); + if (children == null) { + return Collections.emptyList(); } + List legalChildren = new ArrayList<>(children.length); + for (File child : children) { + if (isDescendant(child.getName())) { + FileVF legalChild = new FileVF(child, root); + legalChild.cacheDescendant = true; + legalChildren.add(legalChild); + } + } + return legalChildren; } - return legalChildren; - } - @Override - public Collection list(String includes, String excludes, boolean useDefaultExcludes) throws IOException { - if (isIllegalSymlink()) { - return Collections.emptySet(); + @Override + public Collection list(String includes, String excludes, boolean useDefaultExcludes) throws IOException { + if (isIllegalSymlink()) { + return Collections.emptySet(); + } + return new Scanner(includes, excludes, useDefaultExcludes).invoke(f, null); } - return new Scanner(includes, excludes, useDefaultExcludes).invoke(f, null); - } - - @Override - public Collection list(String includes, String excludes, boolean useDefaultExcludes, - OpenOption... openOptions) throws IOException { - String rootPath = determineRootPath(); - return new Scanner(includes, excludes, useDefaultExcludes, rootPath, openOptions).invoke(f, null); - } - @Override - public int zip(OutputStream outputStream, String includes, String excludes, boolean useDefaultExcludes, - String prefix, OpenOption... openOptions) throws IOException { - String rootPath = determineRootPath(); - DirScanner.Glob globScanner = new DirScanner.Glob(includes, excludes, useDefaultExcludes, openOptions); - ArchiverFactory archiverFactory = prefix == null ? ArchiverFactory.ZIP : ArchiverFactory.createZipWithPrefix(prefix, openOptions); - try (Archiver archiver = archiverFactory.create(outputStream)) { - globScanner.scan(f, FilePath.ignoringTmpDirs(FilePath.ignoringSymlinks(archiver, rootPath, openOptions), rootPath, openOptions)); - return archiver.countEntries(); + @Override + public Collection list(String includes, String excludes, boolean useDefaultExcludes, + OpenOption... openOptions) throws IOException { + String rootPath = determineRootPath(); + return new Scanner(includes, excludes, useDefaultExcludes, rootPath, openOptions).invoke(f, null); } - } - @Override - public boolean hasSymlink(OpenOption... openOptions) throws IOException { - String rootPath = determineRootPath(); - return FilePath.isSymlink(f, rootPath, openOptions); - } + @Override + public int zip(OutputStream outputStream, String includes, String excludes, boolean useDefaultExcludes, + String prefix, OpenOption... openOptions) throws IOException { + String rootPath = determineRootPath(); + DirScanner.Glob globScanner = new DirScanner.Glob(includes, excludes, useDefaultExcludes, openOptions); + ArchiverFactory archiverFactory = prefix == null ? ArchiverFactory.ZIP : ArchiverFactory.createZipWithPrefix(prefix, openOptions); + try (Archiver archiver = archiverFactory.create(outputStream)) { + globScanner.scan(f, FilePath.ignoringTmpDirs(FilePath.ignoringSymlinks(archiver, rootPath, openOptions), rootPath, openOptions)); + return archiver.countEntries(); + } + } - @Override - public VirtualFile child(String name) { - return new FileVF(new File(f, name), root); - } + @Override + public boolean hasSymlink(OpenOption... openOptions) throws IOException { + String rootPath = determineRootPath(); + return FilePath.isSymlink(f, rootPath, openOptions); + } - @Override - public long length() throws IOException { - if (isIllegalSymlink()) { - return 0; + @Override public VirtualFile child(String name) { + return new FileVF(new File(f, name), root); } - return f.length(); - } - @Override - public int mode() throws IOException { - if (isIllegalSymlink()) { - return -1; + @Override public long length() throws IOException { + if (isIllegalSymlink()) { + return 0; + } + return f.length(); } - return IOUtils.mode(f); - } - @Override - public long lastModified() throws IOException { - if (isIllegalSymlink()) { - return 0; + @Override public int mode() throws IOException { + if (isIllegalSymlink()) { + return -1; + } + return IOUtils.mode(f); } - return f.lastModified(); - } - @Override - public boolean canRead() throws IOException { - if (isIllegalSymlink()) { - return false; + @Override public long lastModified() throws IOException { + if (isIllegalSymlink()) { + return 0; + } + return f.lastModified(); } - return f.canRead(); - } - @Override - public InputStream open() throws IOException { - if (isIllegalSymlink()) { - throw new FileNotFoundException(f.getPath()); + @Override public boolean canRead() throws IOException { + if (isIllegalSymlink()) { + return false; + } + return f.canRead(); } - try { - return Files.newInputStream(f.toPath()); - } catch (InvalidPathException e) { - throw new IOException(e); + + @Override public InputStream open() throws IOException { + if (isIllegalSymlink()) { + throw new FileNotFoundException(f.getPath()); + } + try { + return Files.newInputStream(f.toPath()); + } catch (InvalidPathException e) { + throw new IOException(e); + } } - } - @Override - public InputStream open(OpenOption... openOptions) throws IOException { - String rootPath = determineRootPath(); - InputStream inputStream = FilePath.newInputStreamDenyingSymlinkAsNeeded(f, rootPath, openOptions); - return inputStream; - } + @Override + public InputStream open(OpenOption... openOptions) throws IOException { + String rootPath = determineRootPath(); + InputStream inputStream = FilePath.newInputStreamDenyingSymlinkAsNeeded(f, rootPath, openOptions); + return inputStream; + } @Override public boolean containsSymLinkChild(OpenOption... openOptions) { @@ -866,7 +827,8 @@ public boolean isDescendant(String potentialChildRelativePath) throws IOExceptio cacheDescendant = true; } return isDescendant; - } catch (InterruptedException e) { + } + catch (InterruptedException e) { return false; } } @@ -892,7 +854,6 @@ private String computeRelativePathToRoot() { /** * Creates a virtual file wrapper for a remotable file. - * * @param f a local or remote file (need not exist) * @return a wrapper */ @@ -910,86 +871,78 @@ private static final class FilePathVF extends VirtualFile { this.root = root; } - @Override - public String getName() { - return f.getName(); - } + @Override public String getName() { + return f.getName(); + } - @Override - public URI toURI() { - try { - return f.toURI(); - } catch (Exception x) { - return URI.create(f.getRemote()); + @Override public URI toURI() { + try { + return f.toURI(); + } catch (Exception x) { + return URI.create(f.getRemote()); + } } - } - @Override - public VirtualFile getParent() { - return f.getParent().toVirtualFile(); - } + @Override public VirtualFile getParent() { + return f.getParent().toVirtualFile(); + } - @Override - public boolean isDirectory() throws IOException { - try { - return f.isDirectory(); - } catch (InterruptedException x) { - throw new IOException(x); + @Override public boolean isDirectory() throws IOException { + try { + return f.isDirectory(); + } catch (InterruptedException x) { + throw new IOException(x); + } } - } - @Override - public boolean isFile() throws IOException { - // TODO should probably introduce a method for this purpose - return exists() && !isDirectory(); - } + @Override public boolean isFile() throws IOException { + // TODO should probably introduce a method for this purpose + return exists() && !isDirectory(); + } - @Override - public boolean exists() throws IOException { - try { - return f.exists(); - } catch (InterruptedException x) { - throw new IOException(x); + @Override public boolean exists() throws IOException { + try { + return f.exists(); + } catch (InterruptedException x) { + throw new IOException(x); + } } - } - @Override - public String readLink() throws IOException { - try { - return f.readLink(); - } catch (InterruptedException x) { - throw new IOException(x); + @Override public String readLink() throws IOException { + try { + return f.readLink(); + } catch (InterruptedException x) { + throw new IOException(x); + } } - } - @Override - public VirtualFile[] list() throws IOException { - try { - List kids = f.list(); - return convertChildrenToVirtualFile(kids); - } catch (InterruptedException x) { - throw new IOException(x); + @Override public VirtualFile[] list() throws IOException { + try { + List kids = f.list(); + return convertChildrenToVirtualFile(kids); + } catch (InterruptedException x) { + throw new IOException(x); + } } - } - private VirtualFile[] convertChildrenToVirtualFile(List kids) { - VirtualFile[] vfs = new VirtualFile[kids.size()]; - for (int i = 0; i < vfs.length; i++) { - vfs[i] = new FilePathVF(kids.get(i), this.root); + private VirtualFile[] convertChildrenToVirtualFile(List kids) { + VirtualFile[] vfs = new VirtualFile[kids.size()]; + for (int i = 0; i < vfs.length; i++) { + vfs[i] = new FilePathVF(kids.get(i), this.root); + } + return vfs; } - return vfs; - } - @NonNull - @Override - public VirtualFile[] list(OpenOption... openOptions) throws IOException { - try { - List kids = f.list(root, openOptions); - return convertChildrenToVirtualFile(kids); - } catch (InterruptedException x) { - throw new IOException(x); + @NonNull + @Override + public VirtualFile[] list(OpenOption... openOptions) throws IOException { + try { + List kids = f.list(root, openOptions); + return convertChildrenToVirtualFile(kids); + } catch (InterruptedException x) { + throw new IOException(x); + } } - } @Override public boolean containsSymLinkChild(OpenOption... openOptions) throws IOException { @@ -1001,141 +954,130 @@ public boolean containsSymLinkChild(OpenOption... openOptions) throws IOExceptio } @Override - public boolean hasSymlink(OpenOption... openOptions) throws IOException { - try { - return f.hasSymlink(root, openOptions); - } catch (InterruptedException x) { - throw new IOException(x); + public boolean hasSymlink(OpenOption... openOptions) throws IOException { + try { + return f.hasSymlink(root, openOptions); + } catch (InterruptedException x) { + throw new IOException(x); + } } - } - @Override - public boolean supportsQuickRecursiveListing() { - return this.f.getChannel() == FilePath.localChannel; - } + @Override public boolean supportsQuickRecursiveListing() { + return this.f.getChannel() == FilePath.localChannel; + } - @Override - public @NonNull List listOnlyDescendants() throws IOException { - try { - if (!isDescendant("")) { - return Collections.emptyList(); - } + @Override public @NonNull List listOnlyDescendants() throws IOException { + try { + if (!isDescendant("")) { + return Collections.emptyList(); + } - List children = f.list(); - List legalChildren = new ArrayList<>(children.size()); - for (FilePath child : children) { - if (isDescendant(child.getName())) { - FilePathVF legalChild = new FilePathVF(child, this.root); - legalChild.cacheDescendant = true; - legalChildren.add(legalChild); + List children = f.list(); + List legalChildren = new ArrayList<>(children.size()); + for (FilePath child : children) { + if (isDescendant(child.getName())) { + FilePathVF legalChild = new FilePathVF(child, this.root); + legalChild.cacheDescendant = true; + legalChildren.add(legalChild); + } } - } - return legalChildren; - } catch (InterruptedException x) { - throw new IOException(x); + return legalChildren; + } catch (InterruptedException x) { + throw new IOException(x); + } } - } - @Override - public Collection list(String includes, String excludes, boolean useDefaultExcludes) throws IOException { - try { - return f.act(new Scanner(includes, excludes, useDefaultExcludes)); - } catch (InterruptedException x) { - throw new IOException(x); + @Override public Collection list(String includes, String excludes, boolean useDefaultExcludes) throws IOException { + try { + return f.act(new Scanner(includes, excludes, useDefaultExcludes)); + } catch (InterruptedException x) { + throw new IOException(x); + } } - } - @Override - public Collection list(String includes, String excludes, boolean useDefaultExcludes, - OpenOption... openOptions) throws IOException { - try { - String rootPath = root == null ? null : root.getRemote(); - return f.act(new Scanner(includes, excludes, useDefaultExcludes, rootPath, openOptions)); - } catch (InterruptedException x) { - throw new IOException(x); + @Override + public Collection list(String includes, String excludes, boolean useDefaultExcludes, + OpenOption... openOptions) throws IOException { + try { + String rootPath = root == null ? null : root.getRemote(); + return f.act(new Scanner(includes, excludes, useDefaultExcludes, rootPath, openOptions)); + } catch (InterruptedException x) { + throw new IOException(x); + } } - } - @Override - public int zip(OutputStream outputStream, String includes, String excludes, boolean useDefaultExcludes, - String prefix, OpenOption... openOptions) throws IOException { - try { - String rootPath = root == null ? null : root.getRemote(); - DirScanner.Glob globScanner = new DirScanner.Glob(includes, excludes, useDefaultExcludes, openOptions); - return f.zip(outputStream, globScanner, rootPath, prefix, openOptions); - } catch (InterruptedException x) { - throw new IOException(x); + @Override + public int zip(OutputStream outputStream, String includes, String excludes, boolean useDefaultExcludes, + String prefix, OpenOption... openOptions) throws IOException { + try { + String rootPath = root == null ? null : root.getRemote(); + DirScanner.Glob globScanner = new DirScanner.Glob(includes, excludes, useDefaultExcludes, openOptions); + return f.zip(outputStream, globScanner, rootPath, prefix, openOptions); + } catch (InterruptedException x) { + throw new IOException(x); + } } - } - @Override - public VirtualFile child(String name) { - return new FilePathVF(f.child(name), this.root); - } + @Override public VirtualFile child(String name) { + return new FilePathVF(f.child(name), this.root); + } - @Override - public long length() throws IOException { - try { - return f.length(); - } catch (InterruptedException x) { - throw new IOException(x); + @Override public long length() throws IOException { + try { + return f.length(); + } catch (InterruptedException x) { + throw new IOException(x); + } } - } - @Override - public int mode() throws IOException { - try { - return f.mode(); - } catch (InterruptedException x) { - throw new IOException(x); + @Override public int mode() throws IOException { + try { + return f.mode(); + } catch (InterruptedException x) { + throw new IOException(x); + } } - } - @Override - public long lastModified() throws IOException { - try { - return f.lastModified(); - } catch (InterruptedException x) { - throw new IOException(x); + @Override public long lastModified() throws IOException { + try { + return f.lastModified(); + } catch (InterruptedException x) { + throw new IOException(x); + } } - } - @Override - public boolean canRead() throws IOException { - try { - return f.act(new Readable()); - } catch (InterruptedException x) { - throw new IOException(x); + @Override public boolean canRead() throws IOException { + try { + return f.act(new Readable()); + } catch (InterruptedException x) { + throw new IOException(x); + } } - } - @Override - public InputStream open() throws IOException { - try { - return f.read(); - } catch (InterruptedException x) { - throw new IOException(x); + @Override public InputStream open() throws IOException { + try { + return f.read(); + } catch (InterruptedException x) { + throw new IOException(x); + } } - } - @Override - public InputStream open(OpenOption... openOptions) throws IOException { - try { - return f.read(root, openOptions); - } catch (InterruptedException x) { - throw new IOException(x); + @Override public InputStream open(OpenOption... openOptions) throws IOException { + try { + return f.read(root, openOptions); + } catch (InterruptedException x) { + throw new IOException(x); + } } - } - @Override - public V run(Callable callable) throws IOException { - try { - return f.act(callable); - } catch (InterruptedException x) { - throw new IOException(x); + @Override public V run(Callable callable) throws IOException { + try { + return f.act(callable); + } catch (InterruptedException x) { + throw new IOException(x); + } } - } /** * TODO un-restrict it in a weekly after the patch @@ -1172,7 +1114,8 @@ public boolean isDescendant(String potentialChildRelativePath) throws IOExceptio } // not a return false because you can be a non-descendant of your parent but still // inside the root directory - } catch (InterruptedException e) { + } + catch (InterruptedException e) { return false; } } @@ -1181,7 +1124,8 @@ public boolean isDescendant(String potentialChildRelativePath) throws IOExceptio try { return this.root.isDescendant(relativePath + potentialChildRelativePath); - } catch (InterruptedException e) { + } + catch (InterruptedException e) { return false; } } @@ -1223,8 +1167,7 @@ private static final class Scanner extends MasterToSlaveFileCallable invoke(File f, VirtualChannel channel) throws IOException { + @Override public List invoke(File f, VirtualChannel channel) throws IOException { if (includes.isEmpty()) { // see Glob class Javadoc, and list(String, String, boolean) note return Collections.emptyList(); } @@ -1243,8 +1186,7 @@ public void visit(File f, String relativePath) { } private static final class Readable extends MasterToSlaveFileCallable { - @Override - public Boolean invoke(File f, VirtualChannel channel) throws IOException, InterruptedException { + @Override public Boolean invoke(File f, VirtualChannel channel) throws IOException, InterruptedException { return f.canRead(); } } From 63ccc182ef31ceecc989170a6314b75c2e6467f8 Mon Sep 17 00:00:00 2001 From: Simeon L Date: Tue, 15 Oct 2024 10:17:25 +0200 Subject: [PATCH 13/21] chore: dropped unneeded comments --- core/src/main/java/jenkins/util/VirtualFile.java | 1 - core/src/test/java/jenkins/util/VirtualFileTest.java | 7 ------- .../hudson/model/DirectoryBrowserSupportTest.java | 11 ----------- 3 files changed, 19 deletions(-) diff --git a/core/src/main/java/jenkins/util/VirtualFile.java b/core/src/main/java/jenkins/util/VirtualFile.java index 6e50789d330a..660c089eea32 100644 --- a/core/src/main/java/jenkins/util/VirtualFile.java +++ b/core/src/main/java/jenkins/util/VirtualFile.java @@ -327,7 +327,6 @@ public Collection call() throws IOException { private static void collectFiles(VirtualFile d, Collection names, String prefix) throws IOException { for (VirtualFile child : d.list()) { - // JENKINS-73837 - Empty Directories are now included in the list names.add(prefix + child.getName()); collectFiles(child, names, prefix + child.getName() + "/"); } diff --git a/core/src/test/java/jenkins/util/VirtualFileTest.java b/core/src/test/java/jenkins/util/VirtualFileTest.java index e3950bc78686..8fac5f9adacf 100644 --- a/core/src/test/java/jenkins/util/VirtualFileTest.java +++ b/core/src/test/java/jenkins/util/VirtualFileTest.java @@ -255,7 +255,6 @@ public void list_Glob_NoFollowLinks_FilePathVF() throws Exception { File root = tmp.getRoot(); VirtualFile virtualRoot = VirtualFile.forFilePath(new FilePath(root)); Collection children = virtualRoot.list("**", null, true, LinkOption.NOFOLLOW_LINKS); - // JENKINS-73837 - Since we now allow empty directories, this had to be changed to hasItems assertThat(children, hasItems( "a/aa/aa.txt", "a/ab/ab.txt", @@ -284,7 +283,6 @@ public void zip_NoFollowLinks_FilePathVF() throws Exception { assertTrue(unzipPath.isDirectory()); assertTrue(unzipPath.child("a").child("aa").child("aa.txt").exists()); assertTrue(unzipPath.child("a").child("ab").child("ab.txt").exists()); - // JENKINS-73837 - Empty directories are included in the zip file assertTrue(unzipPath.child("a").child("aa").child("aaa").exists()); assertFalse(unzipPath.child("a").child("_b").exists()); assertTrue(unzipPath.child("b").child("ba").child("ba.txt").exists()); @@ -315,7 +313,6 @@ public void zip_NoFollowLinks_FilePathVF_withPrefix() throws Exception { assertTrue(unzipPath.child(prefix).isDirectory()); assertTrue(unzipPath.child(prefix).child("a").child("aa").child("aa.txt").exists()); assertTrue(unzipPath.child(prefix).child("a").child("ab").child("ab.txt").exists()); - // JENKINS-73837 - Empty directories are included in the zip file assertTrue(unzipPath.child(prefix).child("a").child("aa").child("aaa").exists()); assertFalse(unzipPath.child(prefix).child("a").child("_b").exists()); assertTrue(unzipPath.child(prefix).child("b").child("ba").child("ba.txt").exists()); @@ -344,7 +341,6 @@ public void zip_NoFollowLinks_FileVF() throws Exception { assertTrue(unzipPath.isDirectory()); assertTrue(unzipPath.child("a").child("aa").child("aa.txt").exists()); assertTrue(unzipPath.child("a").child("ab").child("ab.txt").exists()); - // JENKINS-73837 - Empty directories are included in the zip file assertTrue(unzipPath.child("a").child("aa").child("aaa").exists()); assertFalse(unzipPath.child("a").child("_b").exists()); assertTrue(unzipPath.child("b").child("ba").child("ba.txt").exists()); @@ -375,7 +371,6 @@ public void zip_NoFollowLinks_FileVF_withPrefix() throws Exception { assertTrue(unzipPath.child(prefix).isDirectory()); assertTrue(unzipPath.child(prefix).child("a").child("aa").child("aa.txt").exists()); assertTrue(unzipPath.child(prefix).child("a").child("ab").child("ab.txt").exists()); - // JENKINS-73837 - Empty directories are included in the zip file assertTrue(unzipPath.child(prefix).child("a").child("aa").child("aaa").exists()); assertFalse(unzipPath.child(prefix).child("a").child("_b").exists()); assertTrue(unzipPath.child(prefix).child("b").child("ba").child("ba.txt").exists()); @@ -511,7 +506,6 @@ public void list_Glob_NoFollowLinks_ExternalSymlink_FilePathVF() throws Exceptio VirtualFile symlinkVirtualPath = VirtualFile.forFilePath(symlinkPath); VirtualFile symlinkChildVirtualPath = symlinkVirtualPath.child("aa"); Collection children = symlinkChildVirtualPath.list("**", null, true, LinkOption.NOFOLLOW_LINKS); - // JENKINS-73837 - Since we now allow empty directories, this had to be changed to hasItems assertThat(children, hasItems("aa.txt")); } @@ -527,7 +521,6 @@ public void list_Glob_NoFollowLinks_ExternalSymlink_FileVF() throws Exception { VirtualFile symlinkVirtualFile = VirtualFile.forFile(symlinkFile); VirtualFile symlinkChildVirtualFile = symlinkVirtualFile.child("aa"); Collection children = symlinkChildVirtualFile.list("**", null, true, LinkOption.NOFOLLOW_LINKS); - // JENKINS-73837 - Since we now allow empty directories, this had to be changed to hasItems assertThat(children, hasItems("aa.txt")); } diff --git a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java index 63b11d98c4ef..2cda73ccc6ba 100644 --- a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java +++ b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java @@ -685,7 +685,6 @@ public void symlink_outsideWorkspace_areNotAllowed() throws Exception { assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - // JENKINS-73837 - Since we now allow empty directories, this had to be changed to hasItems assertThat(entryNames, hasItems( p.getName() + "/intermediateFolder/public2.key", p.getName() + "/public1.key" @@ -696,7 +695,6 @@ public void symlink_outsideWorkspace_areNotAllowed() throws Exception { assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - // JENKINS-73837 - Since we now allow empty directories, this had to be changed to hasItems assertThat(entryNames, hasItems( "intermediateFolder/public2.key", "public1.key" @@ -707,7 +705,6 @@ public void symlink_outsideWorkspace_areNotAllowed() throws Exception { assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - // JENKINS-73837 - Since we now allow empty directories, this had to be changed to hasItems assertThat(entryNames, hasItems("intermediateFolder/public2.key")); } { // workaround for JENKINS-19947 is still supported, i.e. no parent folder, even inside a sub-folder @@ -715,7 +712,6 @@ public void symlink_outsideWorkspace_areNotAllowed() throws Exception { assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - // JENKINS-73837 - Since we now allow empty directories, this had to be changed to hasItems assertThat(entryNames, hasItems("public2.key")); } } @@ -919,7 +915,6 @@ public void junctionAndSymlink_outsideWorkspace_areNotAllowed_windowsJunction() assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - // JENKINS-73837 - Using hasItems instead of containsInAnyOrder to allow unmatched directories to be ignored assertThat(entryNames, hasItems( p.getName() + "/intermediateFolder/public2.key", p.getName() + "/public1.key" @@ -930,7 +925,6 @@ public void junctionAndSymlink_outsideWorkspace_areNotAllowed_windowsJunction() assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - // JENKINS-73837 - Using hasItems instead of containsInAnyOrder to allow unmatched directories to be ignored assertThat(entryNames, hasItems("intermediateFolder/public2.key")); } // Explicitly delete everything including junctions, which TemporaryDirectoryAllocator.dispose may have trouble with: @@ -993,7 +987,6 @@ public void directSymlink_forTestingZip() throws Exception { assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - // JENKINS-73837 - Allows empty folders. assertThat(entryNames, hasSize(6)); } { @@ -1277,9 +1270,7 @@ public boolean perform(AbstractBuild build, Launcher launcher, BuildListen assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - // JENKINS-73837 - Allows empty folders. assertThat(entryNames, hasSize(4)); - // JENKINS-73837 - Using hasItems instead of containsInAnyOrder to allow unmatched directories to be ignored assertThat(entryNames, hasItems( "test0/anotherDir/one.txt", "test0/anotherDir/insideDir/two.txt" @@ -1289,9 +1280,7 @@ public boolean perform(AbstractBuild build, Launcher launcher, BuildListen assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - // JENKINS-73837 - Allows empty folders. assertThat(entryNames, hasSize(3)); - // JENKINS-73837 - Using hasItems instead of containsInAnyOrder to allow unmatched directories to be ignored assertThat(entryNames, hasItems( "anotherDir/one.txt", "anotherDir/insideDir/two.txt" From 384cafeb7a027097f685b3a2fc25f0065298d4cd Mon Sep 17 00:00:00 2001 From: Simeon L Date: Tue, 15 Oct 2024 10:23:13 +0200 Subject: [PATCH 14/21] fix: Adjusted ZipArchiverTest#emptyDirectory to properly close inputstream in try-with-ressource --- core/src/test/java/hudson/util/io/ZipArchiverTest.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/core/src/test/java/hudson/util/io/ZipArchiverTest.java b/core/src/test/java/hudson/util/io/ZipArchiverTest.java index 2f941613113f..749b7eb3460e 100644 --- a/core/src/test/java/hudson/util/io/ZipArchiverTest.java +++ b/core/src/test/java/hudson/util/io/ZipArchiverTest.java @@ -3,7 +3,9 @@ import static org.junit.Assert.assertEquals; import hudson.FilePath; + import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; import java.io.RandomAccessFile; import java.nio.charset.StandardCharsets; @@ -14,6 +16,7 @@ import java.util.zip.ZipEntry; import java.util.zip.ZipFile; import java.util.zip.ZipInputStream; + import org.junit.Assume; import org.junit.Rule; import org.junit.Test; @@ -22,7 +25,8 @@ public class ZipArchiverTest { - @Rule public TemporaryFolder tmp = new TemporaryFolder(); + @Rule + public TemporaryFolder tmp = new TemporaryFolder(); @Issue("JENKINS-9942") @Test @@ -92,7 +96,8 @@ public void emptyDirectory() throws Exception { } Set actual = new HashSet<>(); Set expected = Set.of("a/", "b/", "a/file.txt"); - try (ZipInputStream zipInputStream = new ZipInputStream(Files.newInputStream(zip), StandardCharsets.UTF_8)) { + try (InputStream inputStream = Files.newInputStream(zip); + ZipInputStream zipInputStream = new ZipInputStream(inputStream, StandardCharsets.UTF_8)) { ZipEntry zipEntry; while ((zipEntry = zipInputStream.getNextEntry()) != null) { actual.add(zipEntry.getName()); From 5340b071277e1a81a77803454440fcf1d5000f72 Mon Sep 17 00:00:00 2001 From: Simeon L Date: Tue, 15 Oct 2024 10:25:42 +0200 Subject: [PATCH 15/21] fix: moved emptyDirectory test to reduce diff size --- .../java/hudson/util/io/TarArchiverTest.java | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/core/src/test/java/hudson/util/io/TarArchiverTest.java b/core/src/test/java/hudson/util/io/TarArchiverTest.java index 372735184614..a7c635530592 100644 --- a/core/src/test/java/hudson/util/io/TarArchiverTest.java +++ b/core/src/test/java/hudson/util/io/TarArchiverTest.java @@ -127,25 +127,6 @@ private static void run(FilePath dir, String... cmds) throws InterruptedExceptio } } - /** - * Test backing up an open file - */ - - @Issue("JENKINS-20187") - @Test public void growingFileTar() throws Exception { - File file = new File(tmp.getRoot(), "growing.file"); - GrowingFileRunnable runnable1 = new GrowingFileRunnable(file); - Thread t1 = new Thread(runnable1); - t1.start(); - - try (OutputStream out = OutputStream.nullOutputStream()) { - new FilePath(tmp.getRoot()).tar(out, "**"); - } - - runnable1.doFinish(); - t1.join(); - } - @Issue("JENKINS-73837") @Test public void emptyDirectory() throws Exception { @@ -170,6 +151,25 @@ public void emptyDirectory() throws Exception { assertEquals(Set.of("a/", "b/", "a/file.txt"), names); } + /** + * Test backing up an open file + */ + + @Issue("JENKINS-20187") + @Test public void growingFileTar() throws Exception { + File file = new File(tmp.getRoot(), "growing.file"); + GrowingFileRunnable runnable1 = new GrowingFileRunnable(file); + Thread t1 = new Thread(runnable1); + t1.start(); + + try (OutputStream out = OutputStream.nullOutputStream()) { + new FilePath(tmp.getRoot()).tar(out, "**"); + } + + runnable1.doFinish(); + t1.join(); + } + private static class GrowingFileRunnable implements Runnable { private boolean finish = false; private Exception ex = null; From f286a8595118a032f136406959dd05f6ffe50db7 Mon Sep 17 00:00:00 2001 From: Simeon L Date: Tue, 15 Oct 2024 10:31:21 +0200 Subject: [PATCH 16/21] fix: Adjusted TarArchiverTest#emptyDirectory to properly close inputstream in try-with-ressource --- core/src/test/java/hudson/util/io/TarArchiverTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/src/test/java/hudson/util/io/TarArchiverTest.java b/core/src/test/java/hudson/util/io/TarArchiverTest.java index a7c635530592..81b7327d8f67 100644 --- a/core/src/test/java/hudson/util/io/TarArchiverTest.java +++ b/core/src/test/java/hudson/util/io/TarArchiverTest.java @@ -36,6 +36,7 @@ import hudson.util.StreamTaskListener; import java.io.File; import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; import java.nio.charset.StandardCharsets; import java.nio.file.Files; @@ -142,7 +143,8 @@ public void emptyDirectory() throws Exception { } Set names = new HashSet<>(); - try (TarInputStream tarInputStream = new TarInputStream(Files.newInputStream(tar), StandardCharsets.UTF_8.name())) { + try (InputStream inputStream = Files.newInputStream(tar); + TarInputStream tarInputStream = new TarInputStream(inputStream, StandardCharsets.UTF_8.name())) { TarEntry tarEntry; while ((tarEntry = tarInputStream.getNextEntry()) != null) { names.add(tarEntry.getName()); From 542ef5abb3ecedde042204deaa78915c1a877173 Mon Sep 17 00:00:00 2001 From: Simeon L Date: Tue, 15 Oct 2024 10:34:28 +0200 Subject: [PATCH 17/21] chore: fix spotless --- core/src/test/java/hudson/util/io/ZipArchiverTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/core/src/test/java/hudson/util/io/ZipArchiverTest.java b/core/src/test/java/hudson/util/io/ZipArchiverTest.java index 749b7eb3460e..c1b458b89a9c 100644 --- a/core/src/test/java/hudson/util/io/ZipArchiverTest.java +++ b/core/src/test/java/hudson/util/io/ZipArchiverTest.java @@ -3,7 +3,6 @@ import static org.junit.Assert.assertEquals; import hudson.FilePath; - import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -16,7 +15,6 @@ import java.util.zip.ZipEntry; import java.util.zip.ZipFile; import java.util.zip.ZipInputStream; - import org.junit.Assume; import org.junit.Rule; import org.junit.Test; From 45d34c6d779a813cc2f0000b106e1731b9ac98b7 Mon Sep 17 00:00:00 2001 From: Basil Crow Date: Tue, 15 Oct 2024 08:44:34 -0700 Subject: [PATCH 18/21] Revert unnecessary changes to minimize diff --- .../java/hudson/util/io/TarArchiverTest.java | 25 ++++++++----------- .../java/hudson/util/io/ZipArchiverTest.java | 24 ++++++++---------- 2 files changed, 22 insertions(+), 27 deletions(-) diff --git a/core/src/test/java/hudson/util/io/TarArchiverTest.java b/core/src/test/java/hudson/util/io/TarArchiverTest.java index 81b7327d8f67..0d6d5e2d9e70 100644 --- a/core/src/test/java/hudson/util/io/TarArchiverTest.java +++ b/core/src/test/java/hudson/util/io/TarArchiverTest.java @@ -133,24 +133,21 @@ private static void run(FilePath dir, String... cmds) throws InterruptedExceptio public void emptyDirectory() throws Exception { Path tar = tmp.newFile("test.tar").toPath(); Path root = tmp.newFolder().toPath(); - Files.createDirectory(root.resolve("a")); - Files.createDirectory(root.resolve("b")); - - Files.writeString(root.resolve("a/file.txt"), "empty_dir_test"); - - try (OutputStream outputStream = Files.newOutputStream(tar)) { - new FilePath(root.toFile()).tar(outputStream, "**"); + Files.createDirectory(root.resolve("foo")); + Files.createDirectory(root.resolve("bar")); + Files.writeString(root.resolve("bar/file.txt"), "foobar", StandardCharsets.UTF_8); + try (OutputStream out = Files.newOutputStream(tar)) { + new FilePath(root.toFile()).tar(out, "**"); } - Set names = new HashSet<>(); - try (InputStream inputStream = Files.newInputStream(tar); - TarInputStream tarInputStream = new TarInputStream(inputStream, StandardCharsets.UTF_8.name())) { - TarEntry tarEntry; - while ((tarEntry = tarInputStream.getNextEntry()) != null) { - names.add(tarEntry.getName()); + try (InputStream is = Files.newInputStream(tar); + TarInputStream tis = new TarInputStream(is, StandardCharsets.UTF_8.name())) { + TarEntry te; + while ((te = tis.getNextEntry()) != null) { + names.add(te.getName()); } } - assertEquals(Set.of("a/", "b/", "a/file.txt"), names); + assertEquals(Set.of("foo/", "bar/", "bar/file.txt"), names); } /** diff --git a/core/src/test/java/hudson/util/io/ZipArchiverTest.java b/core/src/test/java/hudson/util/io/ZipArchiverTest.java index c1b458b89a9c..86a939ed1fc5 100644 --- a/core/src/test/java/hudson/util/io/ZipArchiverTest.java +++ b/core/src/test/java/hudson/util/io/ZipArchiverTest.java @@ -23,8 +23,7 @@ public class ZipArchiverTest { - @Rule - public TemporaryFolder tmp = new TemporaryFolder(); + @Rule public TemporaryFolder tmp = new TemporaryFolder(); @Issue("JENKINS-9942") @Test @@ -86,21 +85,20 @@ public void huge64bitFile() throws IOException { public void emptyDirectory() throws Exception { Path zip = tmp.newFile("test.zip").toPath(); Path root = tmp.newFolder().toPath(); - Files.createDirectory(root.resolve("a")); - Files.createDirectory(root.resolve("b")); - Files.writeString(root.resolve("a/file.txt"), "empty_dir_zip_test"); + Files.createDirectory(root.resolve("foo")); + Files.createDirectory(root.resolve("bar")); + Files.writeString(root.resolve("bar/file.txt"), "foobar", StandardCharsets.UTF_8); try (OutputStream out = Files.newOutputStream(zip)) { new FilePath(root.toFile()).zip(out, "**"); } - Set actual = new HashSet<>(); - Set expected = Set.of("a/", "b/", "a/file.txt"); - try (InputStream inputStream = Files.newInputStream(zip); - ZipInputStream zipInputStream = new ZipInputStream(inputStream, StandardCharsets.UTF_8)) { - ZipEntry zipEntry; - while ((zipEntry = zipInputStream.getNextEntry()) != null) { - actual.add(zipEntry.getName()); + Set names = new HashSet<>(); + try (InputStream is = Files.newInputStream(zip); + ZipInputStream zis = new ZipInputStream(is, StandardCharsets.UTF_8)) { + ZipEntry ze; + while ((ze = zis.getNextEntry()) != null) { + names.add(ze.getName()); } } - assertEquals(expected, actual); + assertEquals(Set.of("foo/", "bar/", "bar/file.txt"), names); } } From 14cf0bfcbc98fd10f5680bea244385c4f2fb5139 Mon Sep 17 00:00:00 2001 From: Basil Crow Date: Tue, 15 Oct 2024 09:15:57 -0700 Subject: [PATCH 19/21] Strengthen assertions --- .../java/jenkins/util/VirtualFileTest.java | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/core/src/test/java/jenkins/util/VirtualFileTest.java b/core/src/test/java/jenkins/util/VirtualFileTest.java index 8fac5f9adacf..b8db484b704e 100644 --- a/core/src/test/java/jenkins/util/VirtualFileTest.java +++ b/core/src/test/java/jenkins/util/VirtualFileTest.java @@ -27,11 +27,9 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.hamcrest.Matchers.containsInRelativeOrder; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.greaterThan; -import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.lessThan; @@ -239,10 +237,16 @@ public void list_Glob_NoFollowLinks_FileVF() throws Exception { File root = tmp.getRoot(); VirtualFile virtualRoot = VirtualFile.forFile(root); Collection children = virtualRoot.list("**", null, true, LinkOption.NOFOLLOW_LINKS); - assertThat(children, containsInRelativeOrder( + assertThat(children, containsInAnyOrder( "a/aa/aa.txt", "a/ab/ab.txt", - "b/ba/ba.txt" + "b/ba/ba.txt", + "a", + "a/aa", + "a/aa/aaa", + "a/ab", + "b", + "b/ba" )); } @@ -255,10 +259,16 @@ public void list_Glob_NoFollowLinks_FilePathVF() throws Exception { File root = tmp.getRoot(); VirtualFile virtualRoot = VirtualFile.forFilePath(new FilePath(root)); Collection children = virtualRoot.list("**", null, true, LinkOption.NOFOLLOW_LINKS); - assertThat(children, hasItems( + assertThat(children, containsInAnyOrder( "a/aa/aa.txt", "a/ab/ab.txt", - "b/ba/ba.txt" + "b/ba/ba.txt", + "a", + "a/aa", + "a/aa/aaa", + "a/ab", + "b", + "b/ba" )); } @@ -506,7 +516,7 @@ public void list_Glob_NoFollowLinks_ExternalSymlink_FilePathVF() throws Exceptio VirtualFile symlinkVirtualPath = VirtualFile.forFilePath(symlinkPath); VirtualFile symlinkChildVirtualPath = symlinkVirtualPath.child("aa"); Collection children = symlinkChildVirtualPath.list("**", null, true, LinkOption.NOFOLLOW_LINKS); - assertThat(children, hasItems("aa.txt")); + assertThat(children, containsInAnyOrder("aaa", "aa.txt")); } @Test @@ -521,7 +531,7 @@ public void list_Glob_NoFollowLinks_ExternalSymlink_FileVF() throws Exception { VirtualFile symlinkVirtualFile = VirtualFile.forFile(symlinkFile); VirtualFile symlinkChildVirtualFile = symlinkVirtualFile.child("aa"); Collection children = symlinkChildVirtualFile.list("**", null, true, LinkOption.NOFOLLOW_LINKS); - assertThat(children, hasItems("aa.txt")); + assertThat(children, containsInAnyOrder("aaa", "aa.txt")); } @Test From 52fa89e84cac8b69ba20c5ff47a115b681c67045 Mon Sep 17 00:00:00 2001 From: Basil Crow Date: Tue, 15 Oct 2024 09:16:06 -0700 Subject: [PATCH 20/21] No need to call collectFiles on a file --- core/src/main/java/jenkins/util/VirtualFile.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/jenkins/util/VirtualFile.java b/core/src/main/java/jenkins/util/VirtualFile.java index 660c089eea32..b7456e3a80e3 100644 --- a/core/src/main/java/jenkins/util/VirtualFile.java +++ b/core/src/main/java/jenkins/util/VirtualFile.java @@ -328,7 +328,9 @@ public Collection call() throws IOException { private static void collectFiles(VirtualFile d, Collection names, String prefix) throws IOException { for (VirtualFile child : d.list()) { names.add(prefix + child.getName()); - collectFiles(child, names, prefix + child.getName() + "/"); + if (child.isDirectory()) { + collectFiles(child, names, prefix + child.getName() + "/"); + } } } } From 62f840ba89e251105e24a50d3200fc0d32be3154 Mon Sep 17 00:00:00 2001 From: "Simeon L." Date: Tue, 15 Oct 2024 21:22:07 +0200 Subject: [PATCH 21/21] chore: strengthen assertions in DirectoryBrowserSupportTest --- .../model/DirectoryBrowserSupportTest.java | 37 ++++++++++++------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java index 2cda73ccc6ba..e0161fc3eeda 100644 --- a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java +++ b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java @@ -30,7 +30,6 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItem; -import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertEquals; @@ -685,9 +684,11 @@ public void symlink_outsideWorkspace_areNotAllowed() throws Exception { assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, hasItems( + assertThat(entryNames, containsInAnyOrder( p.getName() + "/intermediateFolder/public2.key", - p.getName() + "/public1.key" + p.getName() + "/public1.key", + p.getName() + "/intermediateFolder/", + p.getName() + "/intermediateFolder/otherFolder/" )); } { // workaround for JENKINS-19947 is still supported, i.e. no parent folder @@ -695,9 +696,12 @@ public void symlink_outsideWorkspace_areNotAllowed() throws Exception { assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, hasItems( + assertThat(entryNames, containsInAnyOrder( "intermediateFolder/public2.key", - "public1.key" + "public1.key", + "intermediateFolder/", + "intermediateFolder/otherFolder/" + )); } { // all the outside folders / files are not included in the zip @@ -705,14 +709,14 @@ public void symlink_outsideWorkspace_areNotAllowed() throws Exception { assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, hasItems("intermediateFolder/public2.key")); + assertThat(entryNames, containsInAnyOrder("intermediateFolder/public2.key", "intermediateFolder/otherFolder/")); } { // workaround for JENKINS-19947 is still supported, i.e. no parent folder, even inside a sub-folder Page zipPage = wc.goTo(p.getUrl() + "ws/intermediateFolder/**/*zip*/intermediateFolder.zip", null); assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, hasItems("public2.key")); + assertThat(entryNames, containsInAnyOrder("public2.key", "otherFolder/")); } } @@ -915,9 +919,11 @@ public void junctionAndSymlink_outsideWorkspace_areNotAllowed_windowsJunction() assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, hasItems( + assertThat(entryNames, containsInAnyOrder( p.getName() + "/intermediateFolder/public2.key", - p.getName() + "/public1.key" + p.getName() + "/public1.key", + p.getName() + "/intermediateFolder/", + p.getName() + "/intermediateFolder/otherFolder/" )); } { // all the outside folders / files are not included in the zip @@ -925,7 +931,7 @@ public void junctionAndSymlink_outsideWorkspace_areNotAllowed_windowsJunction() assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, hasItems("intermediateFolder/public2.key")); + assertThat(entryNames, containsInAnyOrder("intermediateFolder/public2.key", "intermediateFolder/otherFolder/")); } // Explicitly delete everything including junctions, which TemporaryDirectoryAllocator.dispose may have trouble with: new Launcher.LocalLauncher(StreamTaskListener.fromStderr()).launch().cmds("cmd", "/c", "rmdir", "/s", "/q", j.jenkins.getRootDir().getAbsolutePath()).start().join(); @@ -1271,9 +1277,11 @@ public boolean perform(AbstractBuild build, Launcher launcher, BuildListen List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); assertThat(entryNames, hasSize(4)); - assertThat(entryNames, hasItems( + assertThat(entryNames, containsInAnyOrder( "test0/anotherDir/one.txt", - "test0/anotherDir/insideDir/two.txt" + "test0/anotherDir/insideDir/two.txt", + "test0/anotherDir/", + "test0/anotherDir/insideDir/" )); zipPage = wc.goTo("job/" + p.getName() + "/ws/anotherDir/*zip*/" + p.getName(), null); @@ -1281,9 +1289,10 @@ public boolean perform(AbstractBuild build, Launcher launcher, BuildListen entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); assertThat(entryNames, hasSize(3)); - assertThat(entryNames, hasItems( + assertThat(entryNames, containsInAnyOrder( "anotherDir/one.txt", - "anotherDir/insideDir/two.txt" + "anotherDir/insideDir/two.txt", + "anotherDir/insideDir/" )); }