From 3f01a774662d0e6e3d4f644e6a3197009f0c7b14 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 19 Feb 2018 18:01:52 -0500 Subject: [PATCH 01/20] [JENKINS-49635] Defining new VirtualFile methods to better support external artifact storage. --- .../hudson/model/DirectoryBrowserSupport.java | 10 + .../main/java/jenkins/util/VirtualFile.java | 64 +++++- .../model/DirectoryBrowserSupportTest.java | 214 ++++++++++++++++++ 3 files changed, 286 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java index 5426c7842e11..da6ad3c2bd4f 100644 --- a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java +++ b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java @@ -29,6 +29,7 @@ import java.io.InputStream; import java.io.OutputStream; import java.io.Serializable; +import java.net.URL; import java.text.Collator; import java.util.ArrayList; import java.util.Arrays; @@ -51,6 +52,7 @@ import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; import org.kohsuke.stapler.HttpResponse; +import org.kohsuke.stapler.HttpResponses; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; @@ -295,6 +297,14 @@ private void serveFile(StaplerRequest req, StaplerResponse rsp, VirtualFile root return; } + URL external = baseFile.toExternalURL(); + if (external != null) { + // or this URL could be emitted directly from dir.jelly + // though we would prefer to delay toExternalURL calls unless and until needed + rsp.sendRedirect2(external.toExternalForm()); + return; + } + long lastModified = baseFile.lastModified(); long length = baseFile.length(); diff --git a/core/src/main/java/jenkins/util/VirtualFile.java b/core/src/main/java/jenkins/util/VirtualFile.java index e2d27d75ec6a..8376267e8545 100644 --- a/core/src/main/java/jenkins/util/VirtualFile.java +++ b/core/src/main/java/jenkins/util/VirtualFile.java @@ -28,16 +28,17 @@ import hudson.model.DirectoryBrowserSupport; import hudson.remoting.Callable; import hudson.remoting.Channel; +import hudson.remoting.RemoteInputStream; import hudson.remoting.VirtualChannel; import hudson.util.DirScanner; import hudson.util.FileVisitor; import java.io.File; -import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.io.Serializable; import java.net.URI; +import java.net.URL; import java.nio.file.Files; import java.nio.file.InvalidPathException; import java.nio.file.LinkOption; @@ -45,9 +46,10 @@ import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; +import javax.annotation.CheckForNull; import javax.annotation.Nonnull; - import jenkins.MasterToSlaveFileCallable; +import jenkins.model.ArtifactManager; /** * Abstraction over {@link File}, {@link FilePath}, or other items such as network resources or ZIP entries. @@ -62,6 +64,26 @@ * {@link VirtualFile} makes no assumption about where the actual files are, or whether there really exists * {@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, + * but an implementation from {@link #forFile} will not.) + * Thus callers should assume that methods such as {@link #open} will work + * only on the node on which the object was created. + * + *

Since some implementations may in fact use external file storage, + * callers may request optional APIs to access those services more efficiently. + * Otherwise, for example, a plugin copying a file + * previously saved by {@link ArtifactManager} to an external storage servuce + * which tunneled a stream from {@link #open} using {@link RemoteInputStream} + * would wind up transferring the file from the service to the Jenkins master and then on to an agent. + * Similarly, if {@link DirectoryBrowserSupport} rendered a link to an in-Jenkins URL, + * a large file could be transferred from the service to the Jenkins master and then on to the browser. + * To avoid this overhead, callers may check whether an implementation + * supports {@link #asRemotable} and/or {@link #toExternalURL}. + * * @see DirectoryBrowserSupport * @see FilePath * @since 1.532 @@ -80,6 +102,7 @@ public abstract class VirtualFile implements Comparable, Serializab /** * Gets a URI. * Should at least uniquely identify this virtual file within its root, but not necessarily globally. + *

When {@link #toExternalURL} is implemented, it is natural but not required to use that same value here. * @return a URI (need not be absolute) */ public abstract URI toURI(); @@ -208,6 +231,43 @@ public V run(Callable callable) throws IOException { return callable.call(); } + /** + * Optionally produces a variant of this handle which may be safely passed over a Remoting {@link Channel}. + * This would allow remote nodes such as agents to make calls such as {@link #open} + * and be assured of the most efficient possible access. + * Otherwise, all calls must be made on the node originally producing this object, + * and the caller must arrange for transport of the result. + *

Note that the result of {@link #forFilePath} does not implement this method, + * since a {@link FilePath} may only be transferred over the channel on which it was created. + * It cannot, for example, be used to represent a workspace file from one agent on another agent. + * @return this object or a variant which may be passed over a {@link Channel}, or null if there is no such support + * @since FIXME + * @see #toExternalURL + */ + public @CheckForNull VirtualFile asRemotable() { + return null; + } + + /** + * Optionally obtains a URL which may be used to retrieve file contents from any process on any node. + * For example, given cloud storage this might produce a permalink to the file. + *

This is only meaningful for {@link #isFile}: + * no ZIP etc. archiving protocol is defined to allow bulk access to directory trees. + *

Any necessary authentication must be encoded somehow into the URL itself; + * do not include any tokens or other authentication which might allow access to unrelated files + * (for example {@link ArtifactManager} builds from a different job). + *

Generally this will be harder to implement than {@link #asRemotable}, + * which would have the opportunity to perform arbitrary preparation for {@link #open} + * such as negotiating session authentication. + * @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 FIXME + * @see #toURI + * @see #asRemotable + */ + public @CheckForNull URL toExternalURL() throws IOException { + return null; + } + /** * Creates a virtual file wrapper for a local file. * @param f a disk file (need not exist) diff --git a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java index 57583b878faf..524144e3481b 100644 --- a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java +++ b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java @@ -23,6 +23,7 @@ */ package hudson.model; +import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -51,8 +52,26 @@ import com.gargoylesoftware.htmlunit.Page; import com.gargoylesoftware.htmlunit.UnexpectedPage; import com.gargoylesoftware.htmlunit.html.HtmlPage; +import hudson.ExtensionList; +import hudson.Util; +import java.io.ByteArrayInputStream; +import java.io.FileNotFoundException; import java.io.OutputStream; +import java.net.URI; +import java.net.URL; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import jenkins.model.ArtifactManager; +import jenkins.model.ArtifactManagerConfiguration; +import jenkins.model.ArtifactManagerFactory; +import jenkins.model.ArtifactManagerFactoryDescriptor; +import jenkins.model.Jenkins; +import jenkins.util.VirtualFile; import org.apache.commons.io.IOUtils; +import org.jvnet.hudson.test.TestExtension; +import org.kohsuke.stapler.StaplerRequest; +import org.kohsuke.stapler.StaplerResponse; /** * @author Kohsuke Kawaguchi @@ -212,4 +231,199 @@ private File download(UnexpectedPage page) throws IOException { return file; } + + @Issue("JENKINS-49635") + @Test + public void externalURLDownload() throws Exception { + ArtifactManagerConfiguration.get().getArtifactManagerFactories().add(new ExternalArtifactManagerFactory()); + FreeStyleProject p = j.createFreeStyleProject(); + p.setScm(new SingleFileSCM("f", "Hello world!")); + p.getPublishersList().add(new ArtifactArchiver("f")); + j.buildAndAssertSuccess(p); + HtmlPage page = j.createWebClient().goTo("job/" + p.getName() + "/lastSuccessfulBuild/artifact/"); + try { + Page download = page.getAnchorByText("f").click(); + assertEquals("Hello world!", download.getWebResponse().getContentAsString()); + } catch (FailingHttpStatusCodeException x) { + IOUtils.copy(x.getResponse().getContentAsStream(), System.err); + throw x; + } + } + @TestExtension("externalURLDownload") + public static final class ContentAddressableStore implements UnprotectedRootAction { + final List files = new ArrayList<>(); + @Override + public String getUrlName() { + return "files"; + } + @Override + public String getIconFileName() { + return null; + } + @Override + public String getDisplayName() { + return null; + } + public void doDynamic(StaplerRequest req, StaplerResponse rsp) throws Exception { + String hash = req.getRestOfPath().substring(1); + for (byte[] file : files) { + if (Util.getDigestOf(new ByteArrayInputStream(file)).equals(hash)) { + rsp.setContentType("application/octet-stream"); + rsp.getOutputStream().write(file); + return; + } + } + rsp.sendError(404); + } + } + public static final class ExternalArtifactManagerFactory extends ArtifactManagerFactory { + @Override + public ArtifactManager managerFor(Run build) { + return new ExternalArtifactManager(); + } + @TestExtension("externalURLDownload") + public static final class DescriptorImpl extends ArtifactManagerFactoryDescriptor {} + } + private static final class ExternalArtifactManager extends ArtifactManager { + String hash; + @Override + public void archive(FilePath workspace, Launcher launcher, BuildListener listener, Map artifacts) throws IOException, InterruptedException { + assertEquals(1, artifacts.size()); + Map.Entry entry = artifacts.entrySet().iterator().next(); + assertEquals("f", entry.getKey()); + try (InputStream is = workspace.child(entry.getValue()).read()) { + byte[] data = IOUtils.toByteArray(is); + ExtensionList.lookupSingleton(ContentAddressableStore.class).files.add(data); + hash = Util.getDigestOf(new ByteArrayInputStream(data)); + } + } + @Override + public VirtualFile root() { + final VirtualFile file = new VirtualFile() { + @Override + public String getName() { + return "f"; + } + @Override + public URI toURI() { + return URI.create("root:f"); + } + @Override + public VirtualFile getParent() { + return root(); + } + @Override + public boolean isDirectory() throws IOException { + return false; + } + @Override + public boolean isFile() throws IOException { + return true; + } + @Override + public boolean exists() throws IOException { + return true; + } + @Override + public VirtualFile[] list() throws IOException { + return new VirtualFile[0]; + } + @Override + public String[] list(String glob) throws IOException { + return new String[0]; + } + @Override + public VirtualFile child(String name) { + throw new UnsupportedOperationException(); + } + @Override + public long length() throws IOException { + return 0; + } + @Override + public long lastModified() throws IOException { + return 0; + } + @Override + public boolean canRead() throws IOException { + return true; + } + @Override + public InputStream open() throws IOException { + throw new FileNotFoundException("expect to be opened via URL only"); + } + @Override + public URL toExternalURL() throws IOException { + return new URL(Jenkins.get().getRootUrl() + "files/" + hash); + } + }; + return new VirtualFile() { + @Override + public String getName() { + return ""; + } + @Override + public URI toURI() { + return URI.create("root:"); + } + @Override + public VirtualFile getParent() { + return this; + } + @Override + public boolean isDirectory() throws IOException { + return true; + } + @Override + public boolean isFile() throws IOException { + return false; + } + @Override + public boolean exists() throws IOException { + return true; + } + @Override + public VirtualFile[] list() throws IOException { + return new VirtualFile[] {file}; + } + @Override + public String[] list(String glob) throws IOException { + throw new UnsupportedOperationException(); + } + @Override + public VirtualFile child(String name) { + if (name.equals("f")) { + return file; + } else if (name.isEmpty()) { + return this; + } else { + throw new UnsupportedOperationException("trying to call child on " + name); + } + } + @Override + public long length() throws IOException { + return 0; + } + @Override + public long lastModified() throws IOException { + return 0; + } + @Override + public boolean canRead() throws IOException { + return true; + } + @Override + public InputStream open() throws IOException { + throw new FileNotFoundException(); + } + }; + } + @Override + public void onLoad(Run build) {} + @Override + public boolean delete() throws IOException, InterruptedException { + return false; + } + } + } From 8e603a5986079f7137a571e6ce1c2d9d971a772d Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 21 Feb 2018 12:01:43 -0500 Subject: [PATCH 02/20] [JENKINS-26810] Added VirtualFile.mode. --- .../main/java/jenkins/util/VirtualFile.java | 42 +++++++++++++++---- .../java/jenkins/util/VirtualFileTest.java | 19 +++++++++ 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/jenkins/util/VirtualFile.java b/core/src/main/java/jenkins/util/VirtualFile.java index 8376267e8545..45b9c74cd44e 100644 --- a/core/src/main/java/jenkins/util/VirtualFile.java +++ b/core/src/main/java/jenkins/util/VirtualFile.java @@ -26,12 +26,14 @@ import hudson.FilePath; import hudson.model.DirectoryBrowserSupport; +import hudson.os.PosixException; import hudson.remoting.Callable; import hudson.remoting.Channel; import hudson.remoting.RemoteInputStream; import hudson.remoting.VirtualChannel; import hudson.util.DirScanner; import hudson.util.FileVisitor; +import hudson.util.IOUtils; import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; @@ -171,6 +173,15 @@ public abstract class VirtualFile implements Comparable, Serializab */ public abstract long lastModified() throws IOException; + /** + * Gets the file’s Unix mode, if meaningful. + * @return for example, 0644 ~ {@code rw-r--r--}; -1 by default, meaning unknown or inapplicable + * @throws IOException if checking the mode failed + */ + public int mode() throws IOException { + return -1; + } + /** * Checks whether this file can be read. * @return true normally @@ -339,6 +350,12 @@ private static final class FileVF extends VirtualFile { } return f.length(); } + @Override public int mode() throws IOException { + if (isIllegalSymlink()) { + return -1; + } + return IOUtils.mode(f); + } @Override public long lastModified() throws IOException { if (isIllegalSymlink()) { return 0; @@ -408,7 +425,7 @@ private static final class FilePathVF extends VirtualFile { try { return f.isDirectory(); } catch (InterruptedException x) { - throw (IOException) new IOException(x.toString()).initCause(x); + throw new IOException(x); } } @Override public boolean isFile() throws IOException { @@ -419,7 +436,7 @@ private static final class FilePathVF extends VirtualFile { try { return f.exists(); } catch (InterruptedException x) { - throw (IOException) new IOException(x.toString()).initCause(x); + throw new IOException(x); } } @Override public VirtualFile[] list() throws IOException { @@ -431,14 +448,14 @@ private static final class FilePathVF extends VirtualFile { } return vfs; } catch (InterruptedException x) { - throw (IOException) new IOException(x.toString()).initCause(x); + throw new IOException(x); } } @Override public String[] list(String glob) throws IOException { try { return f.act(new Scanner(glob)); } catch (InterruptedException x) { - throw (IOException) new IOException(x.toString()).initCause(x); + throw new IOException(x); } } @Override public VirtualFile child(String name) { @@ -448,35 +465,42 @@ private static final class FilePathVF extends VirtualFile { try { return f.length(); } catch (InterruptedException x) { - throw (IOException) new IOException(x.toString()).initCause(x); + throw new IOException(x); + } + } + @Override public int mode() throws IOException { + try { + return f.mode(); + } catch (InterruptedException | PosixException x) { + throw new IOException(x); } } @Override public long lastModified() throws IOException { try { return f.lastModified(); } catch (InterruptedException x) { - throw (IOException) new IOException(x.toString()).initCause(x); + throw new IOException(x); } } @Override public boolean canRead() throws IOException { try { return f.act(new Readable()); } catch (InterruptedException x) { - throw (IOException) new IOException(x.toString()).initCause(x); + throw new IOException(x); } } @Override public InputStream open() throws IOException { try { return f.read(); } catch (InterruptedException x) { - throw (IOException) new IOException(x.toString()).initCause(x); + throw new IOException(x); } } @Override public V run(Callable callable) throws IOException { try { return f.act(callable); } catch (InterruptedException x) { - throw (IOException) new IOException(x.toString()).initCause(x); + throw new IOException(x); } } } diff --git a/core/src/test/java/jenkins/util/VirtualFileTest.java b/core/src/test/java/jenkins/util/VirtualFileTest.java index 1d0b8457f59f..77c31a828f4b 100644 --- a/core/src/test/java/jenkins/util/VirtualFileTest.java +++ b/core/src/test/java/jenkins/util/VirtualFileTest.java @@ -24,12 +24,15 @@ package jenkins.util; +import hudson.FilePath; import hudson.Functions; import hudson.Util; import hudson.model.TaskListener; import java.io.File; import java.io.FileNotFoundException; +import java.io.IOException; import java.nio.file.NoSuchFileException; +import java.nio.file.attribute.PosixFilePermissions; import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; import org.junit.Test; @@ -69,4 +72,20 @@ public class VirtualFileTest { } } + @Issue("JENKINS-26810") + @Test public void mode() throws Exception { + File f = tmp.newFile(); + VirtualFile vf = VirtualFile.forFile(f); + FilePath fp = new FilePath(f); + VirtualFile vfp = VirtualFile.forFilePath(fp); + assertEquals(modeString(hudson.util.IOUtils.mode(f)), modeString(vf.mode())); + assertEquals(modeString(hudson.util.IOUtils.mode(f)), modeString(vfp.mode())); + fp.chmod(0755); + assertEquals(modeString(hudson.util.IOUtils.mode(f)), modeString(vf.mode())); + assertEquals(modeString(hudson.util.IOUtils.mode(f)), modeString(vfp.mode())); + } + private static String modeString(int mode) throws IOException { + return mode == -1 ? "N/A" : PosixFilePermissions.toString(Util.modeToPermissions(mode)); + } + } From e773bdb24b73aa5995556ca6474b73d878d18e53 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 21 Feb 2018 14:17:48 -0500 Subject: [PATCH 03/20] New VirtualFile.list overload with more capabilities and more precisely defined semantics. --- .../hudson/model/DirectoryBrowserSupport.java | 9 +-- .../main/java/jenkins/util/VirtualFile.java | 72 +++++++++++++++---- .../java/jenkins/util/VirtualFileTest.java | 25 +++++++ .../model/DirectoryBrowserSupportTest.java | 8 ++- 4 files changed, 92 insertions(+), 22 deletions(-) diff --git a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java index da6ad3c2bd4f..3baa9190f596 100644 --- a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java +++ b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java @@ -33,6 +33,7 @@ import java.text.Collator; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.List; @@ -365,7 +366,7 @@ private static String createBackRef(int times) { private static void zip(OutputStream outputStream, VirtualFile dir, String glob) throws IOException { try (ZipOutputStream zos = new ZipOutputStream(outputStream)) { zos.setEncoding(System.getProperty("file.encoding")); // TODO JENKINS-20663 make this overridable via query parameter - for (String n : dir.list(glob.length() == 0 ? "**" : glob)) { + for (String n : dir.list(glob, null, /* TODO what is the user expectation? */true)) { String relativePath; if (glob.length() == 0) { // JENKINS-19947: traditional behavior is to prepend the directory name @@ -545,10 +546,10 @@ private static List> buildChildPaths(VirtualFile cur, Locale locale) * @param baseRef String like "../../../" that cancels the 'rest' portion. Can be "./" */ private static List> patternScan(VirtualFile baseDir, String pattern, String baseRef) throws IOException { - String[] files = baseDir.list(pattern); + Collection files = baseDir.list(pattern, null, /* TODO what is the user expectation? */true); - if (files.length > 0) { - List> r = new ArrayList>(files.length); + if (!files.isEmpty()) { + List> r = new ArrayList>(files.size()); for (String match : files) { List file = buildPathList(baseDir, baseDir.child(match), baseRef); r.add(file); diff --git a/core/src/main/java/jenkins/util/VirtualFile.java b/core/src/main/java/jenkins/util/VirtualFile.java index 45b9c74cd44e..a4f80cb87571 100644 --- a/core/src/main/java/jenkins/util/VirtualFile.java +++ b/core/src/main/java/jenkins/util/VirtualFile.java @@ -25,6 +25,7 @@ package jenkins.util; import hudson.FilePath; +import hudson.Util; import hudson.model.DirectoryBrowserSupport; import hudson.os.PosixException; import hudson.remoting.Callable; @@ -45,13 +46,20 @@ import java.nio.file.InvalidPathException; import java.nio.file.LinkOption; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.stream.Collectors; +import java.util.stream.Stream; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; import jenkins.MasterToSlaveFileCallable; import jenkins.model.ArtifactManager; +import org.apache.tools.ant.types.AbstractFileSet; +import org.apache.tools.ant.types.selectors.SelectorUtils; /** * Abstraction over {@link File}, {@link FilePath}, or other items such as network resources or ZIP entries. @@ -144,13 +152,40 @@ public abstract class VirtualFile implements Comparable, Serializab */ public abstract @Nonnull VirtualFile[] list() throws IOException; + /** + * @deprecated use {@link #list(String, String, boolean)} instead + */ + @Deprecated + public @Nonnull String[] list(String glob) throws IOException { + return list(glob.replace('\\', '/'), null, true).toArray(new String[0]); + } + /** * Lists recursive files of this directory with pattern matching. - * @param glob an Ant-style glob - * @return a list of relative names of children (files directly inside or in subdirectories) + * @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 */ - public abstract @Nonnull String[] list(String glob) throws IOException; + public @Nonnull Collection list(@Nonnull String includes, @CheckForNull String excludes, boolean useDefaultExcludes) throws IOException { + if (Util.isOverridden(VirtualFile.class, getClass(), "list", String.class)) { + String[] included = list(includes.isEmpty() ? "**" : includes); + if (excludes != null) { + // Do our best to apply excludes and hope defaultexcludes are not relevant (original File/FilePath impls assumed uDE=true). + // Or we could use list() recursively and apply includes and excludes without calling list(String) at all. + return Stream.of(included). + filter(path -> SelectorUtils.match(excludes.endsWith("/") ? excludes + SelectorUtils.DEEP_TREE_MATCH : excludes, path)). + map(path -> path.replace('\\', '/')). + collect(Collectors.toList()); + } else { + return Arrays.asList(included); + } + } else { + throw new AbstractMethodError("implement " + getClass().getName() + ".list(String, String, boolean)"); + } + } /** * Obtains a child file. @@ -335,11 +370,12 @@ private static final class FileVF extends VirtualFile { } return vfs; } - @Override public String[] list(String glob) throws IOException { + @Override + public Collection list(String includes, String excludes, boolean useDefaultExcludes) throws IOException { if (isIllegalSymlink()) { - return new String[0]; + return Collections.emptySet(); } - return new Scanner(glob).invoke(f, null); + return new Scanner(includes, excludes, useDefaultExcludes).invoke(f, null); } @Override public VirtualFile child(String name) { return new FileVF(new File(f, name), root); @@ -451,9 +487,9 @@ private static final class FilePathVF extends VirtualFile { throw new IOException(x); } } - @Override public String[] list(String glob) throws IOException { + @Override public Collection list(String includes, String excludes, boolean useDefaultExcludes) throws IOException { try { - return f.act(new Scanner(glob)); + return f.act(new Scanner(includes, excludes, useDefaultExcludes)); } catch (InterruptedException x) { throw new IOException(x); } @@ -504,20 +540,26 @@ private static final class FilePathVF extends VirtualFile { } } } - private static final class Scanner extends MasterToSlaveFileCallable { - private final String glob; - Scanner(String glob) { - this.glob = glob; + private static final class Scanner extends MasterToSlaveFileCallable> { + private final String includes, excludes; + private final boolean useDefaultExcludes; + Scanner(String includes, String excludes, boolean useDefaultExcludes) { + this.includes = includes; + this.excludes = excludes; + this.useDefaultExcludes = useDefaultExcludes; } - @Override public String[] 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(); + } final List paths = new ArrayList(); - new DirScanner.Glob(glob, null).scan(f, new FileVisitor() { + new DirScanner.Glob(includes, excludes, useDefaultExcludes).scan(f, new FileVisitor() { @Override public void visit(File f, String relativePath) throws IOException { paths.add(relativePath); } }); - return paths.toArray(new String[paths.size()]); + return paths; } } diff --git a/core/src/test/java/jenkins/util/VirtualFileTest.java b/core/src/test/java/jenkins/util/VirtualFileTest.java index 77c31a828f4b..c19e1677f5c1 100644 --- a/core/src/test/java/jenkins/util/VirtualFileTest.java +++ b/core/src/test/java/jenkins/util/VirtualFileTest.java @@ -33,6 +33,7 @@ import java.io.IOException; import java.nio.file.NoSuchFileException; import java.nio.file.attribute.PosixFilePermissions; +import java.util.TreeSet; import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; import org.junit.Test; @@ -88,4 +89,28 @@ private static String modeString(int mode) throws IOException { return mode == -1 ? "N/A" : PosixFilePermissions.toString(Util.modeToPermissions(mode)); } + @Issue("JENKINS-26810") + @Test public void list() throws Exception { + File root = tmp.getRoot(); + FilePath rootF = new FilePath(root); + rootF.child("top.txt").write("", null); + rootF.child("sub/mid.txt").write("", null); + rootF.child("sub/subsub/lowest.txt").write("", null); + rootF.child(".hg/config.txt").write("", null); + for (VirtualFile vf : new VirtualFile[] {VirtualFile.forFile(root), VirtualFile.forFilePath(rootF)}) { + 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]", 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("[.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("[top.txt]", new TreeSet<>(vf.list("*.txt", null, false)).toString()); + assertEquals("[sub/subsub/lowest.txt, top.txt]", new TreeSet<>(vf.list("**", "**/mid*,**/conf*", false)).toString()); + } + } + } diff --git a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java index 524144e3481b..8375fcce416c 100644 --- a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java +++ b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java @@ -60,6 +60,8 @@ import java.net.URI; import java.net.URL; import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Map; import jenkins.model.ArtifactManager; @@ -329,8 +331,8 @@ public VirtualFile[] list() throws IOException { return new VirtualFile[0]; } @Override - public String[] list(String glob) throws IOException { - return new String[0]; + public Collection list(String includes, String excludes, boolean useDefaultExcludes) throws IOException { + return Collections.emptySet(); } @Override public VirtualFile child(String name) { @@ -387,7 +389,7 @@ public VirtualFile[] list() throws IOException { return new VirtualFile[] {file}; } @Override - public String[] list(String glob) throws IOException { + public Collection list(String includes, String excludes, boolean useDefaultExcludes) throws IOException { throw new UnsupportedOperationException(); } @Override From 4911c5c04f3e3bd6ad314281f7f8e62f5d472a41 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 21 Feb 2018 15:03:42 -0500 Subject: [PATCH 04/20] More compatible default implementation of VirtualFile.list(String, String, boolean) which is also more convenient for some implementations. --- .../main/java/jenkins/util/VirtualFile.java | 54 +++++++++---- .../java/jenkins/util/VirtualFileTest.java | 76 +++++++++++++++++-- 2 files changed, 107 insertions(+), 23 deletions(-) diff --git a/core/src/main/java/jenkins/util/VirtualFile.java b/core/src/main/java/jenkins/util/VirtualFile.java index a4f80cb87571..6cf765f3981e 100644 --- a/core/src/main/java/jenkins/util/VirtualFile.java +++ b/core/src/main/java/jenkins/util/VirtualFile.java @@ -46,20 +46,21 @@ import java.nio.file.InvalidPathException; import java.nio.file.LinkOption; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; -import java.util.stream.Stream; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; import jenkins.MasterToSlaveFileCallable; import jenkins.model.ArtifactManager; +import org.apache.tools.ant.DirectoryScanner; import org.apache.tools.ant.types.AbstractFileSet; import org.apache.tools.ant.types.selectors.SelectorUtils; +import org.apache.tools.ant.types.selectors.TokenizedPath; +import org.apache.tools.ant.types.selectors.TokenizedPattern; /** * Abstraction over {@link File}, {@link FilePath}, or other items such as network resources or ZIP entries. @@ -162,6 +163,8 @@ public abstract class VirtualFile implements Comparable, Serializab /** * Lists recursive files of this directory with pattern matching. + *

The default implementation calls {@link #list()} recursively 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} @@ -170,21 +173,40 @@ public abstract class VirtualFile implements Comparable, Serializab * @throws IOException if this is not a directory, or listing was not possible for some other reason */ public @Nonnull Collection list(@Nonnull String includes, @CheckForNull String excludes, boolean useDefaultExcludes) throws IOException { - if (Util.isOverridden(VirtualFile.class, getClass(), "list", String.class)) { - String[] included = list(includes.isEmpty() ? "**" : includes); - if (excludes != null) { - // Do our best to apply excludes and hope defaultexcludes are not relevant (original File/FilePath impls assumed uDE=true). - // Or we could use list() recursively and apply includes and excludes without calling list(String) at all. - return Stream.of(included). - filter(path -> SelectorUtils.match(excludes.endsWith("/") ? excludes + SelectorUtils.DEEP_TREE_MATCH : excludes, path)). - map(path -> path.replace('\\', '/')). - collect(Collectors.toList()); - } else { - return Arrays.asList(included); - } - } else { - throw new AbstractMethodError("implement " + getClass().getName() + ".list(String, String, boolean)"); + List r = new ArrayList<>(); + collectFiles(r, ""); + List includePatterns = patterns(includes); + List excludePatterns = patterns(excludes); + if (useDefaultExcludes) { + for (String patt : DirectoryScanner.getDefaultExcludes()) { + excludePatterns.add(new TokenizedPattern(patt)); + } + } + return r.stream().filter(p -> { + TokenizedPath path = new TokenizedPath(p); + return includePatterns.stream().anyMatch(patt -> patt.matchPath(path, true)) && !excludePatterns.stream().anyMatch(patt -> patt.matchPath(path, true)); + }).collect(Collectors.toSet()); + } + private void collectFiles(Collection names, String prefix) throws IOException { + for (VirtualFile child : list()) { + if (child.isFile()) { + names.add(prefix + child.getName()); + } else if (child.isDirectory()) { + child.collectFiles(names, prefix + child.getName() + "/"); + } + } + } + private List patterns(String patts) { + List r = new ArrayList<>(); + if (patts != null) { + for (String patt : patts.split(",")) { + if (patt.endsWith("/")) { + patt += SelectorUtils.DEEP_TREE_MATCH; + } + r.add(new TokenizedPattern(patt)); + } } + return r; } /** diff --git a/core/src/test/java/jenkins/util/VirtualFileTest.java b/core/src/test/java/jenkins/util/VirtualFileTest.java index c19e1677f5c1..8be31af5a71c 100644 --- a/core/src/test/java/jenkins/util/VirtualFileTest.java +++ b/core/src/test/java/jenkins/util/VirtualFileTest.java @@ -24,6 +24,7 @@ package jenkins.util; +import com.google.common.collect.ImmutableSet; import hudson.FilePath; import hudson.Functions; import hudson.Util; @@ -31,11 +32,16 @@ import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; +import java.io.InputStream; +import java.net.URI; import java.nio.file.NoSuchFileException; import java.nio.file.attribute.PosixFilePermissions; +import java.util.Set; import java.util.TreeSet; +import java.util.stream.Collectors; import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; +import org.apache.commons.io.input.NullInputStream; import org.junit.Test; import static org.junit.Assert.*; import static org.junit.Assume.assumeFalse; @@ -93,14 +99,14 @@ private static String modeString(int mode) throws IOException { @Test public void list() throws Exception { File root = tmp.getRoot(); FilePath rootF = new FilePath(root); - rootF.child("top.txt").write("", null); - rootF.child("sub/mid.txt").write("", null); - rootF.child("sub/subsub/lowest.txt").write("", null); - rootF.child(".hg/config.txt").write("", null); - for (VirtualFile vf : new VirtualFile[] {VirtualFile.forFile(root), VirtualFile.forFilePath(rootF)}) { + Set paths = ImmutableSet.of("top.txt", "sub/mid.txt", "sub/subsub/lowest.txt", ".hg/config.txt", "very/deep/path/here"); + for (String path : paths) { + rootF.child(path).write("", null); + } + for (VirtualFile vf : new VirtualFile[] {VirtualFile.forFile(root), VirtualFile.forFilePath(rootF), new Ram(paths.stream().map(p -> "/" + p).collect(Collectors.toSet()), "")}) { 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]", new TreeSet<>(vf.list("**", null, false)).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("[]", 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()); @@ -109,7 +115,63 @@ private static String modeString(int mode) throws IOException { 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("[top.txt]", new TreeSet<>(vf.list("*.txt", null, false)).toString()); - assertEquals("[sub/subsub/lowest.txt, top.txt]", new TreeSet<>(vf.list("**", "**/mid*,**/conf*", false)).toString()); + assertEquals("[sub/subsub/lowest.txt, top.txt, very/deep/path/here]", new TreeSet<>(vf.list("**", "**/mid*,**/conf*", false)).toString()); + } + } + private static final class Ram extends VirtualFile { + private final Set paths; // e.g., [/very/deep/path/here] + private final String path; // e.g., empty string or /very or /very/deep/path/here + Ram(Set paths, String path) { + this.paths = paths; + this.path = path; + } + @Override + public String getName() { + return path.replaceFirst(".*/", ""); + } + @Override + public URI toURI() { + return URI.create("ram:" + path); + } + @Override + public VirtualFile getParent() { + return new Ram(paths, path.replaceFirst("/[^/]+$", "")); + } + @Override + public boolean isDirectory() throws IOException { + return paths.stream().anyMatch(p -> p.startsWith(path + "/")); + } + @Override + public boolean isFile() throws IOException { + return paths.contains(path); + } + @Override + public boolean exists() throws IOException { + return isFile() || isDirectory(); + } + @Override + public VirtualFile[] list() throws IOException { + return paths.stream().filter(p -> p.startsWith(path + "/")).map(p -> new Ram(paths, p.replaceFirst("(\\Q" + path + "\\E/[^/]+)/.+", "$1"))).collect(Collectors.toSet()).toArray(new VirtualFile[0]); + } + @Override + public VirtualFile child(String name) { + return new Ram(paths, path + "/" + name); + } + @Override + public long length() throws IOException { + return 0; + } + @Override + public long lastModified() throws IOException { + return 0; + } + @Override + public boolean canRead() throws IOException { + return isFile(); + } + @Override + public InputStream open() throws IOException { + return new NullInputStream(0); } } From e3e6d570f7065fb2d55287a98ef3b24b6a0b2e6a Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 21 Feb 2018 15:55:38 -0500 Subject: [PATCH 05/20] VirtualFile.readLink --- core/src/main/java/hudson/Util.java | 4 +-- .../main/java/jenkins/util/VirtualFile.java | 25 +++++++++++++++++++ .../java/jenkins/util/VirtualFileTest.java | 17 +++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/hudson/Util.java b/core/src/main/java/hudson/Util.java index ce381d7a0a3a..2f7ea8fcc65d 100644 --- a/core/src/main/java/hudson/Util.java +++ b/core/src/main/java/hudson/Util.java @@ -1403,7 +1403,7 @@ public static File resolveSymlinkToFile(@Nonnull File link) throws InterruptedEx * The relative path is meant to be resolved from the location of the symlink. */ @CheckForNull - public static String resolveSymlink(@Nonnull File link) throws InterruptedException, IOException { + public static String resolveSymlink(@Nonnull File link) throws IOException { try { Path path = link.toPath(); return Files.readSymbolicLink(path).toString(); @@ -1415,7 +1415,7 @@ public static String resolveSymlink(@Nonnull File link) throws InterruptedExcept } catch (IOException x) { throw x; } catch (Exception x) { - throw (IOException) new IOException(x.toString()).initCause(x); + throw new IOException(x); } } diff --git a/core/src/main/java/jenkins/util/VirtualFile.java b/core/src/main/java/jenkins/util/VirtualFile.java index 6cf765f3981e..434fefd82710 100644 --- a/core/src/main/java/jenkins/util/VirtualFile.java +++ b/core/src/main/java/jenkins/util/VirtualFile.java @@ -139,8 +139,20 @@ public abstract class VirtualFile implements Comparable, Serializab */ public abstract boolean isFile() throws IOException; + /** + * 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 + */ + public @CheckForNull String readLink() throws IOException { + return null; + } + /** * 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 */ @@ -378,6 +390,12 @@ private static final class FileVF extends VirtualFile { } 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 VirtualFile[] list() throws IOException { if (isIllegalSymlink()) { return new VirtualFile[0]; @@ -497,6 +515,13 @@ private static final class FilePathVF extends VirtualFile { 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(); diff --git a/core/src/test/java/jenkins/util/VirtualFileTest.java b/core/src/test/java/jenkins/util/VirtualFileTest.java index 8be31af5a71c..d704cf8a8c97 100644 --- a/core/src/test/java/jenkins/util/VirtualFileTest.java +++ b/core/src/test/java/jenkins/util/VirtualFileTest.java @@ -175,4 +175,21 @@ public InputStream open() throws IOException { } } + @Issue("JENKINS-26810") + @Test public void readLink() throws Exception { + assumeFalse("Symlinks do not work well on Windows", Functions.isWindows()); + File root = tmp.getRoot(); + FilePath rootF = new FilePath(root); + rootF.child("plain").write("", null); + rootF.child("link").symlinkTo("physical", TaskListener.NULL); + for (VirtualFile vf : new VirtualFile[] {VirtualFile.forFile(root), VirtualFile.forFilePath(rootF)}) { + assertNull(vf.readLink()); + assertNull(vf.child("plain").readLink()); + VirtualFile link = vf.child("link"); + assertEquals("physical", link.readLink()); + assertFalse(link.isFile()); + assertFalse(link.isDirectory()); + } + } + } From 2e070ca8f2125c42e26243866eb4eebc30383b41 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 21 Feb 2018 15:56:22 -0500 Subject: [PATCH 06/20] Forgotten since tags. --- core/src/main/java/jenkins/util/VirtualFile.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/src/main/java/jenkins/util/VirtualFile.java b/core/src/main/java/jenkins/util/VirtualFile.java index 434fefd82710..46c44ff9a874 100644 --- a/core/src/main/java/jenkins/util/VirtualFile.java +++ b/core/src/main/java/jenkins/util/VirtualFile.java @@ -145,6 +145,7 @@ public abstract class VirtualFile implements Comparable, Serializab * 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 FIXME */ public @CheckForNull String readLink() throws IOException { return null; @@ -183,6 +184,7 @@ public abstract class VirtualFile implements Comparable, Serializab * @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 + * @since FIXME */ public @Nonnull Collection list(@Nonnull String includes, @CheckForNull String excludes, boolean useDefaultExcludes) throws IOException { List r = new ArrayList<>(); @@ -246,6 +248,7 @@ private List patterns(String patts) { * Gets the file’s Unix mode, if meaningful. * @return for example, 0644 ~ {@code rw-r--r--}; -1 by default, meaning unknown or inapplicable * @throws IOException if checking the mode failed + * @since FIXME */ public int mode() throws IOException { return -1; From 9735a349c2321d4ffe99afdec15a5819b6f4f3f2 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 21 Feb 2018 15:58:45 -0500 Subject: [PATCH 07/20] Compilation error. --- .../src/main/java/hudson/util/DirScanner.java | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/hudson/util/DirScanner.java b/core/src/main/java/hudson/util/DirScanner.java index a694c7c21b82..2551aa0b081a 100644 --- a/core/src/main/java/hudson/util/DirScanner.java +++ b/core/src/main/java/hudson/util/DirScanner.java @@ -7,7 +7,6 @@ import java.io.File; import java.io.FileFilter; import java.io.IOException; -import java.io.InterruptedIOException; import java.io.Serializable; import static hudson.Util.fixEmpty; @@ -31,19 +30,15 @@ public abstract class DirScanner implements Serializable { */ protected final void scanSingle(File f, String relative, FileVisitor visitor) throws IOException { if (visitor.understandsSymlink()) { + String target; try { - String target; - try { - target = Util.resolveSymlink(f); - } catch (IOException x) { // JENKINS-13202 - target = null; - } - if (target != null) { - visitor.visitSymlink(f, target, relative); - return; - } - } catch (InterruptedException e) { - throw (IOException) new InterruptedIOException().initCause(e); + target = Util.resolveSymlink(f); + } catch (IOException x) { // JENKINS-13202 + target = null; + } + if (target != null) { + visitor.visitSymlink(f, target, relative); + return; } } visitor.visit(f, relative); From a9430d34fd576051c3f24373b6eb3a641ad09e33 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 21 Feb 2018 17:57:37 -0500 Subject: [PATCH 08/20] Mistake caught by DirectoryBrowserSupportTest.zipDownload. --- core/src/main/java/hudson/model/DirectoryBrowserSupport.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java index 3baa9190f596..f5f1080373f9 100644 --- a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java +++ b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java @@ -366,7 +366,7 @@ private static String createBackRef(int times) { private static void zip(OutputStream outputStream, VirtualFile dir, String glob) throws IOException { try (ZipOutputStream zos = new ZipOutputStream(outputStream)) { zos.setEncoding(System.getProperty("file.encoding")); // TODO JENKINS-20663 make this overridable via query parameter - for (String n : dir.list(glob, null, /* TODO what is the user expectation? */true)) { + for (String n : dir.list(glob.isEmpty() ? "**" : glob, null, /* TODO what is the user expectation? */true)) { String relativePath; if (glob.length() == 0) { // JENKINS-19947: traditional behavior is to prepend the directory name From e0fab1a6a2ac0152a794d51edf5d4f3ffd548504 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 21 Feb 2018 17:59:52 -0500 Subject: [PATCH 09/20] list(String, String, boolean) was returning \-separated paths on Windows, contrary to Javadoc. --- core/src/main/java/jenkins/util/VirtualFile.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/jenkins/util/VirtualFile.java b/core/src/main/java/jenkins/util/VirtualFile.java index 46c44ff9a874..e62ae56eed19 100644 --- a/core/src/main/java/jenkins/util/VirtualFile.java +++ b/core/src/main/java/jenkins/util/VirtualFile.java @@ -606,7 +606,7 @@ private static final class Scanner extends MasterToSlaveFileCallable Date: Wed, 21 Feb 2018 18:18:05 -0500 Subject: [PATCH 10/20] SelectorUtils.tokenizePath apparently expects the native path separator, which is normally handled deep within DirectoryScanner. --- core/src/main/java/jenkins/util/VirtualFile.java | 6 +++--- core/src/test/java/jenkins/util/VirtualFileTest.java | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/jenkins/util/VirtualFile.java b/core/src/main/java/jenkins/util/VirtualFile.java index e62ae56eed19..dcd89669082a 100644 --- a/core/src/main/java/jenkins/util/VirtualFile.java +++ b/core/src/main/java/jenkins/util/VirtualFile.java @@ -193,11 +193,11 @@ public abstract class VirtualFile implements Comparable, Serializab List excludePatterns = patterns(excludes); if (useDefaultExcludes) { for (String patt : DirectoryScanner.getDefaultExcludes()) { - excludePatterns.add(new TokenizedPattern(patt)); + excludePatterns.add(new TokenizedPattern(patt.replace('/', File.separatorChar))); } } return r.stream().filter(p -> { - TokenizedPath path = new TokenizedPath(p); + TokenizedPath path = new TokenizedPath(p.replace('/', File.separatorChar)); return includePatterns.stream().anyMatch(patt -> patt.matchPath(path, true)) && !excludePatterns.stream().anyMatch(patt -> patt.matchPath(path, true)); }).collect(Collectors.toSet()); } @@ -217,7 +217,7 @@ private List patterns(String patts) { if (patt.endsWith("/")) { patt += SelectorUtils.DEEP_TREE_MATCH; } - r.add(new TokenizedPattern(patt)); + r.add(new TokenizedPattern(patt.replace('/', File.separatorChar))); } } return r; diff --git a/core/src/test/java/jenkins/util/VirtualFileTest.java b/core/src/test/java/jenkins/util/VirtualFileTest.java index d704cf8a8c97..c8eeca0f55e2 100644 --- a/core/src/test/java/jenkins/util/VirtualFileTest.java +++ b/core/src/test/java/jenkins/util/VirtualFileTest.java @@ -104,6 +104,7 @@ private static String modeString(int mode) throws IOException { rootF.child(path).write("", null); } for (VirtualFile vf : new VirtualFile[] {VirtualFile.forFile(root), VirtualFile.forFilePath(rootF), new Ram(paths.stream().map(p -> "/" + p).collect(Collectors.toSet()), "")}) { + 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()); From 7131067f771cf110b8c6b219406c4099fc74bc8d Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 21 Feb 2018 18:40:50 -0500 Subject: [PATCH 11/20] Minor comments and simplifications. --- core/src/test/java/jenkins/util/VirtualFileTest.java | 6 ++++-- .../test/java/hudson/model/DirectoryBrowserSupportTest.java | 5 +++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/core/src/test/java/jenkins/util/VirtualFileTest.java b/core/src/test/java/jenkins/util/VirtualFileTest.java index c8eeca0f55e2..8bab74655734 100644 --- a/core/src/test/java/jenkins/util/VirtualFileTest.java +++ b/core/src/test/java/jenkins/util/VirtualFileTest.java @@ -87,7 +87,7 @@ public class VirtualFileTest { VirtualFile vfp = VirtualFile.forFilePath(fp); assertEquals(modeString(hudson.util.IOUtils.mode(f)), modeString(vf.mode())); assertEquals(modeString(hudson.util.IOUtils.mode(f)), modeString(vfp.mode())); - fp.chmod(0755); + fp.chmod(0755); // no-op on Windows, but harmless assertEquals(modeString(hudson.util.IOUtils.mode(f)), modeString(vf.mode())); assertEquals(modeString(hudson.util.IOUtils.mode(f)), modeString(vfp.mode())); } @@ -119,6 +119,7 @@ private static String modeString(int mode) throws IOException { assertEquals("[sub/subsub/lowest.txt, top.txt, very/deep/path/here]", new TreeSet<>(vf.list("**", "**/mid*,**/conf*", false)).toString()); } } + /** Roughly analogous to {@code org.jenkinsci.plugins.compress_artifacts.ZipStorage}. */ private static final class Ram extends VirtualFile { private final Set paths; // e.g., [/very/deep/path/here] private final String path; // e.g., empty string or /very or /very/deep/path/here @@ -152,7 +153,7 @@ public boolean exists() throws IOException { } @Override public VirtualFile[] list() throws IOException { - return paths.stream().filter(p -> p.startsWith(path + "/")).map(p -> new Ram(paths, p.replaceFirst("(\\Q" + path + "\\E/[^/]+)/.+", "$1"))).collect(Collectors.toSet()).toArray(new VirtualFile[0]); + return paths.stream().filter(p -> p.startsWith(path + "/")).map(p -> new Ram(paths, p.replaceFirst("(\\Q" + path + "\\E/[^/]+)/.+", "$1"))).toArray(VirtualFile[]::new); } @Override public VirtualFile child(String name) { @@ -190,6 +191,7 @@ public InputStream open() throws IOException { assertEquals("physical", link.readLink()); assertFalse(link.isFile()); assertFalse(link.isDirectory()); + // not checking .exists() for now } } diff --git a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java index 8375fcce416c..a578e26aa3e6 100644 --- a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java +++ b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java @@ -251,6 +251,7 @@ public void externalURLDownload() throws Exception { throw x; } } + /** Simulation of a storage service with URLs unrelated to {@link Run#doArtifact}. */ @TestExtension("externalURLDownload") public static final class ContentAddressableStore implements UnprotectedRootAction { final List files = new ArrayList<>(); @@ -301,7 +302,7 @@ public void archive(FilePath workspace, Launcher launcher, BuildListener listene } @Override public VirtualFile root() { - final VirtualFile file = new VirtualFile() { + final VirtualFile file = new VirtualFile() { // the file inside the root @Override public String getName() { return "f"; @@ -359,7 +360,7 @@ public URL toExternalURL() throws IOException { return new URL(Jenkins.get().getRootUrl() + "files/" + hash); } }; - return new VirtualFile() { + return new VirtualFile() { // the root @Override public String getName() { return ""; From 63885e664d6d953364db2229e6152cd58aedde37 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 5 Mar 2018 10:03:07 -0500 Subject: [PATCH 12/20] Typo noticed by @olivergondza. --- core/src/main/java/jenkins/util/VirtualFile.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/jenkins/util/VirtualFile.java b/core/src/main/java/jenkins/util/VirtualFile.java index dcd89669082a..96baa4f946d1 100644 --- a/core/src/main/java/jenkins/util/VirtualFile.java +++ b/core/src/main/java/jenkins/util/VirtualFile.java @@ -87,7 +87,7 @@ *

Since some implementations may in fact use external file storage, * callers may request optional APIs to access those services more efficiently. * Otherwise, for example, a plugin copying a file - * previously saved by {@link ArtifactManager} to an external storage servuce + * previously saved by {@link ArtifactManager} to an external storage service * which tunneled a stream from {@link #open} using {@link RemoteInputStream} * would wind up transferring the file from the service to the Jenkins master and then on to an agent. * Similarly, if {@link DirectoryBrowserSupport} rendered a link to an in-Jenkins URL, From d32b9b1d167643714146dd7bf48c058216274158 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 14 Mar 2018 14:08:20 -0400 Subject: [PATCH 13/20] @oleg-nenashev suggests documenting behavior of VirtualFile.mode on symlinks. --- core/src/main/java/hudson/util/IOUtils.java | 2 +- core/src/main/java/jenkins/util/VirtualFile.java | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/hudson/util/IOUtils.java b/core/src/main/java/hudson/util/IOUtils.java index 5022dc021f7f..2bcdf90acbdc 100644 --- a/core/src/main/java/hudson/util/IOUtils.java +++ b/core/src/main/java/hudson/util/IOUtils.java @@ -117,7 +117,7 @@ public static boolean isAbsolute(String path) { * execute permissions for the owner, group, and others, i.e. the max return value * is 0777. Consider using {@link Files#getPosixFilePermissions} instead if you only * care about access permissions. - * + *

If the file is symlink, the mode is that of the link target, not the link itself. * @return a file mode, or -1 if not on Unix * @throws PosixException if the file could not be statted, e.g. broken symlink */ diff --git a/core/src/main/java/jenkins/util/VirtualFile.java b/core/src/main/java/jenkins/util/VirtualFile.java index 96baa4f946d1..278c61acd8e9 100644 --- a/core/src/main/java/jenkins/util/VirtualFile.java +++ b/core/src/main/java/jenkins/util/VirtualFile.java @@ -246,6 +246,7 @@ private List patterns(String patts) { /** * 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 FIXME From beea2aaf2142fa861266f9bfa33d08a8710c3fb1 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 14 Mar 2018 16:32:02 -0400 Subject: [PATCH 14/20] More details on toURI and toExternalURL. --- core/src/main/java/jenkins/util/VirtualFile.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/jenkins/util/VirtualFile.java b/core/src/main/java/jenkins/util/VirtualFile.java index 278c61acd8e9..6a88e24b4741 100644 --- a/core/src/main/java/jenkins/util/VirtualFile.java +++ b/core/src/main/java/jenkins/util/VirtualFile.java @@ -113,10 +113,11 @@ public abstract class VirtualFile implements Comparable, Serializab /** * Gets a URI. * Should at least uniquely identify this virtual file within its root, but not necessarily globally. - *

When {@link #toExternalURL} is implemented, it is natural but not required to use that same value here. + *

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 URI toURI(); + public abstract @Nonnull URI toURI(); /** * Gets the parent file. @@ -340,6 +341,8 @@ public V run(Callable callable) throws IOException { *

Any necessary authentication must be encoded somehow into the URL itself; * do not include any tokens or other authentication which might allow access to unrelated files * (for example {@link ArtifactManager} builds from a different job). + * 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. *

Generally this will be harder to implement than {@link #asRemotable}, * which would have the opportunity to perform arbitrary preparation for {@link #open} * such as negotiating session authentication. From 7223632a06a8679b86c862400accdf5bdb83aff7 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 16 Mar 2018 17:01:02 -0400 Subject: [PATCH 15/20] Should not attempt to call VirtualFile.child with the empty string. --- core/src/main/java/hudson/model/DirectoryBrowserSupport.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java index f5f1080373f9..5b32d5b755dd 100644 --- a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java +++ b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java @@ -216,7 +216,7 @@ private void serveFile(StaplerRequest req, StaplerResponse rsp, VirtualFile root String rest = _rest.toString(); // this is the base file/directory - VirtualFile baseFile = root.child(base); + VirtualFile baseFile = base.isEmpty() ? root : root.child(base); if(baseFile.isDirectory()) { if(zip) { From 90fd89ff67e881e5efbc7085defbc0be674a92c8 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 23 Mar 2018 16:47:53 -0400 Subject: [PATCH 16/20] Use run(Callable) from the default list(String, String, boolean) so it can run efficiently without an override. --- .../main/java/jenkins/util/VirtualFile.java | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/jenkins/util/VirtualFile.java b/core/src/main/java/jenkins/util/VirtualFile.java index 6a88e24b4741..77d56157877e 100644 --- a/core/src/main/java/jenkins/util/VirtualFile.java +++ b/core/src/main/java/jenkins/util/VirtualFile.java @@ -56,6 +56,7 @@ import javax.annotation.Nonnull; import jenkins.MasterToSlaveFileCallable; import jenkins.model.ArtifactManager; +import jenkins.security.MasterToSlaveCallable; import org.apache.tools.ant.DirectoryScanner; import org.apache.tools.ant.types.AbstractFileSet; import org.apache.tools.ant.types.selectors.SelectorUtils; @@ -177,7 +178,7 @@ public abstract class VirtualFile implements Comparable, Serializab /** * Lists recursive files of this directory with pattern matching. - *

The default implementation calls {@link #list()} recursively and applies filtering to the result. + *

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) @@ -188,8 +189,7 @@ public abstract class VirtualFile implements Comparable, Serializab * @since FIXME */ public @Nonnull Collection list(@Nonnull String includes, @CheckForNull String excludes, boolean useDefaultExcludes) throws IOException { - List r = new ArrayList<>(); - collectFiles(r, ""); + Collection r = run(new CollectFiles(this)); List includePatterns = patterns(includes); List excludePatterns = patterns(excludes); if (useDefaultExcludes) { @@ -202,12 +202,25 @@ public abstract class VirtualFile implements Comparable, Serializab return includePatterns.stream().anyMatch(patt -> patt.matchPath(path, true)) && !excludePatterns.stream().anyMatch(patt -> patt.matchPath(path, true)); }).collect(Collectors.toSet()); } - private void collectFiles(Collection names, String prefix) throws IOException { - for (VirtualFile child : list()) { - if (child.isFile()) { - names.add(prefix + child.getName()); - } else if (child.isDirectory()) { - child.collectFiles(names, prefix + child.getName() + "/"); + private static final class CollectFiles extends MasterToSlaveCallable, IOException> { + private static final long serialVersionUID = 1; + private final VirtualFile root; + CollectFiles(VirtualFile root) { + this.root = root; + } + @Override + public Collection call() throws IOException { + List r = new ArrayList<>(); + collectFiles(root, r, ""); + return r; + } + 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() + "/"); + } } } } From 9b75cd28f6db3b057cc8da5c8d85bf1cac3375ae Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 23 Mar 2018 18:09:42 -0400 Subject: [PATCH 17/20] Making Run.getArtifactsUpTo call VirtualFile.run. --- .../hudson/model/DirectoryBrowserSupport.java | 1 + core/src/main/java/hudson/model/Run.java | 89 ++++++++++++++++--- 2 files changed, 79 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java index 5b32d5b755dd..504eaba0d17d 100644 --- a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java +++ b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java @@ -366,6 +366,7 @@ private static String createBackRef(int times) { private static void zip(OutputStream outputStream, VirtualFile dir, String glob) throws IOException { try (ZipOutputStream zos = new ZipOutputStream(outputStream)) { zos.setEncoding(System.getProperty("file.encoding")); // TODO JENKINS-20663 make this overridable via query parameter + // TODO consider using run(Callable) here for (String n : dir.list(glob.isEmpty() ? "**" : glob, null, /* TODO what is the user expectation? */true)) { String relativePath; if (glob.length() == 0) { diff --git a/core/src/main/java/hudson/model/Run.java b/core/src/main/java/hudson/model/Run.java index f7e11b1c657f..879a5bd402e2 100644 --- a/core/src/main/java/hudson/model/Run.java +++ b/core/src/main/java/hudson/model/Run.java @@ -73,6 +73,7 @@ import java.io.PrintWriter; import java.io.RandomAccessFile; import java.io.Reader; +import java.io.Serializable; import java.nio.charset.Charset; import java.text.DateFormat; import java.text.SimpleDateFormat; @@ -108,6 +109,7 @@ import jenkins.model.StandardArtifactManager; import jenkins.model.lazy.BuildReference; import jenkins.model.lazy.LazyBuildMixIn; +import jenkins.security.MasterToSlaveCallable; import jenkins.util.VirtualFile; import jenkins.util.io.OnMaster; import net.sf.json.JSONObject; @@ -1090,12 +1092,16 @@ public File getArtifactsDir() { * @return The list can be empty but never null */ public @Nonnull List getArtifactsUpTo(int artifactsNumber) { - ArtifactList r = new ArtifactList(); + SerializableArtifactList sal; + VirtualFile root = getArtifactManager().root(); try { - addArtifacts(getArtifactManager().root(), "", "", r, null, artifactsNumber); + sal = root.run(new AddArtifacts(root, artifactsNumber)); } catch (IOException x) { LOGGER.log(Level.WARNING, null, x); + sal = new SerializableArtifactList(); } + ArtifactList r = new ArtifactList(); + r.updateFrom(sal); r.computeDisplayName(); return r; } @@ -1109,9 +1115,25 @@ public boolean getHasArtifacts() { return !getArtifactsUpTo(1).isEmpty(); } - private int addArtifacts(@Nonnull VirtualFile dir, + private static final class AddArtifacts extends MasterToSlaveCallable { + private static final long serialVersionUID = 1L; + private final VirtualFile root; + private final int artifactsNumber; + AddArtifacts(VirtualFile root, int artifactsNumber) { + this.root = root; + this.artifactsNumber = artifactsNumber; + } + @Override + public SerializableArtifactList call() throws IOException { + SerializableArtifactList sal = new SerializableArtifactList(); + addArtifacts(root, "", "", sal, null, artifactsNumber); + return sal; + } + } + + private static int addArtifacts(@Nonnull VirtualFile dir, @Nonnull String path, @Nonnull String pathHref, - @Nonnull ArtifactList r, @Nonnull Artifact parent, int upTo) throws IOException { + @Nonnull SerializableArtifactList r, @Nonnull SerializableArtifact parent, int upTo) throws IOException { VirtualFile[] kids = dir.list(); Arrays.sort(kids); @@ -1122,26 +1144,26 @@ private int addArtifacts(@Nonnull VirtualFile dir, String childHref = pathHref + Util.rawEncode(child); String length = sub.isFile() ? String.valueOf(sub.length()) : ""; boolean collapsed = (kids.length==1 && parent!=null); - Artifact a; + SerializableArtifact a; if (collapsed) { // Collapse single items into parent node where possible: - a = new Artifact(parent.getFileName() + '/' + child, childPath, + a = new SerializableArtifact(parent.name + '/' + child, childPath, sub.isDirectory() ? null : childHref, length, - parent.getTreeNodeId()); + parent.treeNodeId); r.tree.put(a, r.tree.remove(parent)); } else { // Use null href for a directory: - a = new Artifact(child, childPath, + a = new SerializableArtifact(child, childPath, sub.isDirectory() ? null : childHref, length, "n" + ++r.idSeq); - r.tree.put(a, parent!=null ? parent.getTreeNodeId() : null); + r.tree.put(a, parent!=null ? parent.treeNodeId : null); } if (sub.isDirectory()) { n += addArtifacts(sub, childPath + '/', childHref + '/', r, a, upTo-n); if (n>=upTo) break; } else { // Don't store collapsed path in ArrayList (for correct data in external API) - r.add(collapsed ? new Artifact(child, a.relativePath, a.href, length, a.treeNodeId) : a); + r.add(collapsed ? new SerializableArtifact(child, a.relativePath, a.href, length, a.treeNodeId) : a); if (++n>=upTo) break; } } @@ -1159,6 +1181,30 @@ private int addArtifacts(@Nonnull VirtualFile dir, public static final int TREE_CUTOFF = Integer.parseInt(SystemProperties.getString("hudson.model.Run.ArtifactList.treeCutoff", "40")); // ..and then "too many" + + /** {@link Run.Artifact} without the implicit link to {@link Run} */ + private static final class SerializableArtifact implements Serializable { + private static final long serialVersionUID = 1L; + final String name; + final String relativePath; + final String href; + final String length; + final String treeNodeId; + SerializableArtifact(String name, String relativePath, String href, String length, String treeNodeId) { + this.name = name; + this.relativePath = relativePath; + this.href = href; + this.length = length; + this.treeNodeId = treeNodeId; + } + } + + /** {@link Run.ArtifactList} without the implicit link to {@link Run} */ + private static final class SerializableArtifactList extends ArrayList { + private static final long serialVersionUID = 1L; + private LinkedHashMap tree = new LinkedHashMap<>(); + private int idSeq = 0; + } public final class ArtifactList extends ArrayList { private static final long serialVersionUID = 1L; @@ -1167,7 +1213,24 @@ public final class ArtifactList extends ArrayList { * Contains Artifact objects for directories and files (the ArrayList contains only files). */ private LinkedHashMap tree = new LinkedHashMap(); - private int idSeq = 0; + + void updateFrom(SerializableArtifactList clone) { + Map artifacts = new HashMap<>(); // need to share objects between tree and list, since computeDisplayName mutates displayPath + for (SerializableArtifact sa : clone) { + Artifact a = new Artifact(sa); + artifacts.put(a.relativePath, a); + add(a); + } + tree = new LinkedHashMap<>(); + for (Map.Entry entry : clone.tree.entrySet()) { + SerializableArtifact sa = entry.getKey(); + Artifact a = artifacts.get(sa.relativePath); + if (a == null) { + a = new Artifact(sa); + } + tree.put(a, entry.getValue()); + } + } public Map getTree() { return tree; @@ -1282,6 +1345,10 @@ public class Artifact { */ private String length; + Artifact(SerializableArtifact clone) { + this(clone.name, clone.relativePath, clone.href, clone.length, clone.treeNodeId); + } + /*package for test*/ Artifact(String name, String relativePath, String href, String len, String treeNodeId) { this.name = name; this.relativePath = relativePath; From 2090468d82e49345519a2457f1d1e7426f01540b Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 26 Mar 2018 18:37:45 -0400 Subject: [PATCH 18/20] Using new beta annotation. --- core/src/main/java/jenkins/util/VirtualFile.java | 7 +++++++ pom.xml | 7 ++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/jenkins/util/VirtualFile.java b/core/src/main/java/jenkins/util/VirtualFile.java index 77d56157877e..9427bbd85e14 100644 --- a/core/src/main/java/jenkins/util/VirtualFile.java +++ b/core/src/main/java/jenkins/util/VirtualFile.java @@ -62,6 +62,8 @@ import org.apache.tools.ant.types.selectors.SelectorUtils; import org.apache.tools.ant.types.selectors.TokenizedPath; import org.apache.tools.ant.types.selectors.TokenizedPattern; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.Beta; /** * Abstraction over {@link File}, {@link FilePath}, or other items such as network resources or ZIP entries. @@ -149,6 +151,7 @@ public abstract class VirtualFile implements Comparable, Serializab * @throws IOException if reading the link, or even determining whether this file is a link, failed * @since FIXME */ + @Restricted(Beta.class) public @CheckForNull String readLink() throws IOException { return null; } @@ -188,6 +191,7 @@ public abstract class VirtualFile implements Comparable, Serializab * @throws IOException if this is not a directory, or listing was not possible for some other reason * @since FIXME */ + @Restricted(Beta.class) public @Nonnull Collection list(@Nonnull String includes, @CheckForNull String excludes, boolean useDefaultExcludes) throws IOException { Collection r = run(new CollectFiles(this)); List includePatterns = patterns(includes); @@ -265,6 +269,7 @@ private List patterns(String patts) { * @throws IOException if checking the mode failed * @since FIXME */ + @Restricted(Beta.class) public int mode() throws IOException { return -1; } @@ -342,6 +347,7 @@ public V run(Callable callable) throws IOException { * @since FIXME * @see #toExternalURL */ + @Restricted(Beta.class) public @CheckForNull VirtualFile asRemotable() { return null; } @@ -364,6 +370,7 @@ public V run(Callable callable) throws IOException { * @see #toURI * @see #asRemotable */ + @Restricted(Beta.class) public @CheckForNull URL toExternalURL() throws IOException { return null; } diff --git a/pom.xml b/pom.xml index 5545718723ad..d468f98f4e34 100644 --- a/pom.xml +++ b/pom.xml @@ -93,9 +93,10 @@ THE SOFTWARE. 1.4.1 0.11 ${skipTests} - 1.13 - ${access-modifier.version} - ${access-modifier.version} + 1.14-SNAPSHOT + + 1.14-20180326.221427-2 + 1.14-20180326.221430-2 8 From 547fd6fe6ef2823d04a1517fb08f6d714ff4423a Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 2 Apr 2018 15:55:40 -0400 Subject: [PATCH 19/20] Removing VirtualFile.asRemotable in favor of toExternalURL. --- core/src/main/java/hudson/FilePath.java | 23 +++++++++++++++ .../main/java/jenkins/util/VirtualFile.java | 28 ++----------------- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java index 2f21b2cd5d50..49f6037655a7 100644 --- a/core/src/main/java/hudson/FilePath.java +++ b/core/src/main/java/hudson/FilePath.java @@ -139,6 +139,7 @@ import java.util.Collections; import org.apache.tools.ant.BuildException; +import org.kohsuke.accmod.restrictions.Beta; /** * {@link File} like object with remoting support. @@ -903,6 +904,28 @@ public void copyFrom(URL url) throws IOException, InterruptedException { } } + /** + * Copies the content of a URL to a remote file. + * Unlike {@link #copyFrom} this will not transfer content over a Remoting channel. + * @since FIXME + */ + @Restricted(Beta.class) + public void copyFromRemotely(URL url) throws IOException, InterruptedException { + act(new CopyFromRemotely(url)); + } + private final class CopyFromRemotely extends MasterToSlaveFileCallable { + private static final long serialVersionUID = 1; + private final URL url; + CopyFromRemotely(URL url) { + this.url = url; + } + @Override + public Void invoke(File f, VirtualChannel channel) throws IOException, InterruptedException { + copyFrom(url); + return null; + } + } + /** * Replaces the content of this file by the data from the given {@link InputStream}. * diff --git a/core/src/main/java/jenkins/util/VirtualFile.java b/core/src/main/java/jenkins/util/VirtualFile.java index 9427bbd85e14..e59ad888bd66 100644 --- a/core/src/main/java/jenkins/util/VirtualFile.java +++ b/core/src/main/java/jenkins/util/VirtualFile.java @@ -95,8 +95,7 @@ * would wind up transferring the file from the service to the Jenkins master and then on to an agent. * Similarly, if {@link DirectoryBrowserSupport} rendered a link to an in-Jenkins URL, * a large file could be transferred from the service to the Jenkins master and then on to the browser. - * To avoid this overhead, callers may check whether an implementation - * supports {@link #asRemotable} and/or {@link #toExternalURL}. + * To avoid this overhead, callers may check whether an implementation supports {@link #toExternalURL}. * * @see DirectoryBrowserSupport * @see FilePath @@ -334,24 +333,6 @@ public V run(Callable callable) throws IOException { return callable.call(); } - /** - * Optionally produces a variant of this handle which may be safely passed over a Remoting {@link Channel}. - * This would allow remote nodes such as agents to make calls such as {@link #open} - * and be assured of the most efficient possible access. - * Otherwise, all calls must be made on the node originally producing this object, - * and the caller must arrange for transport of the result. - *

Note that the result of {@link #forFilePath} does not implement this method, - * since a {@link FilePath} may only be transferred over the channel on which it was created. - * It cannot, for example, be used to represent a workspace file from one agent on another agent. - * @return this object or a variant which may be passed over a {@link Channel}, or null if there is no such support - * @since FIXME - * @see #toExternalURL - */ - @Restricted(Beta.class) - public @CheckForNull VirtualFile asRemotable() { - return null; - } - /** * Optionally obtains a URL which may be used to retrieve file contents from any process on any node. * For example, given cloud storage this might produce a permalink to the file. @@ -360,15 +341,12 @@ public V run(Callable callable) throws IOException { *

Any necessary authentication must be encoded somehow into the URL itself; * do not include any tokens or other authentication which might allow access to unrelated files * (for example {@link ArtifactManager} builds from a different job). - * The URL might be valid for only a limited amount of time or even only a single use; + * 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. - *

Generally this will be harder to implement than {@link #asRemotable}, - * which would have the opportunity to perform arbitrary preparation for {@link #open} - * such as negotiating session authentication. * @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 FIXME * @see #toURI - * @see #asRemotable */ @Restricted(Beta.class) public @CheckForNull URL toExternalURL() throws IOException { From a54d49b634e2cdd2fc11ab0021a1865630fc339e Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 3 Apr 2018 11:29:16 -0400 Subject: [PATCH 20/20] access-modifier.version=1.14 --- pom.xml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pom.xml b/pom.xml index ac3afe14894e..722c99acb5f4 100644 --- a/pom.xml +++ b/pom.xml @@ -93,10 +93,9 @@ THE SOFTWARE. 1.4.1 0.11 ${skipTests} - 1.14-SNAPSHOT - - 1.14-20180326.221427-2 - 1.14-20180326.221430-2 + 1.14 + ${access-modifier.version} + ${access-modifier.version} 8