SONAR-4685 The update center cannot download JARs if the URL is a redirect

2 files changed, 42 insertions, 15 deletions

diff --git a/sonar-server/src/main/java/org/sonar/server/plugins/PluginDownloader.java b/sonar-server/src/main/java/org/sonar/server/plugins/PluginDownloader.java
index d1ac365862a..0bdb0d2bba4 100644
--- a/sonar-server/src/main/java/org/sonar/server/plugins/PluginDownloader.java
+++ b/sonar-server/src/main/java/org/sonar/server/plugins/PluginDownloader.java
@@ -43,6 +43,7 @@ public class PluginDownloader implements ServerComponent, Startable {
 
   private static final Logger LOG = LoggerFactory.getLogger(PluginDownloader.class);
   private static final String TMP_SUFFIX = "tmp";
+  private static final String PLUGIN_EXTENSION = "jar";
 
   private final UpdateCenterMatrixFactory updateCenterMatrixFactory;
   private final HttpDownloader downloader;
@@ -56,13 +57,14 @@ public class PluginDownloader implements ServerComponent, Startable {
 
   /**
    * Delete the temporary files remaining from previous downloads
+   *
    * @see #downloadRelease(org.sonar.updatecenter.common.Release)
    */
   @Override
   public void start() {
     try {
       FileUtils.forceMkdir(downloadDir);
-      Collection<File> tempFiles = FileUtils.listFiles(downloadDir, new String[] {TMP_SUFFIX}, false);
+      Collection<File> tempFiles = FileUtils.listFiles(downloadDir, new String[]{TMP_SUFFIX}, false);
       for (File tempFile : tempFiles) {
         FileUtils.deleteQuietly(tempFile);
       }
@@ -93,7 +95,7 @@ public class PluginDownloader implements ServerComponent, Startable {
 
   public List<String> getDownloads() {
     List<String> names = new ArrayList<String>();
-    List<File> files = (List<File>) FileUtils.listFiles(downloadDir, new String[] {"jar"}, false);
+    List<File> files = (List<File>) FileUtils.listFiles(downloadDir, new String[]{PLUGIN_EXTENSION}, false);
     for (File file : files) {
       names.add(file.getName());
     }
@@ -106,8 +108,9 @@ public class PluginDownloader implements ServerComponent, Startable {
         downloadRelease(release);
 
       } catch (Exception e) {
-        String message = "Fail to download the plugin (" + release.getArtifact().getKey() + ", version " + release.getVersion().getName() + ") from " + release.getDownloadUrl();
-        LOG.warn(message, e);
+        String message = String.format("Fail to download the plugin (%s, version %s) from %s (error is : %s)",
+          release.getArtifact().getKey(), release.getVersion().getName(), release.getDownloadUrl(), e.getMessage());
+        LOG.debug(message, e);
         throw new SonarException(message, e);
       }
     }
@@ -115,6 +118,7 @@ public class PluginDownloader implements ServerComponent, Startable {
 
   private void downloadRelease(Release release) throws URISyntaxException, IOException {
     String url = release.getDownloadUrl();
+
     URI uri = new URI(url);
     if (url.startsWith("file:")) {
       // used for tests
@@ -122,6 +126,9 @@ public class PluginDownloader implements ServerComponent, Startable {
       FileUtils.copyFileToDirectory(file, downloadDir);
     } else {
       String filename = StringUtils.substringAfterLast(uri.getPath(), "/");
+      if (!filename.endsWith("." + PLUGIN_EXTENSION)) {
+        filename = release.getKey() + "-" + release.getVersion() + "." + PLUGIN_EXTENSION;
+      }
       File targetFile = new File(downloadDir, filename);
       File tempFile = new File(downloadDir, filename + "." + TMP_SUFFIX);
       downloader.download(uri, tempFile);
diff --git a/sonar-server/src/test/java/org/sonar/server/plugins/PluginDownloaderTest.java b/sonar-server/src/test/java/org/sonar/server/plugins/PluginDownloaderTest.java
index 174ab7a5ab6..18831dbf899 100644
--- a/sonar-server/src/test/java/org/sonar/server/plugins/PluginDownloaderTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/plugins/PluginDownloaderTest.java
@@ -21,7 +21,10 @@ package org.sonar.server.plugins;
 
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.FilenameUtils;
-import org.junit.*;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 import org.mockito.ArgumentMatcher;
 import org.mockito.invocation.InvocationOnMock;
@@ -35,7 +38,6 @@ import org.sonar.updatecenter.common.UpdateCenter;
 import org.sonar.updatecenter.common.Version;
 
 import java.io.File;
-import java.io.FilenameFilter;
 import java.net.URI;
 
 import static com.google.common.collect.Lists.newArrayList;
@@ -66,7 +68,7 @@ public class PluginDownloaderTest {
     doAnswer(new Answer() {
       @Override
       public Object answer(InvocationOnMock inv) throws Throwable {
-        File toFile = (File)inv.getArguments()[1];
+        File toFile = (File) inv.getArguments()[1];
         FileUtils.touch(toFile);
         return null;
       }
@@ -85,7 +87,7 @@ public class PluginDownloaderTest {
   }
 
   @Test
-  public void should_clean_temporary_files_at_startup() throws Exception {
+  public void clean_temporary_files_at_startup() throws Exception {
     FileUtils.touch(new File(downloadDir, "sonar-php.jar"));
     FileUtils.touch(new File(downloadDir, "sonar-js.jar.tmp"));
     assertThat(downloadDir.listFiles()).hasSize(2);
@@ -97,7 +99,7 @@ public class PluginDownloaderTest {
   }
 
   @Test
-  public void should_download_from_url() throws Exception {
+  public void download_from_url() throws Exception {
     Plugin test = new Plugin("test");
     Release test10 = new Release(test, "1.0").setDownloadUrl("http://server/test-1.0.jar");
     test.addRelease(test10);
@@ -115,7 +117,25 @@ public class PluginDownloaderTest {
   }
 
   @Test
-  public void should_throw_exception_if_download_dir_is_invalid() throws Exception {
+  public void download_from_url_not_finishing_by_jar_extension() throws Exception {
+    Plugin test = new Plugin("plugin-test");
+    Release test10 = new Release(test, "1.0").setDownloadUrl("http://server/redirect?r=release&g=test&a=test&v=1.0&e=jar");
+    test.addRelease(test10);
+
+    when(updateCenter.findInstallablePlugins("foo", Version.create("1.0"))).thenReturn(newArrayList(test10));
+
+    pluginDownloader.start();
+    pluginDownloader.download("foo", Version.create("1.0"));
+
+    // SONAR-4523: do not corrupt JAR files when restarting the server while a plugin is being downloaded.
+    // The JAR file is downloaded in a temp file
+    verify(httpDownloader).download(any(URI.class), argThat(new HasFileName("plugin-test-1.0.jar.tmp")));
+    assertThat(new File(downloadDir, "plugin-test-1.0.jar")).exists();
+    assertThat(new File(downloadDir, "plugin-test-1.0.jar.tmp")).doesNotExist();
+  }
+
+  @Test
+  public void throw_exception_if_download_dir_is_invalid() throws Exception {
     DefaultServerFileSystem defaultServerFileSystem = mock(DefaultServerFileSystem.class);
     // download dir is a file instead of being a directory
     File downloadDir = testFolder.newFile();
@@ -131,7 +151,7 @@ public class PluginDownloaderTest {
   }
 
   @Test
-  public void should_download_from_file() throws Exception {
+  public void download_from_file() throws Exception {
     Plugin test = new Plugin("test");
     File file = testFolder.newFile("test-1.0.jar");
     file.createNewFile();
@@ -147,7 +167,7 @@ public class PluginDownloaderTest {
   }
 
   @Test
-  public void should_throw_exception_if_could_not_download() throws Exception {
+  public void throw_exception_if_could_not_download() throws Exception {
     Plugin test = new Plugin("test");
     Release test10 = new Release(test, "1.0").setDownloadUrl("file://not_found");
     test.addRelease(test10);
@@ -164,7 +184,7 @@ public class PluginDownloaderTest {
   }
 
   @Test
-  public void should_throw_exception_if_download_fail() throws Exception {
+  public void throw_exception_if_download_fail() throws Exception {
     Plugin test = new Plugin("test");
     Release test10 = new Release(test, "1.0").setDownloadUrl("http://server/test-1.0.jar");
     test.addRelease(test10);
@@ -182,7 +202,7 @@ public class PluginDownloaderTest {
   }
 
   @Test
-  public void should_read_download_folder() throws Exception {
+  public void read_download_folder() throws Exception {
     pluginDownloader.start();
     assertThat(pluginDownloader.getDownloads()).hasSize(0);
 
@@ -195,7 +215,7 @@ public class PluginDownloaderTest {
   }
 
   @Test
-  public void should_cancel_downloads() throws Exception {
+  public void cancel_downloads() throws Exception {
     File file1 = new File(downloadDir, "file1.jar");
     file1.createNewFile();
     File file2 = new File(downloadDir, "file2.jar");