diff --git a/changelog.html b/changelog.html index dc29064468b6..5ea64e0a3cac 100644 --- a/changelog.html +++ b/changelog.html @@ -55,6 +55,9 @@ diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java index 0cc9403ff0e9..ec596ea87857 100644 --- a/core/src/main/java/hudson/FilePath.java +++ b/core/src/main/java/hudson/FilePath.java @@ -830,7 +830,7 @@ private T act(final FileCallable callable, ClassLoader cl) throws IOExcep try { return channel.call(new FileCallableWrapper(callable,cl)); } catch (TunneledInterruptedException e) { - throw (InterruptedException)new InterruptedException().initCause(e); + throw (InterruptedException)new InterruptedException(e.getMessage()).initCause(e); } catch (AbortException e) { throw e; // pass through so that the caller can catch it as AbortException } catch (IOException e) { @@ -1903,8 +1903,26 @@ public Boolean call() throws IOException { * @see #validateFileMask(FilePath, String) */ public String validateAntFileMask(final String fileMasks) throws IOException, InterruptedException { + return validateAntFileMask(fileMasks, Integer.MAX_VALUE); + } + + /** + * Like {@link #validateAntFileMask(String)} but performing only a bounded number of operations. + *

Whereas the unbounded overload is appropriate for calling from cancelable, long-running tasks such as build steps, + * this overload should be used when an answer is needed quickly, such as for {@link #validateFileMask(String)} + * or anything else returning {@link FormValidation}. + *

If a positive match is found, {@code null} is returned immediately. + * A message is returned in case the file pattern can definitely be determined to not match anything in the directory within the alloted time. + * If the time runs out without finding a match but without ruling out the possibility that there might be one, {@link InterruptedException} is thrown, + * in which case the calling code should give the user the benefit of the doubt and use {@link hudson.util.FormValidation.Kind#OK} (with or without a message). + * @param bound a maximum number of negative operations (deliberately left vague) to perform before giving up on a precise answer; 10_000 is a reasonable pick + * @throws InterruptedException not only in case of a channel failure, but also if too many operations were performed without finding any matches + * @since 1.484 + */ + public String validateAntFileMask(final String fileMasks, final int bound) throws IOException, InterruptedException { return act(new FileCallable() { - public String invoke(File dir, VirtualChannel channel) throws IOException { + private static final long serialVersionUID = 1; + public String invoke(File dir, VirtualChannel channel) throws IOException, InterruptedException { if(fileMasks.startsWith("~")) return Messages.FilePath_TildaDoesntWork(); @@ -2001,10 +2019,28 @@ public String invoke(File dir, VirtualChannel channel) throws IOException { return null; // no error } - private boolean hasMatch(File dir, String pattern) { - FileSet fs = Util.createFileSet(dir,pattern); - DirectoryScanner ds = fs.getDirectoryScanner(new Project()); - + private boolean hasMatch(File dir, String pattern) throws InterruptedException { + class Cancel extends RuntimeException {} + DirectoryScanner ds = bound == Integer.MAX_VALUE ? new DirectoryScanner() : new DirectoryScanner() { + int ticks; + @Override public synchronized boolean isCaseSensitive() { + if (!filesIncluded.isEmpty() || !dirsIncluded.isEmpty() || ticks++ > bound) { + throw new Cancel(); + } + return super.isCaseSensitive(); + } + }; + ds.setBasedir(dir); + ds.setIncludes(new String[] {pattern}); + try { + ds.scan(); + } catch (Cancel c) { + if (ds.getIncludedFilesCount()!=0 || ds.getIncludedDirsCount()!=0) { + return true; + } else { + throw new InterruptedException("no matches found within " + bound); + } + } return ds.getIncludedFilesCount()!=0 || ds.getIncludedDirsCount()!=0; } @@ -2053,11 +2089,11 @@ public FormValidation validateFileMask(String value, boolean errorIfNotExist) th if(!exists()) // no workspace. can't check return FormValidation.ok(); - String msg = validateAntFileMask(value); + String msg = validateAntFileMask(value, 10000); if(errorIfNotExist) return FormValidation.error(msg); else return FormValidation.warning(msg); } catch (InterruptedException e) { - return FormValidation.ok(); + return FormValidation.ok(Messages.FilePath_did_not_manage_to_validate_may_be_too_sl(value)); } } diff --git a/core/src/main/java/hudson/util/FormFieldValidator.java b/core/src/main/java/hudson/util/FormFieldValidator.java index 9261588bf86d..e644e028bede 100644 --- a/core/src/main/java/hudson/util/FormFieldValidator.java +++ b/core/src/main/java/hudson/util/FormFieldValidator.java @@ -378,11 +378,11 @@ protected void check() throws IOException, ServletException { return; } - String msg = ws.validateAntFileMask(value); + String msg = ws.validateAntFileMask(value, 10000); if(errorIfNotExist) error(msg); else warning(msg); } catch (InterruptedException e) { - ok(); // coundn't check + ok(Messages.FormFieldValidator_did_not_manage_to_validate_may_be_too_sl(value)); } } diff --git a/core/src/main/resources/hudson/Messages.properties b/core/src/main/resources/hudson/Messages.properties index 38ec4e855eb6..a9fcae1043f5 100644 --- a/core/src/main/resources/hudson/Messages.properties +++ b/core/src/main/resources/hudson/Messages.properties @@ -20,6 +20,7 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN # THE SOFTWARE. +FilePath.did_not_manage_to_validate_may_be_too_sl=Did not manage to validate {0} (may be too slow) FilePath.validateAntFileMask.whitespaceSeprator=\ Whitespace can no longer be used as the separator. Please Use '','' as the separator instead. FilePath.validateAntFileMask.doesntMatchAndSuggest=\ diff --git a/core/src/main/resources/hudson/util/Messages.properties b/core/src/main/resources/hudson/util/Messages.properties index 3a677120185e..5f40d7f1c373 100644 --- a/core/src/main/resources/hudson/util/Messages.properties +++ b/core/src/main/resources/hudson/util/Messages.properties @@ -24,6 +24,7 @@ ClockDifference.InSync=In sync ClockDifference.Ahead=\ ahead ClockDifference.Behind=\ behind ClockDifference.Failed=Failed to check +FormFieldValidator.did_not_manage_to_validate_may_be_too_sl=Did not manage to validate {0} (may be too slow) FormValidation.ValidateRequired=Required FormValidation.Error.Details=(show details) -HttpResponses.Saved=Saved \ No newline at end of file +HttpResponses.Saved=Saved diff --git a/core/src/test/java/hudson/FilePathTest.java b/core/src/test/java/hudson/FilePathTest.java index 8f5d301d4030..4d2a9fab2342 100644 --- a/core/src/test/java/hudson/FilePathTest.java +++ b/core/src/test/java/hudson/FilePathTest.java @@ -424,4 +424,55 @@ public void testMultiSegmentRelativePaths() throws Exception { assertEquals("/opt/jenkins/workspace/foo/bar/manchu", new FilePath(nixPath, "foo/bar\\manchu").getRemote()); assertEquals("/opt/jenkins/workspace/foo/bar/manchu", new FilePath(nixPath, "foo/bar/manchu").getRemote()); } + + public void testValidateAntFileMask() throws Exception { + File tmp = Util.createTempDir(); + try { + FilePath d = new FilePath(french, tmp.getPath()); + d.child("d1/d2/d3").mkdirs(); + d.child("d1/d2/d3/f.txt").touch(0); + d.child("d1/d2/d3/f.html").touch(0); + d.child("d1/d2/f.txt").touch(0); + assertValidateAntFileMask(null, d, "**/*.txt"); + assertValidateAntFileMask(null, d, "d1/d2/d3/f.txt"); + assertValidateAntFileMask(null, d, "**/*.html"); + assertValidateAntFileMask(Messages.FilePath_validateAntFileMask_portionMatchButPreviousNotMatchAndSuggest("**/*.js", "**", "**/*.js"), d, "**/*.js"); + assertValidateAntFileMask(Messages.FilePath_validateAntFileMask_doesntMatchAnything("index.htm"), d, "index.htm"); + assertValidateAntFileMask(Messages.FilePath_validateAntFileMask_doesntMatchAndSuggest("f.html", "d1/d2/d3/f.html"), d, "f.html"); + // XXX lots more to test, e.g. multiple patterns separated by commas; ought to have full code coverage for this method + } finally { + Util.deleteRecursive(tmp); + } + } + + private static void assertValidateAntFileMask(String expected, FilePath d, String fileMasks) throws Exception { + assertEquals(expected, d.validateAntFileMask(fileMasks)); + } + + @Bug(7214) + public void testValidateAntFileMaskBounded() throws Exception { + File tmp = Util.createTempDir(); + try { + FilePath d = new FilePath(french, tmp.getPath()); + FilePath d2 = d.child("d1/d2"); + d2.mkdirs(); + for (int i = 0; i < 100; i++) { + FilePath d3 = d2.child("d" + i); + d3.mkdirs(); + d3.child("f.txt").touch(0); + } + assertEquals(null, d.validateAntFileMask("d1/d2/**/f.txt")); + assertEquals(null, d.validateAntFileMask("d1/d2/**/f.txt", 10)); + assertEquals(Messages.FilePath_validateAntFileMask_portionMatchButPreviousNotMatchAndSuggest("**/*.js", "**", "**/*.js"), d.validateAntFileMask("**/*.js", 1000)); + try { + d.validateAntFileMask("**/*.js", 10); + fail(); + } catch (InterruptedException x) { + // good + } + } finally { + Util.deleteRecursive(tmp); + } + } + }