fix: file param validation (#808)

crowdin · May 23, 2024 · df576ef · df576ef
1 parent 407150a
commit df576ef
Show file tree

Hide file tree

Showing 16 changed files with 73 additions and 16 deletions.
diff --git a/src/main/java/com/crowdin/cli/client/CrowdinProjectClient.java b/src/main/java/com/crowdin/cli/client/CrowdinProjectClient.java
@@ -1,5 +1,6 @@
 package com.crowdin.cli.client;
 
+import com.crowdin.cli.commands.picocli.ExitCodeExceptionMapper;
 import com.crowdin.cli.utils.Utils;
 import com.crowdin.client.branches.model.*;
 import com.crowdin.client.core.model.PatchRequest;
@@ -69,7 +70,7 @@ private void populateProjectWithStructure(CrowdinProjectFull project, String bra
         project.setBranches(this.listBranches());
         Optional.ofNullable(branchName)
                 .map(name -> project.findBranchByName(name)
-                        .orElseThrow(() -> new RuntimeException(RESOURCE_BUNDLE.getString("error.not_found_branch")))
+                        .orElseThrow(() -> new ExitCodeExceptionMapper.NotFoundException(RESOURCE_BUNDLE.getString("error.not_found_branch")))
                 )
                 .ifPresent(project::setBranch);
         Long branchId = Optional.ofNullable(project.getBranch()).map(Branch::getId).orElse(null);

diff --git a/src/main/java/com/crowdin/cli/commands/actions/FileDeleteAction.java b/src/main/java/com/crowdin/cli/commands/actions/FileDeleteAction.java
@@ -4,6 +4,7 @@
 import com.crowdin.cli.client.ProjectClient;
 import com.crowdin.cli.commands.NewAction;
 import com.crowdin.cli.commands.Outputter;
+import com.crowdin.cli.commands.picocli.ExitCodeExceptionMapper;
 import com.crowdin.cli.properties.ProjectProperties;
 import com.crowdin.cli.utils.Utils;
 import com.crowdin.cli.utils.console.ConsoleSpinner;
@@ -41,7 +42,7 @@ public void act(Outputter out, ProjectProperties properties, ProjectClient clien
         FileInfo foundFile = projectFiles.stream()
             .filter(f -> Objects.equals(filePath, f.getPath()))
             .findFirst()
-            .orElseThrow(() -> new RuntimeException(String.format(RESOURCE_BUNDLE.getString("error.file_not_found"), filePath)));
+            .orElseThrow(() -> new ExitCodeExceptionMapper.NotFoundException(String.format(RESOURCE_BUNDLE.getString("error.file_not_found"), filePath)));
         client.deleteSource(foundFile.getId());
         out.println(ExecutionStatus.OK.withIcon(String.format(RESOURCE_BUNDLE.getString("message.file_deleted"), filePath)));
     }

diff --git a/src/main/java/com/crowdin/cli/commands/actions/FileDownloadAction.java b/src/main/java/com/crowdin/cli/commands/actions/FileDownloadAction.java
@@ -6,6 +6,7 @@
 import com.crowdin.cli.commands.Outputter;
 import com.crowdin.cli.commands.functionality.FilesInterface;
 import com.crowdin.cli.commands.functionality.FsFiles;
+import com.crowdin.cli.commands.picocli.ExitCodeExceptionMapper;
 import com.crowdin.cli.properties.ProjectProperties;
 import com.crowdin.cli.utils.Utils;
 import com.crowdin.cli.utils.console.ConsoleSpinner;
@@ -50,7 +51,7 @@ public void act(Outputter out, ProjectProperties properties, ProjectClient clien
         FileInfo foundFile = project.getFileInfos().stream()
             .filter(f -> Objects.equals(filePath, f.getPath()))
             .findFirst()
-            .orElseThrow(() -> new RuntimeException(String.format(RESOURCE_BUNDLE.getString("error.file_not_found"), filePath)));
+            .orElseThrow(() -> new ExitCodeExceptionMapper.NotFoundException(String.format(RESOURCE_BUNDLE.getString("error.file_not_found"), filePath)));
         ConsoleSpinner.execute(
             out,
             "message.spinner.downloading_file",

diff --git a/src/main/java/com/crowdin/cli/commands/actions/FileDownloadTranslationAction.java b/src/main/java/com/crowdin/cli/commands/actions/FileDownloadTranslationAction.java
@@ -6,6 +6,7 @@
 import com.crowdin.cli.commands.Outputter;
 import com.crowdin.cli.commands.functionality.FilesInterface;
 import com.crowdin.cli.commands.functionality.FsFiles;
+import com.crowdin.cli.commands.picocli.ExitCodeExceptionMapper;
 import com.crowdin.cli.properties.ProjectProperties;
 import com.crowdin.cli.utils.PlaceholderUtil;
 import com.crowdin.cli.utils.Utils;
@@ -67,7 +68,7 @@ public void act(Outputter out, ProjectProperties properties, ProjectClient clien
         FileInfo sourceFileInfo = project.getFileInfos().stream()
             .filter(fi -> Objects.equals(sourcePath, fi.getPath()))
             .findFirst()
-            .orElseThrow(() -> new RuntimeException(String.format(RESOURCE_BUNDLE.getString("error.file_not_found"), sourcePath)));
+            .orElseThrow(() -> new ExitCodeExceptionMapper.NotFoundException(String.format(RESOURCE_BUNDLE.getString("error.file_not_found"), sourcePath)));
         PlaceholderUtil placeholderUtil = new PlaceholderUtil(
             project.getSupportedLanguages(),
             project.getProjectLanguages(true),

diff --git a/src/main/java/com/crowdin/cli/commands/actions/FileUploadTranslationAction.java b/src/main/java/com/crowdin/cli/commands/actions/FileUploadTranslationAction.java
@@ -62,7 +62,7 @@ public void act(Outputter out, ProjectProperties properties, ProjectClient clien
             FileInfo sourceFileInfo = project.getFileInfos().stream()
                 .filter(fi -> Objects.equals(sourcePath, fi.getPath()))
                 .findFirst()
-                .orElseThrow(() -> new RuntimeException(String.format(RESOURCE_BUNDLE.getString("error.file_not_found"), sourcePath)));
+                .orElseThrow(() -> new ExitCodeExceptionMapper.NotFoundException(String.format(RESOURCE_BUNDLE.getString("error.file_not_found"), sourcePath)));
 
             UploadTranslationsRequest request = new UploadTranslationsRequest();
             Long storageId = getStorageId(client);

diff --git a/src/main/java/com/crowdin/cli/commands/actions/StatusAction.java b/src/main/java/com/crowdin/cli/commands/actions/StatusAction.java
@@ -5,6 +5,7 @@
 import com.crowdin.cli.commands.NewAction;
 import com.crowdin.cli.commands.Outputter;
 import com.crowdin.cli.commands.functionality.BranchUtils;
+import com.crowdin.cli.commands.picocli.ExitCodeExceptionMapper;
 import com.crowdin.cli.properties.ProjectProperties;
 import com.crowdin.cli.utils.Utils;
 import com.crowdin.cli.utils.console.ConsoleSpinner;
@@ -45,7 +46,7 @@ public void act(Outputter out, ProjectProperties pb, ProjectClient client) {
 
         List<Branch> branches = client.listBranches();
         Long branchId = (branchName == null) ? null : BranchUtils.getBranch(branchName, branches)
-            .orElseThrow(() -> new RuntimeException(RESOURCE_BUNDLE.getString("error.not_found_branch")))
+            .orElseThrow(() -> new ExitCodeExceptionMapper.NotFoundException(RESOURCE_BUNDLE.getString("error.not_found_branch")))
             .getId();
 
         List<LanguageProgress> progresses;

diff --git a/src/main/java/com/crowdin/cli/commands/functionality/FsFiles.java b/src/main/java/com/crowdin/cli/commands/functionality/FsFiles.java
@@ -1,5 +1,6 @@
 package com.crowdin.cli.commands.functionality;
 
+import com.crowdin.cli.commands.picocli.ExitCodeExceptionMapper;
 import com.crowdin.cli.utils.file.FileUtils;
 import net.lingala.zip4j.ZipFile;
 import net.lingala.zip4j.model.FileHeader;
@@ -40,7 +41,7 @@ public List<File> extractZipArchive(File zipArchive, File dir) {
         try {
             zipFile = new ZipFile(zipArchive);
         } catch (IllegalArgumentException e) {
-            throw new RuntimeException(String.format(RESOURCE_BUNDLE.getString("error.archive_not_exist"), zipArchive.getAbsolutePath()));
+            throw new ExitCodeExceptionMapper.NotFoundException(String.format(RESOURCE_BUNDLE.getString("error.archive_not_exist"), zipArchive.getAbsolutePath()));
         }
         if (!dir.exists()) {
             try {
@@ -80,7 +81,7 @@ public List<String> zipArchiveContent(File zipArchive) {
         try {
             zipFile = new ZipFile(zipArchive);
         } catch (IllegalArgumentException e) {
-            throw new RuntimeException(String.format(RESOURCE_BUNDLE.getString("error.archive_not_exist"), zipArchive.getAbsolutePath()));
+            throw new ExitCodeExceptionMapper.NotFoundException(String.format(RESOURCE_BUNDLE.getString("error.archive_not_exist"), zipArchive.getAbsolutePath()));
         }
         try {
             List<FileHeader> fileHeaders = zipFile.getFileHeaders();

diff --git a/src/main/java/com/crowdin/cli/commands/picocli/ConfigurationFilesProperties.java b/src/main/java/com/crowdin/cli/commands/picocli/ConfigurationFilesProperties.java
@@ -24,6 +24,8 @@ public File getConfigFile() {
             configFile = this.getDefaultConfigFile();
         } else if (!configFile.exists()) {
             throw new ExitCodeExceptionMapper.NotFoundException(RESOURCE_BUNDLE.getString("error.configuration_file_not_exist"));
+        } else if (configFile.isDirectory()) {
+            throw new ExitCodeExceptionMapper.ValidationException(RESOURCE_BUNDLE.getString("error.file.is_folder"));
         }
         return configFile;
     }
@@ -33,6 +35,8 @@ public File getIdentityFile() {
             identityFile = this.getDefaultIdentityFile();
         } else if (!identityFile.exists()) {
             throw new ExitCodeExceptionMapper.NotFoundException(String.format(RESOURCE_BUNDLE.getString("error.identity_file_not_exist"), identityFile.getAbsolutePath()));
+        } else if (configFile.isDirectory()) {
+            throw new ExitCodeExceptionMapper.ValidationException(RESOURCE_BUNDLE.getString("error.file.is_folder"));
         }
         return identityFile;
     }

diff --git a/src/main/java/com/crowdin/cli/commands/picocli/FileUploadSubcommand.java b/src/main/java/com/crowdin/cli/commands/picocli/FileUploadSubcommand.java
@@ -60,10 +60,16 @@ class FileUploadSubcommand extends ActCommandProject {
 
     @Override
     protected List<String> checkOptions() {
+        if (!file.exists()) {
+            throw new ExitCodeExceptionMapper.NotFoundException(String.format(RESOURCE_BUNDLE.getString("error.file_not_found"), file));
+        }
         List<String> errors = new ArrayList<>();
         if (parserVersion != null && type == null) {
             errors.add(RESOURCE_BUNDLE.getString("error.file.type_required"));
         }
+        if (file.isDirectory()) {
+            errors.add(RESOURCE_BUNDLE.getString("error.file.is_folder"));
+        }
         return errors;
     }
 

diff --git a/src/main/java/com/crowdin/cli/commands/picocli/GlossaryUploadSubcommand.java b/src/main/java/com/crowdin/cli/commands/picocli/GlossaryUploadSubcommand.java
@@ -46,7 +46,10 @@ protected NewAction<BaseProperties, ClientGlossary> getAction(Actions actions) {
     protected List<String> checkOptions() {
         List<String> errors = new ArrayList<>();
         if (!file.exists()) {
-            errors.add(String.format("File '%s' doesn't exist", file));
+            throw new ExitCodeExceptionMapper.NotFoundException(String.format(RESOURCE_BUNDLE.getString("error.file_not_found"), file));
+        }
+        if (file.isDirectory()) {
+            errors.add(RESOURCE_BUNDLE.getString("error.file.is_folder"));
         }
         if (!equalsAny(FilenameUtils.getExtension(file.getName()), "tbx", "csv", "xls", "xlsx")) {
             errors.add(RESOURCE_BUNDLE.getString("error.glossary.wrong_format"));

diff --git a/src/main/java/com/crowdin/cli/commands/picocli/ScreenshotUploadSubcommand.java b/src/main/java/com/crowdin/cli/commands/picocli/ScreenshotUploadSubcommand.java
@@ -52,8 +52,14 @@ protected NewAction<ProjectProperties, ClientScreenshot> getAction(Actions actio
 
     @Override
     protected List<String> checkOptions() {
+        if (!file.exists()) {
+            throw new ExitCodeExceptionMapper.NotFoundException(String.format(RESOURCE_BUNDLE.getString("error.file_not_found"), file));
+        }
         List<String> errors = new ArrayList<>();
         String extension = FilenameUtils.getExtension(file.getName());
+        if (file.isDirectory()) {
+            errors.add(RESOURCE_BUNDLE.getString("error.file.is_folder"));
+        }
         if (!equalsAny(extension, "jpeg", "jpg", "png", "gif")) {
             errors.add(RESOURCE_BUNDLE.getString("error.screenshot.wrong_format"));
         }

diff --git a/src/main/java/com/crowdin/cli/commands/picocli/TmUploadSubcommand.java b/src/main/java/com/crowdin/cli/commands/picocli/TmUploadSubcommand.java
@@ -46,6 +46,12 @@ protected NewAction<BaseProperties, ClientTm> getAction(Actions actions) {
     protected List<String> checkOptions() {
         List<String> errors = new ArrayList<>();
         String extension = FilenameUtils.getExtension(file.getName());
+        if (!file.exists()) {
+            throw new ExitCodeExceptionMapper.NotFoundException(String.format(RESOURCE_BUNDLE.getString("error.file_not_found"), file));
+        }
+        if (file.isDirectory()) {
+            errors.add(RESOURCE_BUNDLE.getString("error.file.is_folder"));
+        }
         if (scheme == null && ("csv".equalsIgnoreCase(extension) || "xslx".equalsIgnoreCase(extension))) {
             errors.add(RESOURCE_BUNDLE.getString("error.tm.scheme_is_required"));
         }

diff --git a/src/main/resources/messages/messages.properties b/src/main/resources/messages/messages.properties
@@ -583,6 +583,7 @@ error.label.not_found=Couldn't find label by the specified title
 
 error.file.dest_required='dest' parameter required to specify source file path
 error.file.type_required='--type' is required for '--parser-version' option
+error.file.is_folder=The specified file is a directory
 error.file.context_file_based_only='--context' parameter is only available for file-based projects
 
 error.branch.clone=Failed to clone the branch

diff --git a/src/test/java/com/crowdin/cli/commands/picocli/FileUploadSubcommandTest.java b/src/test/java/com/crowdin/cli/commands/picocli/FileUploadSubcommandTest.java
@@ -2,22 +2,27 @@
 
 import org.junit.jupiter.api.Test;
 
+import java.net.URISyntaxException;
+import java.nio.file.Path;
+
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.Mockito.verify;
 
 class FileUploadSubcommandTest extends PicocliTestUtils {
 
     @Test
-    public void testFileUpload() {
-        this.execute(CommandNames.FILE, CommandNames.FILE_UPLOAD, "file.txt");
+    public void testFileUpload() throws URISyntaxException {
+        var file = Path.of(this.getClass().getClassLoader().getResource("file.txt").toURI()).toFile();
+        this.execute(CommandNames.FILE, CommandNames.FILE_UPLOAD, file.getAbsolutePath());
         verify(actionsMock).fileUpload(any(), any(), anyBoolean(), any(), any(), any(), any(), any(), any(), anyBoolean(), anyBoolean(), anyBoolean());
         this.check(true);
     }
 
     @Test
-    public void testFileUploadTranslations() {
-        this.execute(CommandNames.FILE, CommandNames.FILE_UPLOAD, "file.txt", "--language", "uk");
+    public void testFileUploadTranslations() throws URISyntaxException {
+        var file = Path.of(this.getClass().getClassLoader().getResource("file.txt").toURI()).toFile();
+        this.execute(CommandNames.FILE, CommandNames.FILE_UPLOAD, file.getAbsolutePath(), "--language", "uk");
         verify(actionsMock).fileUploadTranslation(any(), any(), any(), any(), anyBoolean());
         this.check(true);
     }

diff --git a/src/test/java/com/crowdin/cli/commands/picocli/ScreenshotUploadSubcommandTest.java b/src/test/java/com/crowdin/cli/commands/picocli/ScreenshotUploadSubcommandTest.java
@@ -16,6 +16,7 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.params.provider.Arguments.arguments;
+import static org.mockito.Mockito.*;
 
 public class ScreenshotUploadSubcommandTest extends PicocliTestUtils {
 
@@ -28,14 +29,21 @@ public void testScreenshotUploadInvalidOptions() {
     @MethodSource
     public void testSubCommandCheckValidOptions(File file, boolean autoTag, String filePath, String branchName, String directoryPath) {
         ScreenshotUploadSubcommand screenshotUploadSubcommand = new ScreenshotUploadSubcommand();
-        screenshotUploadSubcommand.file = file;
+        var mockedFile = mock(File.class);
+        screenshotUploadSubcommand.file = mockedFile;
         screenshotUploadSubcommand.autoTag = autoTag;
         screenshotUploadSubcommand.filePath = filePath;
         screenshotUploadSubcommand.branchName = branchName;
         screenshotUploadSubcommand.directoryPath = directoryPath;
 
+        when(mockedFile.exists()).thenReturn(true);
+        when(mockedFile.getName()).thenReturn(file.getName());
+
         List<String> errors = screenshotUploadSubcommand.checkOptions();
         assertEquals(Collections.emptyList(), errors);
+
+        verify(mockedFile).exists();
+        verify(mockedFile).getName();
     }
 
     public static Stream<Arguments> testSubCommandCheckValidOptions() {
@@ -50,14 +58,21 @@ public static Stream<Arguments> testSubCommandCheckValidOptions() {
     @MethodSource
     public void testSubCommandCheckInvalidOptions(File file, boolean autoTag, String filePath, String branchName, String directoryPath, List<String> expErrors) {
         ScreenshotUploadSubcommand screenshotUploadSubcommand = new ScreenshotUploadSubcommand();
-        screenshotUploadSubcommand.file = file;
+        var mockedFile = mock(File.class);
+        screenshotUploadSubcommand.file = mockedFile;
         screenshotUploadSubcommand.autoTag = autoTag;
         screenshotUploadSubcommand.filePath = filePath;
         screenshotUploadSubcommand.branchName = branchName;
         screenshotUploadSubcommand.directoryPath = directoryPath;
 
+        when(mockedFile.exists()).thenReturn(true);
+        when(mockedFile.getName()).thenReturn(file.getName());
+
         List<String> errors = screenshotUploadSubcommand.checkOptions();
         assertThat(errors, Matchers.equalTo(expErrors));
+
+        verify(mockedFile).exists();
+        verify(mockedFile).getName();
     }
 
     public static Stream<Arguments> testSubCommandCheckInvalidOptions() {

diff --git a/src/test/java/com/crowdin/cli/commands/picocli/TmUploadSubcommandTest.java b/src/test/java/com/crowdin/cli/commands/picocli/TmUploadSubcommandTest.java
@@ -1,15 +1,20 @@
 package com.crowdin.cli.commands.picocli;
 
+import lombok.SneakyThrows;
 import org.junit.jupiter.api.Test;
 
+import java.nio.file.Path;
+
 import static org.mockito.ArgumentMatchers.*;
 import static org.mockito.Mockito.verify;
 
 public class TmUploadSubcommandTest extends PicocliTestUtils {
 
+    @SneakyThrows
     @Test
     public void testTmUpload() {
-        this.execute(CommandNames.TM, CommandNames.TM_UPLOAD, "file.tmx", "--id", "42", "--debug");
+        var file = Path.of(this.getClass().getClassLoader().getResource("file.tmx").toURI()).toFile();
+        this.execute(CommandNames.TM, CommandNames.TM_UPLOAD, file.getAbsolutePath(), "--id", "42", "--debug");
         verify(actionsMock)
             .tmUpload(any(), eq(42L), isNull(), isNull(), isNull(), anyBoolean());
         this.check(true);