From 97600b26e28f1fa79ed45c81c2f8dc957a8c5c40 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Thu, 23 Jan 2020 17:02:53 +0100 Subject: [PATCH 1/6] Check plugin dependencies after download --- .../sonia/scm/plugin/PluginInstaller.java | 15 +++- .../scm/plugin/SmpDDescriptorExtractor.java | 28 +++++++ .../sonia/scm/plugin/PluginInstallerTest.java | 23 +++++- .../plugin/SmpDDescriptorExtractorTest.java | 76 +++++++++++++++++++ 4 files changed, 140 insertions(+), 2 deletions(-) create mode 100644 scm-webapp/src/main/java/sonia/scm/plugin/SmpDDescriptorExtractor.java create mode 100644 scm-webapp/src/test/java/sonia/scm/plugin/SmpDDescriptorExtractorTest.java 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 6f003c1e31..c06039bf0a 100644 --- a/scm-webapp/src/main/java/sonia/scm/plugin/PluginInstaller.java +++ b/scm-webapp/src/main/java/sonia/scm/plugin/PluginInstaller.java @@ -19,11 +19,13 @@ class PluginInstaller { private final SCMContextProvider context; private final AdvancedHttpClient client; + private final SmpDDescriptorExtractor smpDDescriptorExtractor; @Inject - public PluginInstaller(SCMContextProvider context, AdvancedHttpClient client) { + public PluginInstaller(SCMContextProvider context, AdvancedHttpClient client, SmpDDescriptorExtractor smpDDescriptorExtractor) { this.context = context; this.client = client; + this.smpDDescriptorExtractor = smpDDescriptorExtractor; } @SuppressWarnings("squid:S4790") // hashing should be safe @@ -34,6 +36,17 @@ class PluginInstaller { Files.copy(input, file); verifyChecksum(plugin, input.hash(), file); + InstalledPluginDescriptor pluginDescriptor = smpDDescriptorExtractor.extractPluginDescriptor(file); + if (!pluginDescriptor.getCondition().isSupported()) { + cleanup(file); + throw new PluginConditionFailedException( + pluginDescriptor.getCondition(), + String.format( + "could not load plugin %s, the plugin condition does not match", + pluginDescriptor.getInformation().getId() + ) + ); + } return new PendingPluginInstallation(plugin.install(), file); } catch (IOException ex) { cleanup(file); diff --git a/scm-webapp/src/main/java/sonia/scm/plugin/SmpDDescriptorExtractor.java b/scm-webapp/src/main/java/sonia/scm/plugin/SmpDDescriptorExtractor.java new file mode 100644 index 0000000000..b218acf157 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/plugin/SmpDDescriptorExtractor.java @@ -0,0 +1,28 @@ +package sonia.scm.plugin; + +import javax.xml.bind.JAXBContext; +import javax.xml.bind.JAXBException; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.zip.ZipEntry; +import java.util.zip.ZipInputStream; + +class SmpDDescriptorExtractor { + + InstalledPluginDescriptor extractPluginDescriptor(Path file) throws IOException { + try (ZipInputStream zipInputStream = new ZipInputStream(Files.newInputStream(file), StandardCharsets.UTF_8)) { + ZipEntry nextEntry; + while ((nextEntry = zipInputStream.getNextEntry()) != null) { + if ("META-INF/scm/plugin.xml".equals(nextEntry.getName())) { + JAXBContext context = JAXBContext.newInstance(ScmModule.class, InstalledPluginDescriptor.class); + return (InstalledPluginDescriptor) context.createUnmarshaller().unmarshal(zipInputStream); + } + } + } catch (JAXBException e) { + throw new IOException("failed to read descriptor file META-INF/scm/plugin.xml from plugin", e); + } + throw new IOException("Missing plugin descriptor META-INF/scm/plugin.xml in download package"); + } +} 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 3f918cd4fa..b71fec5bc6 100644 --- a/scm-webapp/src/test/java/sonia/scm/plugin/PluginInstallerTest.java +++ b/scm-webapp/src/test/java/sonia/scm/plugin/PluginInstallerTest.java @@ -32,18 +32,23 @@ class PluginInstallerTest { @Mock(answer = Answers.RETURNS_DEEP_STUBS) private AdvancedHttpClient client; + @Mock + private SmpDDescriptorExtractor extractor; + @InjectMocks private PluginInstaller installer; private Path directory; @BeforeEach - void setUpContext(@TempDirectory.TempDir Path directory) { + void setUpContext(@TempDirectory.TempDir Path directory) throws IOException { this.directory = directory; lenient().when(context.resolve(any())).then(ic -> { Path arg = ic.getArgument(0); return directory.resolve(arg); }); + InstalledPluginDescriptor supportedPlugin = createPluginDescriptor(true); + lenient().when(extractor.extractPluginDescriptor(any())).thenReturn(supportedPlugin); } @Test @@ -105,6 +110,15 @@ class PluginInstallerTest { assertThat(directory.resolve("plugins").resolve("scm-git-plugin.smp")).doesNotExist(); } + @Test + void shouldFailForUnsupportedPlugin() throws IOException { + mockContent("42"); + InstalledPluginDescriptor supportedPlugin = createPluginDescriptor(false); + when(extractor.extractPluginDescriptor(any())).thenReturn(supportedPlugin); + + assertThrows(PluginConditionFailedException.class, () -> installer.install(createGitPlugin())); + assertThat(directory.resolve("plugins").resolve("scm-git-plugin.smp")).doesNotExist(); + } private AvailablePlugin createPlugin(String name, String url, String checksum) { PluginInformation information = new PluginInformation(); @@ -114,4 +128,11 @@ class PluginInstallerTest { ); return new AvailablePlugin(descriptor); } + + private InstalledPluginDescriptor createPluginDescriptor(boolean supported) { + InstalledPluginDescriptor installedPluginDescriptor = mock(InstalledPluginDescriptor.class, RETURNS_DEEP_STUBS); + lenient().when(installedPluginDescriptor.getCondition().isSupported()).thenReturn(supported); + lenient().when(installedPluginDescriptor.getInformation().getId()).thenReturn("scm-git-plugin"); + return installedPluginDescriptor; + } } diff --git a/scm-webapp/src/test/java/sonia/scm/plugin/SmpDDescriptorExtractorTest.java b/scm-webapp/src/test/java/sonia/scm/plugin/SmpDDescriptorExtractorTest.java new file mode 100644 index 0000000000..51d9005cc0 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/plugin/SmpDDescriptorExtractorTest.java @@ -0,0 +1,76 @@ +package sonia.scm.plugin; + +import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junitpioneer.jupiter.TempDirectory; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.jupiter.api.Assertions.assertThrows; + +@ExtendWith(TempDirectory.class) +class SmpDDescriptorExtractorTest { + + private static final String PLUGIN_XML = "\n" + + "\n" + + "\n" + + " 2\n" + + "\n" + + " \n" + + " Test\n" + + " Cloudogu GmbH\n" + + " Testing\n" + + " scm-test-plugin\n" + + "2.0.0\n" + + "Collects information for support cases\n" + + "\n" + + "\n" + + " \n" + + " 2.0.0-rc1\n" + + " \n" + + "\n" + + " \n" + + " \n" + + " \n" + + "\n" + + "\n"; + + @Test + void shouldExtractPluginXml(@TempDirectory.TempDir Path tempDir) throws IOException { + Path pluginFile = createZipFile(tempDir, "META-INF/scm/plugin.xml", PLUGIN_XML); + + InstalledPluginDescriptor installedPluginDescriptor = new SmpDDescriptorExtractor().extractPluginDescriptor(pluginFile); + + Assertions.assertThat(installedPluginDescriptor.getInformation().getName()).isEqualTo("scm-test-plugin"); + } + + @Test + void shouldFailWithoutPluginXml(@TempDirectory.TempDir Path tempDir) throws IOException { + Path pluginFile = createZipFile(tempDir, "META-INF/wrong/plugin.xml", PLUGIN_XML); + + assertThrows(IOException.class, () -> new SmpDDescriptorExtractor().extractPluginDescriptor(pluginFile)); + } + + @Test + void shouldFailWithIllegalPluginXml(@TempDirectory.TempDir Path tempDir) throws IOException { + Path pluginFile = createZipFile(tempDir, "META-INF/scm/plugin.xml", "content"); + + assertThrows(IOException.class, () -> new SmpDDescriptorExtractor().extractPluginDescriptor(pluginFile)); + } + + Path createZipFile(Path tempDir, String internalFileName, String content) throws IOException { + Path pluginFile = tempDir.resolve("scm-test-plugin.smp"); + ZipOutputStream zipOutputStream = new ZipOutputStream(Files.newOutputStream(pluginFile), UTF_8); + zipOutputStream.putNextEntry(new ZipEntry(internalFileName)); + zipOutputStream.write(content.getBytes(UTF_8)); + zipOutputStream.closeEntry(); + zipOutputStream.close(); + return pluginFile; + } +} From c80ee02d8a5131f48604ac5e2cf4f773a28ec724 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Thu, 23 Jan 2020 17:05:02 +0100 Subject: [PATCH 2/6] Use plugin name from available plugin --- scm-webapp/src/main/java/sonia/scm/plugin/PluginInstaller.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 c06039bf0a..9f0637124c 100644 --- a/scm-webapp/src/main/java/sonia/scm/plugin/PluginInstaller.java +++ b/scm-webapp/src/main/java/sonia/scm/plugin/PluginInstaller.java @@ -43,7 +43,7 @@ class PluginInstaller { pluginDescriptor.getCondition(), String.format( "could not load plugin %s, the plugin condition does not match", - pluginDescriptor.getInformation().getId() + plugin.getDescriptor().getInformation().getName() ) ); } From c0fd9b97b4679d5f55a087f4e400be066c29cfa0 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Thu, 23 Jan 2020 17:05:55 +0100 Subject: [PATCH 3/6] Cleanup code --- .../sonia/scm/plugin/PluginInstaller.java | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 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 9f0637124c..1fc7ac5011 100644 --- a/scm-webapp/src/main/java/sonia/scm/plugin/PluginInstaller.java +++ b/scm-webapp/src/main/java/sonia/scm/plugin/PluginInstaller.java @@ -36,17 +36,7 @@ class PluginInstaller { Files.copy(input, file); verifyChecksum(plugin, input.hash(), file); - InstalledPluginDescriptor pluginDescriptor = smpDDescriptorExtractor.extractPluginDescriptor(file); - if (!pluginDescriptor.getCondition().isSupported()) { - cleanup(file); - throw new PluginConditionFailedException( - pluginDescriptor.getCondition(), - String.format( - "could not load plugin %s, the plugin condition does not match", - plugin.getDescriptor().getInformation().getName() - ) - ); - } + verifyConditions(plugin, file); return new PendingPluginInstallation(plugin.install(), file); } catch (IOException ex) { cleanup(file); @@ -77,6 +67,20 @@ class PluginInstaller { } } + private void verifyConditions(AvailablePlugin plugin, Path file) throws IOException { + InstalledPluginDescriptor pluginDescriptor = smpDDescriptorExtractor.extractPluginDescriptor(file); + if (!pluginDescriptor.getCondition().isSupported()) { + cleanup(file); + throw new PluginConditionFailedException( + pluginDescriptor.getCondition(), + String.format( + "could not load plugin %s, the plugin condition does not match", + plugin.getDescriptor().getInformation().getName() + ) + ); + } + } + private InputStream download(AvailablePlugin plugin) throws IOException { return client.get(plugin.getDescriptor().getUrl()).request().contentAsStream(); } From 868cfb4a0ae7212a6d252cb607946c1cd3b37cb0 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Fri, 24 Jan 2020 12:01:21 +0100 Subject: [PATCH 4/6] Fix class name --- .../src/main/java/sonia/scm/plugin/PluginInstaller.java | 8 ++++---- ...scriptorExtractor.java => SmpDescriptorExtractor.java} | 2 +- .../test/java/sonia/scm/plugin/PluginInstallerTest.java | 2 +- ...ExtractorTest.java => SmpDescriptorExtractorTest.java} | 8 ++++---- 4 files changed, 10 insertions(+), 10 deletions(-) rename scm-webapp/src/main/java/sonia/scm/plugin/{SmpDDescriptorExtractor.java => SmpDescriptorExtractor.java} (97%) rename scm-webapp/src/test/java/sonia/scm/plugin/{SmpDDescriptorExtractorTest.java => SmpDescriptorExtractorTest.java} (88%) 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 1fc7ac5011..082058d663 100644 --- a/scm-webapp/src/main/java/sonia/scm/plugin/PluginInstaller.java +++ b/scm-webapp/src/main/java/sonia/scm/plugin/PluginInstaller.java @@ -19,13 +19,13 @@ class PluginInstaller { private final SCMContextProvider context; private final AdvancedHttpClient client; - private final SmpDDescriptorExtractor smpDDescriptorExtractor; + private final SmpDescriptorExtractor smpDescriptorExtractor; @Inject - public PluginInstaller(SCMContextProvider context, AdvancedHttpClient client, SmpDDescriptorExtractor smpDDescriptorExtractor) { + public PluginInstaller(SCMContextProvider context, AdvancedHttpClient client, SmpDescriptorExtractor smpDescriptorExtractor) { this.context = context; this.client = client; - this.smpDDescriptorExtractor = smpDDescriptorExtractor; + this.smpDescriptorExtractor = smpDescriptorExtractor; } @SuppressWarnings("squid:S4790") // hashing should be safe @@ -68,7 +68,7 @@ class PluginInstaller { } private void verifyConditions(AvailablePlugin plugin, Path file) throws IOException { - InstalledPluginDescriptor pluginDescriptor = smpDDescriptorExtractor.extractPluginDescriptor(file); + InstalledPluginDescriptor pluginDescriptor = smpDescriptorExtractor.extractPluginDescriptor(file); if (!pluginDescriptor.getCondition().isSupported()) { cleanup(file); throw new PluginConditionFailedException( diff --git a/scm-webapp/src/main/java/sonia/scm/plugin/SmpDDescriptorExtractor.java b/scm-webapp/src/main/java/sonia/scm/plugin/SmpDescriptorExtractor.java similarity index 97% rename from scm-webapp/src/main/java/sonia/scm/plugin/SmpDDescriptorExtractor.java rename to scm-webapp/src/main/java/sonia/scm/plugin/SmpDescriptorExtractor.java index b218acf157..b8e9cddabd 100644 --- a/scm-webapp/src/main/java/sonia/scm/plugin/SmpDDescriptorExtractor.java +++ b/scm-webapp/src/main/java/sonia/scm/plugin/SmpDescriptorExtractor.java @@ -9,7 +9,7 @@ import java.nio.file.Path; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; -class SmpDDescriptorExtractor { +class SmpDescriptorExtractor { InstalledPluginDescriptor extractPluginDescriptor(Path file) throws IOException { try (ZipInputStream zipInputStream = new ZipInputStream(Files.newInputStream(file), StandardCharsets.UTF_8)) { 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 b71fec5bc6..fbc22e1ebd 100644 --- a/scm-webapp/src/test/java/sonia/scm/plugin/PluginInstallerTest.java +++ b/scm-webapp/src/test/java/sonia/scm/plugin/PluginInstallerTest.java @@ -33,7 +33,7 @@ class PluginInstallerTest { private AdvancedHttpClient client; @Mock - private SmpDDescriptorExtractor extractor; + private SmpDescriptorExtractor extractor; @InjectMocks private PluginInstaller installer; diff --git a/scm-webapp/src/test/java/sonia/scm/plugin/SmpDDescriptorExtractorTest.java b/scm-webapp/src/test/java/sonia/scm/plugin/SmpDescriptorExtractorTest.java similarity index 88% rename from scm-webapp/src/test/java/sonia/scm/plugin/SmpDDescriptorExtractorTest.java rename to scm-webapp/src/test/java/sonia/scm/plugin/SmpDescriptorExtractorTest.java index 51d9005cc0..6ca19a3692 100644 --- a/scm-webapp/src/test/java/sonia/scm/plugin/SmpDDescriptorExtractorTest.java +++ b/scm-webapp/src/test/java/sonia/scm/plugin/SmpDescriptorExtractorTest.java @@ -15,7 +15,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.jupiter.api.Assertions.assertThrows; @ExtendWith(TempDirectory.class) -class SmpDDescriptorExtractorTest { +class SmpDescriptorExtractorTest { private static final String PLUGIN_XML = "\n" + "\n" + @@ -45,7 +45,7 @@ class SmpDDescriptorExtractorTest { void shouldExtractPluginXml(@TempDirectory.TempDir Path tempDir) throws IOException { Path pluginFile = createZipFile(tempDir, "META-INF/scm/plugin.xml", PLUGIN_XML); - InstalledPluginDescriptor installedPluginDescriptor = new SmpDDescriptorExtractor().extractPluginDescriptor(pluginFile); + InstalledPluginDescriptor installedPluginDescriptor = new SmpDescriptorExtractor().extractPluginDescriptor(pluginFile); Assertions.assertThat(installedPluginDescriptor.getInformation().getName()).isEqualTo("scm-test-plugin"); } @@ -54,14 +54,14 @@ class SmpDDescriptorExtractorTest { void shouldFailWithoutPluginXml(@TempDirectory.TempDir Path tempDir) throws IOException { Path pluginFile = createZipFile(tempDir, "META-INF/wrong/plugin.xml", PLUGIN_XML); - assertThrows(IOException.class, () -> new SmpDDescriptorExtractor().extractPluginDescriptor(pluginFile)); + assertThrows(IOException.class, () -> new SmpDescriptorExtractor().extractPluginDescriptor(pluginFile)); } @Test void shouldFailWithIllegalPluginXml(@TempDirectory.TempDir Path tempDir) throws IOException { Path pluginFile = createZipFile(tempDir, "META-INF/scm/plugin.xml", "content"); - assertThrows(IOException.class, () -> new SmpDDescriptorExtractor().extractPluginDescriptor(pluginFile)); + assertThrows(IOException.class, () -> new SmpDescriptorExtractor().extractPluginDescriptor(pluginFile)); } Path createZipFile(Path tempDir, String internalFileName, String content) throws IOException { From 55dcbc1947c75d942b104b92d052edcf0c931b3c Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Fri, 24 Jan 2020 12:01:27 +0100 Subject: [PATCH 5/6] Add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a83e5b743b..2995cea4fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Set individual page title - Copy on write - A new repository can be initialized with a branch (for git and mercurial) and custom files (README.md on default) +- Plugins are validated directly after download ### Changed - Stop fetching commits when it takes too long From 2816370e8af2460649c801b6a0123800d98f1fbb Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Fri, 24 Jan 2020 12:20:27 +0000 Subject: [PATCH 6/6] Close branch feature/check_plugin_dependencies_after_download