[JENKINS-23507] Make FilePath.installIfNecessaryFrom follow redirects (…

…#2331)
jenkinsci · May 13, 2016 · 0c16c85 · 0c16c85
1 parent 782804f
commit 0c16c85
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 6 deletions.
diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java
@@ -25,6 +25,7 @@
  */
 package hudson;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.jcraft.jzlib.GZIPInputStream;
 import com.jcraft.jzlib.GZIPOutputStream;
 import hudson.Launcher.LocalLauncher;
@@ -71,6 +72,8 @@
 import org.apache.tools.ant.types.FileSet;
 import org.apache.tools.zip.ZipEntry;
 import org.apache.tools.zip.ZipFile;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.stapler.Stapler;
 
 import javax.annotation.CheckForNull;
@@ -91,6 +94,7 @@
 import java.io.Serializable;
 import java.io.Writer;
 import java.net.HttpURLConnection;
+import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.URL;
 import java.net.URLConnection;
@@ -188,6 +192,11 @@
  * @see VirtualFile
  */
 public final class FilePath implements Serializable {
+    /**
+     * Maximum http redirects we will follow. This defaults to the same number as Firefox/Chrome tolerates.
+     */
+    private static final int MAX_REDIRECTS = 20;
+
     /**
      * When this {@link FilePath} represents the remote path,
      * this field is always non-null on master (the field represents
@@ -755,6 +764,10 @@ public Void invoke(File dir, VirtualChannel channel) throws IOException {
      * @since 1.299
      */
     public boolean installIfNecessaryFrom(@Nonnull URL archive, @CheckForNull TaskListener listener, @Nonnull String message) throws IOException, InterruptedException {
+        return installIfNecessaryFrom(archive, listener, message, MAX_REDIRECTS);
+    }
+
+    private boolean installIfNecessaryFrom(@Nonnull URL archive, @CheckForNull TaskListener listener, @Nonnull String message, int maxRedirects) throws InterruptedException, IOException {
         try {
             FilePath timestamp = this.child(".timestamp");
             long lastModified = timestamp.lastModified();
@@ -777,14 +790,28 @@ public boolean installIfNecessaryFrom(@Nonnull URL archive, @CheckForNull TaskLi
                 }
             }
 
-            if (lastModified != 0 && con instanceof HttpURLConnection) {
+            if (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;
+                if (responseCode == HttpURLConnection.HTTP_MOVED_PERM
+                        || responseCode == HttpURLConnection.HTTP_MOVED_TEMP) {
+                    // follows redirect
+                    if (maxRedirects > 0) {
+                        String location = httpCon.getHeaderField("Location");
+                        listener.getLogger().println("Following redirect " + archive.toExternalForm() + " -> " + location);
+                        return installIfNecessaryFrom(getUrlFactory().newURL(location), listener, message, maxRedirects - 1);
+                    } else {
+                        listener.getLogger().println("Skipping installation of " + archive + " to " + remote + " due to too many redirects.");
+                        return false;
+                    }
+                }
+                if (lastModified != 0) {
+                    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;
+                    }
                 }
             }
 
@@ -2520,6 +2547,31 @@ private int findSeparator(String pattern) {
         });
     }
 
+    private static final UrlFactory DEFAULT_URL_FACTORY = new UrlFactory();
+
+    @Restricted(NoExternalUse.class)
+    static class UrlFactory {
+        public URL newURL(String location) throws MalformedURLException {
+            return new URL(location);
+        }
+    }
+
+    private UrlFactory urlFactory;
+
+    @VisibleForTesting
+    @Restricted(NoExternalUse.class)
+    void setUrlFactory(UrlFactory urlFactory) {
+        this.urlFactory = urlFactory;
+    }
+
+    private UrlFactory getUrlFactory() {
+        if (urlFactory != null) {
+            return urlFactory;
+        } else {
+            return DEFAULT_URL_FACTORY;
+        }
+    }
+
     /**
      * Short for {@code validateFileMask(path, value, true)}
      */

diff --git a/core/src/test/java/hudson/FilePathTest.java b/core/src/test/java/hudson/FilePathTest.java
@@ -38,9 +38,11 @@
 import java.io.RandomAccessFile;
 import java.net.ConnectException;
 import java.net.HttpURLConnection;
+import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.URLConnection;
 import java.net.URLStreamHandler;
+import java.net.URLStreamHandlerFactory;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashSet;
@@ -637,6 +639,28 @@ public void testValidateCaseSensitivity() throws Exception {
         assertTrue(log, log.contains("504 Gateway Timeout"));
     }
 
+    @Issue("JENKINS-23507")
+    @Test public void installIfNecessaryFollowsRedirects() throws Exception{
+        File tmp = temp.getRoot();
+        final FilePath d = new FilePath(tmp);
+        FilePath.UrlFactory urlFactory = mock(FilePath.UrlFactory.class);
+        d.setUrlFactory(urlFactory);
+        final HttpURLConnection con = mock(HttpURLConnection.class);
+        final HttpURLConnection con2 = mock(HttpURLConnection.class);
+        final URL url = someUrlToZipFile(con);
+        when(con.getResponseCode()).thenReturn(HttpURLConnection.HTTP_MOVED_TEMP);
+        URL url2 = someUrlToZipFile(con2);
+        String someUrl = url2.toExternalForm();
+        when(con.getHeaderField("Location")).thenReturn(someUrl);
+        when(urlFactory.newURL(someUrl)).thenReturn(url2);
+        when(con2.getResponseCode()).thenReturn(HttpURLConnection.HTTP_OK);
+        when(con2.getInputStream()).thenReturn(someZippedContent());
+
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        String message = "going ahead";
+        assertTrue(d.installIfNecessaryFrom(url, new StreamTaskListener(baos), message));
+    }
+
     private URL someUrlToZipFile(final URLConnection con) throws IOException {
 
         final URLStreamHandler urlHandler = new URLStreamHandler() {