diff --git a/changelog.html b/changelog.html index 13ee11beba..f268d49ab7 100644 --- a/changelog.html +++ b/changelog.html @@ -58,6 +58,9 @@ Upcoming changes
  • Log recorders do not work reliably. (issue 15226) +
  • + FilePath.validateAntFileMask too slow for /configure. + (issue 7214)
  • diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java index 8d8254ec25..8c771dcdd1 100644 --- a/core/src/main/java/hudson/FilePath.java +++ b/core/src/main/java/hudson/FilePath.java @@ -27,7 +27,6 @@ package hudson; import hudson.Launcher.LocalLauncher; import hudson.Launcher.RemoteLauncher; -import hudson.model.AbstractDescribableImpl; import jenkins.model.Jenkins; import hudson.model.TaskListener; import hudson.model.AbstractProject; @@ -840,7 +839,7 @@ public final class FilePath implements Serializable { return channel.call(wrapper); } 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) { @@ -1976,8 +1975,26 @@ public final class FilePath implements Serializable { * @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(); @@ -2074,10 +2091,28 @@ public final class FilePath implements Serializable { 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; } @@ -2126,11 +2161,11 @@ public final class FilePath implements Serializable { 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 9261588bf8..e644e028be 100644 --- a/core/src/main/java/hudson/util/FormFieldValidator.java +++ b/core/src/main/java/hudson/util/FormFieldValidator.java @@ -378,11 +378,11 @@ public abstract class FormFieldValidator { 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 38ec4e855e..a9fcae1043 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 3a67712018..5f40d7f1c3 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 0501d90e3b..4d2a9fab23 100644 --- a/core/src/test/java/hudson/FilePathTest.java +++ b/core/src/test/java/hudson/FilePathTest.java @@ -449,4 +449,30 @@ public class FilePathTest extends ChannelTestCase { 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); + } + } + }