From 7ef4b30027cb7d5e387bd51d3bdaf532c9b7303c Mon Sep 17 00:00:00 2001
From: Sebastian Sdorra <sebastian.sdorra@cloudogu.com>
Date: Wed, 21 Aug 2019 07:56:52 +0200
Subject: [PATCH] remove downloaded artifact on error

---
 .../sonia/scm/plugin/PluginInstaller.java     | 22 ++++++++++++++-----
 .../sonia/scm/plugin/PluginInstallerTest.java | 18 +++++++++++----
 2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/scm-webapp/src/main/java/sonia/scm/plugin/PluginInstaller.java b/scm-webapp/src/main/java/sonia/scm/plugin/PluginInstaller.java
index b001059ea0..71a4c35a3a 100644
--- a/scm-webapp/src/main/java/sonia/scm/plugin/PluginInstaller.java
+++ b/scm-webapp/src/main/java/sonia/scm/plugin/PluginInstaller.java
@@ -27,25 +27,35 @@ class PluginInstaller {
   }
 
   public PendingPluginInstallation install(AvailablePlugin plugin) {
+    Path file = null;
     try (HashingInputStream input = new HashingInputStream(Hashing.sha256(), download(plugin))) {
-      Path file = createFile(plugin);
+      file = createFile(plugin);
       Files.copy(input, file);
 
-      verifyChecksum(plugin, input.hash());
-
-      // TODO clean up in case of error
-
+      verifyChecksum(plugin, input.hash(), file);
       return new PendingPluginInstallation(plugin, file);
     } catch (IOException ex) {
+      cleanup(file);
       throw new PluginDownloadException("failed to download plugin", ex);
     }
   }
 
-  private void verifyChecksum(AvailablePlugin plugin, HashCode hash) {
+  private void cleanup(Path file) {
+    try {
+      if (file != null) {
+        Files.deleteIfExists(file);
+      }
+    } catch (IOException e) {
+      throw new PluginInstallException("failed to cleanup, after broken installation");
+    }
+  }
+
+  private void verifyChecksum(AvailablePlugin plugin, HashCode hash, Path file) {
     Optional<String> checksum = plugin.getDescriptor().getChecksum();
     if (checksum.isPresent()) {
       String calculatedChecksum = hash.toString();
       if (!checksum.get().equalsIgnoreCase(calculatedChecksum)) {
+        cleanup(file);
         throw new PluginChecksumMismatchException(
           String.format("downloaded plugin checksum %s does not match expected %s", calculatedChecksum, checksum.get())
         );
diff --git a/scm-webapp/src/test/java/sonia/scm/plugin/PluginInstallerTest.java b/scm-webapp/src/test/java/sonia/scm/plugin/PluginInstallerTest.java
index 0aa99c358c..4e2de333b9 100644
--- a/scm-webapp/src/test/java/sonia/scm/plugin/PluginInstallerTest.java
+++ b/scm-webapp/src/test/java/sonia/scm/plugin/PluginInstallerTest.java
@@ -13,16 +13,15 @@ import sonia.scm.net.ahc.AdvancedHttpClient;
 
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
 import java.util.Collections;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.in;
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.lenient;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
 
 @ExtendWith({MockitoExtension.class, TempDirectory.class})
 class PluginInstallerTest {
@@ -92,6 +91,17 @@ class PluginInstallerTest {
     mockContent("21");
 
     assertThrows(PluginChecksumMismatchException.class, () -> installer.install(createGitPlugin()));
+    assertThat(directory.resolve("plugins").resolve("scm-git-plugin.smp")).doesNotExist();
+  }
+
+  @Test
+  void shouldThrowPluginDownloadExceptionAndCleanup() throws IOException {
+    InputStream stream = mock(InputStream.class);
+    when(stream.read(any(), anyInt(), anyInt())).thenThrow(new IOException("failed to read"));
+    when(client.get("https://download.hitchhiker.com").request().contentAsStream()).thenReturn(stream);
+
+    assertThrows(PluginDownloadException.class, () -> installer.install(createGitPlugin()));
+    assertThat(directory.resolve("plugins").resolve("scm-git-plugin.smp")).doesNotExist();
   }