From 53baf3c648f96debf6c0e6a6d343dca0646c412b Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Wed, 15 Jul 2026 17:17:06 +0200 Subject: [PATCH] Improve error handling Signed-off-by: Armin Schrenk --- .../launcher/VaultTemplateExtractor.java | 120 ++++++++++++------ .../org/cryptomator/ui/dialogs/Dialogs.java | 11 ++ .../ImportTemplateLocationController.java | 13 +- .../fxml/import_template_location.fxml | 8 +- src/main/resources/i18n/strings.properties | 4 +- .../launcher/VaultTemplateExtractorTest.java | 31 +++-- 6 files changed, 128 insertions(+), 59 deletions(-) diff --git a/src/main/java/org/cryptomator/launcher/VaultTemplateExtractor.java b/src/main/java/org/cryptomator/launcher/VaultTemplateExtractor.java index bfe843f7c..4857d3824 100644 --- a/src/main/java/org/cryptomator/launcher/VaultTemplateExtractor.java +++ b/src/main/java/org/cryptomator/launcher/VaultTemplateExtractor.java @@ -16,8 +16,8 @@ import java.nio.file.SimpleFileVisitor; import java.nio.file.StandardCopyOption; import java.nio.file.attribute.BasicFileAttributes; import java.util.Comparator; -import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Stream; +import java.util.zip.ZipException; /** * Unpacks a vault template (a ZIP archive holding a ready-made vault) and moves the contained vault to a destination. @@ -41,7 +41,8 @@ public final class VaultTemplateExtractor { * @param template the ZIP archive bytes * @param destination the target vault directory, which must not yet exist * @return {@code destination} - * @throws IOException if the template is not a valid ZIP, contains no vault, the destination already exists, or the + * @throws MalformedTemplateException if the template is not a valid zip or does not contain a vault + * @throws IOException if the destination already exists, or the * files cannot be written or moved */ public static Path extractAndMove(byte[] template, Path destination) throws IOException { @@ -66,58 +67,81 @@ public final class VaultTemplateExtractor { } private static Path unzip(Path zipFile, Path targetDir) throws IOException { - AtomicReference vaultConfig = new AtomicReference<>(); - long[] totalBytes = {0}; - int[] entryCount = {0}; Path normalizedTarget = targetDir.normalize(); try (FileSystem zipFs = FileSystems.newFileSystem(zipFile)) { - for (Path zipRoot : zipFs.getRootDirectories()) { - Files.walkFileTree(zipRoot, new SimpleFileVisitor<>() { - @Override - public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException { - if (!dir.equals(zipRoot)) { - countEntry(entryCount); - } - Files.createDirectories(resolveSafely(normalizedTarget, zipRoot, dir)); - return FileVisitResult.CONTINUE; - } + Path zipRoot = zipFs.getRootDirectories().iterator().next(); //we take the first available root and ignore others + var templateExtractor = new TemplateExtractionVisitor(zipRoot, normalizedTarget, MAX_ENTRIES, MAX_TOTAL_BYTES); + Files.walkFileTree(zipRoot, templateExtractor); + var vaultConfig = templateExtractor.getVaultConfig(); + if (vaultConfig == null) { + throw new MalformedTemplateException("Template does not contain a vault (no " + Constants.VAULTCONFIG_FILENAME + " found)."); + } + return vaultConfig.getParent(); + } catch (ZipException e) { + throw new MalformedTemplateException("Template is not a readable ZIP archive.", e); + } + } - @Override - public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { - countEntry(entryCount); - totalBytes[0] += attrs.size(); - if (totalBytes[0] > MAX_TOTAL_BYTES) { - throw new IOException("Vault template exceeds the maximum allowed size of " + MAX_TOTAL_BYTES + " bytes."); - } - var target = resolveSafely(normalizedTarget, zipRoot, file); - Files.copy(file, target, StandardCopyOption.REPLACE_EXISTING); - if (file.getFileName() != null && Constants.VAULTCONFIG_FILENAME.equals(file.getFileName().toString())) { - if (!vaultConfig.compareAndSet(null, target)) { - throw new IOException("Vault template contains more than one " + Constants.VAULTCONFIG_FILENAME); - } - } - return FileVisitResult.CONTINUE; - } - }); + static class TemplateExtractionVisitor extends SimpleFileVisitor { + + private final Path zipRoot; + private final Path target; + private final int maxEntries; + private final long maxSize; + + private int totalEntries = 0; + private long totalBytes = 0; + private Path vaultConfig = null; + + TemplateExtractionVisitor(Path zipRoot, Path target, int maxEntries, long maxSize) { + this.zipRoot = zipRoot; + this.target = target; + this.maxEntries = maxEntries + 1; //counting the root directory + this.maxSize = maxSize; + } + + @Override + public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException { + countEntry(); + Files.createDirectories(resolveSafely(target, zipRoot, dir)); + return FileVisitResult.CONTINUE; + } + + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { + countEntry(); + totalBytes += attrs.size(); + if (totalBytes > maxSize) { + throw new MalformedTemplateException("Vault template exceeds the maximum allowed size of " + maxSize + " bytes."); + } + + var actualTarget = Files.copy(file, resolveSafely(target, zipRoot, file), StandardCopyOption.REPLACE_EXISTING); + + if (Constants.VAULTCONFIG_FILENAME.equals(file.getFileName().toString())) { + if (vaultConfig != null) { + throw new MalformedTemplateException("Vault template contains more than one " + Constants.VAULTCONFIG_FILENAME); + } + vaultConfig = actualTarget; + } + return FileVisitResult.CONTINUE; + } + + private void countEntry() throws IOException { + totalEntries++; + if ( totalEntries > maxEntries) { + throw new MalformedTemplateException("Vault template contains more than the maximum allowed " + maxEntries + " entries."); } } - if (vaultConfig.get() == null) { - throw new IOException("Template does not contain a vault (no " + Constants.VAULTCONFIG_FILENAME + " found)."); - } - return vaultConfig.get().getParent(); - } - - private static void countEntry(int[] entryCount) throws IOException { - if (++entryCount[0] > MAX_ENTRIES) { - throw new IOException("Vault template contains more than the maximum allowed " + MAX_ENTRIES + " entries."); + Path getVaultConfig() { + return vaultConfig; } } private static Path resolveSafely(Path targetDir, Path zipRoot, Path entry) throws IOException { Path resolved = targetDir.resolve(zipRoot.relativize(entry).toString()).normalize(); if (!resolved.startsWith(targetDir)) { - throw new IOException("Refusing to extract entry outside of target directory: " + entry); + throw new MalformedTemplateException("Refusing to extract entry outside of target directory: " + entry); } return resolved; } @@ -174,4 +198,16 @@ public final class VaultTemplateExtractor { } } + public static class MalformedTemplateException extends IOException { + private static final long serialVersionUID = 1L; + + public MalformedTemplateException(String message) { + super(message); + } + + public MalformedTemplateException(String message, Throwable cause) { + super(message, cause); + } + } + } diff --git a/src/main/java/org/cryptomator/ui/dialogs/Dialogs.java b/src/main/java/org/cryptomator/ui/dialogs/Dialogs.java index 4933cefc7..a55fdfbd7 100644 --- a/src/main/java/org/cryptomator/ui/dialogs/Dialogs.java +++ b/src/main/java/org/cryptomator/ui/dialogs/Dialogs.java @@ -158,6 +158,17 @@ public class Dialogs { .setOkAction(Stage::close); } + public SimpleDialog.Builder prepareMalformedTemplateDialog(Stage window) { + return createDialogBuilder() // + .setOwner(window) // + .setTitleKey("importTemplate.title") // + .setMessageKey("importTemplate.malformedTemplate.message") // + .setDescriptionKey("importTemplate.malformedTemplate.description") // + .setIcon(FontAwesome5Icon.EXCLAMATION) // + .setOkButtonKey(BUTTON_KEY_CLOSE) // + .setOkAction(Stage::close); + } + public SimpleDialog.Builder prepareNoDDirectorySelectedDialog(Stage window) { return createDialogBuilder() // .setOwner(window) // diff --git a/src/main/java/org/cryptomator/ui/importtemplate/ImportTemplateLocationController.java b/src/main/java/org/cryptomator/ui/importtemplate/ImportTemplateLocationController.java index 70a83daa0..fad5c7613 100644 --- a/src/main/java/org/cryptomator/ui/importtemplate/ImportTemplateLocationController.java +++ b/src/main/java/org/cryptomator/ui/importtemplate/ImportTemplateLocationController.java @@ -8,11 +8,13 @@ import org.cryptomator.common.settings.Settings; import org.cryptomator.common.vaults.Vault; import org.cryptomator.common.vaults.VaultListManager; import org.cryptomator.launcher.VaultTemplateExtractor; +import org.cryptomator.launcher.VaultTemplateExtractor.MalformedTemplateException; import org.cryptomator.ui.common.FxController; import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlScene; import org.cryptomator.ui.common.Tasks; import org.cryptomator.ui.controls.FontAwesome5IconView; +import org.cryptomator.ui.dialogs.Dialogs; import org.cryptomator.ui.fxapp.FxApplicationWindows; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -69,6 +71,7 @@ public class ImportTemplateLocationController implements FxController { private final ExecutorService executor; private final Settings settings; private final ResourceBundle resourceBundle; + private final Dialogs dialogs; private final ObservableValue vaultPathStatus; private final ObservableValue validVaultPath; private final BooleanProperty usePresetPath; @@ -101,7 +104,8 @@ public class ImportTemplateLocationController implements FxController { VaultListManager vaultListManager, // ExecutorService executor, // Settings settings, // - ResourceBundle resourceBundle) { + ResourceBundle resourceBundle, // + Dialogs dialogs) { this.window = window; this.vaultName = vaultName; this.template = template; @@ -113,6 +117,7 @@ public class ImportTemplateLocationController implements FxController { this.executor = executor; this.settings = settings; this.resourceBundle = resourceBundle; + this.dialogs = dialogs; this.vaultPathStatus = ObservableUtil.mapWithDefault(vaultPath, this::validatePath, new VaultPathStatus(false, "error.message")); this.validVaultPath = ObservableUtil.mapWithDefault(vaultPathStatus, VaultPathStatus::valid, false); this.vaultPathStatus.addListener(this::updateStatusLabel); @@ -252,6 +257,12 @@ public class ImportTemplateLocationController implements FxController { vault.set(newVault); rememberParentDirectory(destination); window.setScene(successScene.get()); + }).onError(MalformedTemplateException.class, e -> { // must precede the IOException handler: Tasks picks the first matching one + LOG.error("Vault template is malformed.", e); + dialogs.prepareMalformedTemplateDialog(window).setOkAction(stage -> { + stage.close(); + window.close(); //TODO: before showing this dialog, close the window and use the mainWindow as owner + }).build().showAndWait(); }).onError(IOException.class, e -> { LOG.error("Failed to import vault template.", e); appWindows.showErrorWindow(e, window, window.getScene()); diff --git a/src/main/resources/fxml/import_template_location.fxml b/src/main/resources/fxml/import_template_location.fxml index 10dba486e..03915875a 100644 --- a/src/main/resources/fxml/import_template_location.fxml +++ b/src/main/resources/fxml/import_template_location.fxml @@ -30,10 +30,10 @@ - - + + diff --git a/src/main/resources/i18n/strings.properties b/src/main/resources/i18n/strings.properties index f1f7f0af3..a7d1bb423 100644 --- a/src/main/resources/i18n/strings.properties +++ b/src/main/resources/i18n/strings.properties @@ -670,7 +670,9 @@ retryIfReadonly.retry=Change and Retry # Import Template importTemplate.title=Setup Vault importTemplate.nameLabel=Vault name -importTemplate.createVaultBtn=Create Vault +importTemplate.createVaultBtn=Setup Vault +importTemplate.malformedTemplate.message=Invalid vault template +importTemplate.malformedTemplate.description=The linked vault template is damaged or unsupported. No vault has been created. Please download and import the vault template manually. importTemplate.success.message=Vault %s has been setup. importTemplate.success.unlockNow=Unlock Now diff --git a/src/test/java/org/cryptomator/launcher/VaultTemplateExtractorTest.java b/src/test/java/org/cryptomator/launcher/VaultTemplateExtractorTest.java index c8b3f109e..00a4565ae 100644 --- a/src/test/java/org/cryptomator/launcher/VaultTemplateExtractorTest.java +++ b/src/test/java/org/cryptomator/launcher/VaultTemplateExtractorTest.java @@ -1,5 +1,6 @@ package org.cryptomator.launcher; +import org.cryptomator.launcher.VaultTemplateExtractor.MalformedTemplateException; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -63,26 +64,34 @@ public class VaultTemplateExtractorTest { } @Test - @DisplayName("an archive without a vault config fails") - public void testNoVaultConfig(@TempDir Path tmp) throws IOException { - var zip = zip(Map.of("readme.txt", CONFIG)); + @DisplayName("data that is not a zip archive fails as malformed") + public void testNotAZip(@TempDir Path tmp) { + var notAZip = "this is not a zip archive".getBytes(StandardCharsets.UTF_8); - Assertions.assertThrows(IOException.class, () -> VaultTemplateExtractor.extractAndMove(zip, tmp.resolve("MyVault"))); + Assertions.assertThrows(MalformedTemplateException.class, () -> VaultTemplateExtractor.extractAndMove(notAZip, tmp.resolve("MyVault"))); } @Test - @DisplayName("an archive with more than one vault config fails") + @DisplayName("an archive without a vault config fails as malformed") + public void testNoVaultConfig(@TempDir Path tmp) throws IOException { + var zip = zip(Map.of("readme.txt", CONFIG)); + + Assertions.assertThrows(MalformedTemplateException.class, () -> VaultTemplateExtractor.extractAndMove(zip, tmp.resolve("MyVault"))); + } + + @Test + @DisplayName("an archive with more than one vault config fails as malformed") public void testMultipleVaultConfigs(@TempDir Path tmp) throws IOException { var zip = zip(Map.of( // "vault.cryptomator", CONFIG, // "nested/vault.cryptomator", CONFIG // )); - Assertions.assertThrows(IOException.class, () -> VaultTemplateExtractor.extractAndMove(zip, tmp.resolve("MyVault"))); + Assertions.assertThrows(MalformedTemplateException.class, () -> VaultTemplateExtractor.extractAndMove(zip, tmp.resolve("MyVault"))); } @Test - @DisplayName("an archive exceeding the size limit fails") + @DisplayName("an archive exceeding the size limit fails as malformed") public void testExceedsSizeLimit(@TempDir Path tmp) throws IOException { var big = new byte[2 * 1024 * 1024 + 1]; var zip = zip(Map.of( // @@ -90,20 +99,20 @@ public class VaultTemplateExtractorTest { "d/AB/CDEF/0.c9r", big // )); - Assertions.assertThrows(IOException.class, () -> VaultTemplateExtractor.extractAndMove(zip, tmp.resolve("MyVault"))); + Assertions.assertThrows(MalformedTemplateException.class, () -> VaultTemplateExtractor.extractAndMove(zip, tmp.resolve("MyVault"))); } @Test - @DisplayName("an archive exceeding the entry-count limit fails") + @DisplayName("an archive exceeding the entry-count limit fails as malformed") public void testExceedsEntryLimit(@TempDir Path tmp) throws IOException { var entries = new LinkedHashMap(); entries.put("vault.cryptomator", CONFIG); - for (int i = 0; i < 11; i++) { + for (int i = 0; i < 31; i++) { entries.put("file" + i + ".c9r", CIPHERTEXT); } var zip = zip(entries); - Assertions.assertThrows(IOException.class, () -> VaultTemplateExtractor.extractAndMove(zip, tmp.resolve("MyVault"))); + Assertions.assertThrows(MalformedTemplateException.class, () -> VaultTemplateExtractor.extractAndMove(zip, tmp.resolve("MyVault"))); } @Test