Not fully solved, since the scannedDirs field can still grow to be large, but at least clearing files/dirsNotIncluded.
Also imposing a 5s timeout on the scan regardless of file count, and defining a user-customizable bound.
This code duplication in two sides of the if statement is undesirable.
Post-release, I need to come back here to introduce a variant of FileCallable that runs on the given FilePath but with InputStream to another FilePath.
Still do it so we capture FilePath.remote (which many JRE methods suppress),
but fix the wrapper Throwable.message to include cause.toString() according to convention.
If we ever decide to develop a compatibility thunk library for these changes to help promote deployment in plugins,
then these classes being a nested type is problematic as they'll fail to load under onld Jenkins
If a @SlaveToMaster Callable gets sent to the master, and if inside its call() method the code directly/indirectly performs file system access (for example on files and directories that it didn't receive as parameters), that is not meant to be the subject of the filter check.
Rather, the check is meant for FilePath that the other side sends to us, when it points to a local file.
So it makes more sense to compute the effective FilePathFilter during the deserialization, as opposed to doing so when the actual operation is attempted.
Various FileCallable impls in FilePath fits this pattern, as it drags outer FilePath instance with itself. In addition, this change addresses other situations correctly, where FilePath is used as arguments and return values of other Callables, such as the following:
FilePath f = channel.call(new SlaveWillDoSomeComputationAndGiveMeTheResultAsFile());
f.copyTo(build.getRootDir().child("data.xml"))
"f.copyTo" happens in the context of Channel.current()==null, but we need to check that 'f' is legit and not something like
new FilePath(new File("/etc/shadow"))
- If a write results in a file creation, do the create check, to align with the POSIX semantics.
- Don't make pointless mkdirs check if the directory already exists
- read vs readSymlink difference is dangerous because read() can be called for symlinks in other context as well.
java.lang.NullPointerException: null
at hudson.model.AbstractBuild.setWorkspace(AbstractBuild.java:296)
at hudson.model.ProjectTest.testCheckout(ProjectTest.java:423)
(Util.createSymlink is partly to blame for not throwing a descriptive IOException when it fails to create a link,
but this is its historical behavior needed for compatibility.
copyRecursiveTo also continues to suppress error text from this method because there is nowhere for it to go.
Perhaps need a new variant of createSymlink that is guaranteed to either have created the link or throw an exception.)
Oleg Nenashev