diff options
author | Tomasz Wrobel <tomasz.wrobel@nokia.com> | 2021-03-18 15:03:51 +0100 |
---|---|---|
committer | Piotr Marcinkiewicz <piotr.marcinkiewicz@nokia.com> | 2021-03-19 08:54:08 +0000 |
commit | 92c83fe593132a522cb26a872d94612373e46315 (patch) | |
tree | be8cb71d333a3b6ba7d19b26015463e5ed6e3e2c | |
parent | 6df36c389d23a73f46fafb8cc1b70e510bdfbaeb (diff) |
Fix sonar issues
-Fix thread interrupt bug
-Fix security vulnerabilities
Issue-ID: SDC-3185
Signed-off-by: Tomasz Wrobel <tomasz.wrobel@nokia.com>
Change-Id: I0e32215a6de9e04a26acfad580701b36278270b0
5 files changed, 45 insertions, 18 deletions
diff --git a/src/main/java/org/onap/sdc/helmvalidator/helm/validation/BashExecutor.java b/src/main/java/org/onap/sdc/helmvalidator/helm/validation/BashExecutor.java index 066e731..e30659b 100644 --- a/src/main/java/org/onap/sdc/helmvalidator/helm/validation/BashExecutor.java +++ b/src/main/java/org/onap/sdc/helmvalidator/helm/validation/BashExecutor.java @@ -41,15 +41,18 @@ public class BashExecutor { BashOutput execute(String helmCommand) { try { - ProcessBuilder pb = new ProcessBuilder("bash", "-c", helmCommand); + ProcessBuilder pb = new ProcessBuilder("/bin/bash", "-c", helmCommand); pb.redirectErrorStream(true); LOGGER.debug("Start process"); Process process = pb.start(); List<String> processOutput = readOutputAndCloseProcess(process); return new BashOutput(process.exitValue(), processOutput); - } catch (IOException | InterruptedException e) { + } catch (IOException e) { throw new BashExecutionException("Error during bash execution: ", e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new BashExecutionException("Bash execution interrupted, error: ", e); } } diff --git a/src/main/java/org/onap/sdc/helmvalidator/helm/validation/FileManager.java b/src/main/java/org/onap/sdc/helmvalidator/helm/validation/FileManager.java index 3617df7..4bc8ea6 100644 --- a/src/main/java/org/onap/sdc/helmvalidator/helm/validation/FileManager.java +++ b/src/main/java/org/onap/sdc/helmvalidator/helm/validation/FileManager.java @@ -47,6 +47,7 @@ public class FileManager { String saveFile(MultipartFile file) { LOGGER.debug("Base PATH: {}", basePath); + try { String filePath = basePath + File.separator + generateFileName(file.getOriginalFilename()); LOGGER.info("Attempt to save file : {}", filePath); @@ -67,6 +68,16 @@ public class FileManager { } private String generateFileName(String fileName) { - return Instant.now().toEpochMilli() + "_" + fileName; + if (isValidFileName(fileName)) { + return Instant.now().toEpochMilli() + "_" + fileName.replaceAll("\\s+", ""); + } + throw new SaveFileException("Not allowed file name"); + } + + private boolean isValidFileName(String fileName) { + if (fileName == null){ + return false; + } + return !fileName.contains("/"); } } diff --git a/src/main/java/org/onap/sdc/helmvalidator/helm/validation/ValidationService.java b/src/main/java/org/onap/sdc/helmvalidator/helm/validation/ValidationService.java index 02f28a3..53689c2 100644 --- a/src/main/java/org/onap/sdc/helmvalidator/helm/validation/ValidationService.java +++ b/src/main/java/org/onap/sdc/helmvalidator/helm/validation/ValidationService.java @@ -99,16 +99,6 @@ public class ValidationService { } private String getSupportedHelmVersion(String desiredVersion, String chartPath) { - String helmVersion = getHelmVersion(desiredVersion, chartPath); - - if (isNotSupportedVersion(helmVersion)) { - throw new NotSupportedVersionException(helmVersion); - } - - return helmVersion; - } - - private String getHelmVersion(String desiredVersion, String chartPath) { if (desiredVersion == null) { return chartBasedVersionProvider.getVersion(chartPath); } @@ -118,7 +108,11 @@ public class ValidationService { return supportedVersionsProvider.getLatestVersion(helmMajorVersion); } - return desiredVersion; + return supportedVersionsProvider.getVersions() + .stream() + .filter(s -> s.equals(desiredVersion)) + .findFirst() + .orElseThrow(() -> new NotSupportedVersionException(desiredVersion)); } private ValidationResult validateChart(String version, MultipartFile file, boolean isLinted, boolean isStrictLinted, @@ -141,10 +135,6 @@ public class ValidationService { return new ValidationResult(templateValidationResult, version); } - private boolean isNotSupportedVersion(String version) { - return !supportedVersionsProvider.getVersions().contains(version); - } - private String buildHelmTemplateCommand(String version, String chartPath) { return "helm-v" + version + " " + TEMPLATE_OPTION + " " + chartPath; diff --git a/src/main/java/org/onap/sdc/helmvalidator/helm/validation/exception/SaveFileException.java b/src/main/java/org/onap/sdc/helmvalidator/helm/validation/exception/SaveFileException.java index cdb9079..59e5b10 100644 --- a/src/main/java/org/onap/sdc/helmvalidator/helm/validation/exception/SaveFileException.java +++ b/src/main/java/org/onap/sdc/helmvalidator/helm/validation/exception/SaveFileException.java @@ -25,4 +25,8 @@ public class SaveFileException extends RuntimeException { public SaveFileException(String message, Throwable cause) { super(message, cause); } + + public SaveFileException(String message) { + super(message); + } } diff --git a/src/test/java/org/onap/sdc/helmvalidator/helm/validation/FileManagerTest.java b/src/test/java/org/onap/sdc/helmvalidator/helm/validation/FileManagerTest.java index 2d05cd9..89eab80 100644 --- a/src/test/java/org/onap/sdc/helmvalidator/helm/validation/FileManagerTest.java +++ b/src/test/java/org/onap/sdc/helmvalidator/helm/validation/FileManagerTest.java @@ -21,6 +21,7 @@ package org.onap.sdc.helmvalidator.helm.validation; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.when; import java.io.ByteArrayInputStream; @@ -37,6 +38,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.onap.sdc.helmvalidator.helm.validation.exception.SaveFileException; import org.springframework.web.multipart.MultipartFile; @ExtendWith(MockitoExtension.class) @@ -93,6 +95,23 @@ class FileManagerTest { assertThat(Files.exists(Paths.get(filePath))).isFalse(); } + @Test + void shouldThrowExceptionWhenFileContainsSlash() { + when(multipartFile.getOriginalFilename()).thenReturn(SAMPLE_FILE_NAME + "/"); + Exception exception = assertThrows(SaveFileException.class, + () -> fileManager.saveFile(multipartFile)); + assertThat(exception).hasMessageContaining("Not allowed file name"); + } + + @Test + void shouldThrowExceptionWhenFileNameIsNull() { + when(multipartFile.getOriginalFilename()).thenReturn(null); + + Exception exception = assertThrows(SaveFileException.class, + () -> fileManager.saveFile(multipartFile)); + assertThat(exception).hasMessageContaining("Not allowed file name"); + } + private void mockMultipartFile() throws IOException { when(multipartFile.getOriginalFilename()).thenReturn(SAMPLE_FILE_NAME); when(multipartFile.getInputStream()).thenReturn(TEST_INPUT_STREAM); |