From 1fcee97d2fdda236fea149f671d9c8a95cdb8528 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 29 Dec 2014 14:49:27 -0500 Subject: [PATCH] [FIXED JENKINS-26196] More robust handling of server errors in FilePath.installIfNecessaryFrom. --- changelog.html | 3 +++ core/src/main/java/hudson/FilePath.java | 19 +++++++++++++------ core/src/test/java/hudson/FilePathTest.java | 21 +++++++++++++++++++++ 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/changelog.html b/changelog.html index 47f7795e79..3694e31222 100644 --- a/changelog.html +++ b/changelog.html @@ -63,6 +63,9 @@ Upcoming changes
  • Do not throw exception on /signup when not possible. (issue 11172) +
  • + Tool installer which downloads and unpacks archives should not fail the build if the tool already exists and the server returns an error code. + (issue 26196)
  • Fingerprint compaction aggravated lazy-loading performance issues. (issue 19392) diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java index aac8200829..fbbc89e876 100644 --- a/core/src/main/java/hudson/FilePath.java +++ b/core/src/main/java/hudson/FilePath.java @@ -756,11 +756,12 @@ public final class FilePath implements Serializable { public boolean installIfNecessaryFrom(@Nonnull URL archive, @CheckForNull TaskListener listener, @Nonnull String message) throws IOException, InterruptedException { try { FilePath timestamp = this.child(".timestamp"); + long lastModified = timestamp.lastModified(); URLConnection con; try { con = ProxyConfiguration.open(archive); - if (timestamp.exists()) { - con.setIfModifiedSince(timestamp.lastModified()); + if (lastModified != 0) { + con.setIfModifiedSince(lastModified); } con.connect(); } catch (IOException x) { @@ -775,15 +776,21 @@ public final class FilePath implements Serializable { } } - if (con instanceof HttpURLConnection - && ((HttpURLConnection)con).getResponseCode() == HttpURLConnection.HTTP_NOT_MODIFIED) { - return false; + if (lastModified != 0 && con instanceof HttpURLConnection) { + HttpURLConnection httpCon = (HttpURLConnection) con; + int responseCode = httpCon.getResponseCode(); + if (responseCode == HttpURLConnection.HTTP_NOT_MODIFIED) { + return false; + } else if (responseCode != HttpURLConnection.HTTP_OK) { + listener.getLogger().println("Skipping installation of " + archive + " to " + remote + " due to server error: " + responseCode + " " + httpCon.getResponseMessage()); + return false; + } } long sourceTimestamp = con.getLastModified(); if(this.exists()) { - if(timestamp.exists() && sourceTimestamp ==timestamp.lastModified()) + if (lastModified != 0 && sourceTimestamp == lastModified) return false; // already up to date this.deleteContents(); } else { diff --git a/core/src/test/java/hudson/FilePathTest.java b/core/src/test/java/hudson/FilePathTest.java index e6d60ecdfd..d9370cf145 100644 --- a/core/src/test/java/hudson/FilePathTest.java +++ b/core/src/test/java/hudson/FilePathTest.java @@ -27,6 +27,7 @@ import hudson.FilePath.TarCompression; import hudson.model.TaskListener; import hudson.remoting.VirtualChannel; import hudson.util.NullStream; +import hudson.util.StreamTaskListener; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.File; @@ -34,6 +35,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.net.ConnectException; import java.net.HttpURLConnection; import java.net.URL; import java.net.URLConnection; @@ -545,6 +547,25 @@ public class FilePathTest { assertTrue(d.installIfNecessaryFrom(url, null, null)); } + @Issue("JENKINS-26196") + @Test public void installIfNecessarySkipsDownloadWhenErroneous() throws Exception { + File tmp = temp.getRoot(); + final FilePath d = new FilePath(tmp); + d.child(".timestamp").touch(123000); + final HttpURLConnection con = mock(HttpURLConnection.class); + final URL url = someUrlToZipFile(con); + when(con.getResponseCode()).thenReturn(HttpURLConnection.HTTP_GATEWAY_TIMEOUT); + when(con.getResponseMessage()).thenReturn("Gateway Timeout"); + when(con.getInputStream()).thenThrow(new ConnectException()); + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + String message = "going ahead"; + assertFalse(d.installIfNecessaryFrom(url, new StreamTaskListener(baos), message)); + verify(con).setIfModifiedSince(123000); + String log = baos.toString(); + assertFalse(log, log.contains(message)); + assertTrue(log, log.contains("504 Gateway Timeout")); + } + private URL someUrlToZipFile(final URLConnection con) throws IOException { final URLStreamHandler urlHandler = new URLStreamHandler() {