From 20d84a10eb349b0c90169f9308a31cbd0a83d357 Mon Sep 17 00:00:00 2001 From: Stefan Klotz <35995139+StefanKlt@users.noreply.github.com> Date: Mon, 17 Dec 2018 10:17:46 +0100 Subject: [PATCH] Fix artifact filename validation (#770) * use validated ArtifactUpload object when creating a new artifact Signed-off-by: Stefan Klotz * rename method Signed-off-by: Stefan Klotz * add regular expression classes Signed-off-by: Stefan Klotz * add filename validation to UI upload button Signed-off-by: Stefan Klotz * move filename validation to uploadStarted Signed-off-by: Stefan Klotz * clean up code for UI error handling during artifact upload, assert filename validation Signed-off-by: Stefan Klotz * update visibilities Signed-off-by: Stefan Klotz * clean up code Signed-off-by: Stefan Klotz * clean up code Signed-off-by: Stefan Klotz * change RegexChar class to enum and use i18n Signed-off-by: Stefan Klotz * typo, use StringBuilder Signed-off-by: Stefan Klotz * typo Signed-off-by: Stefan Klotz * use dedicated class for collections of regular expression characters Signed-off-by: Stefan Klotz * remove Optional, remove stringBuilder Signed-off-by: Stefan Klotz * PR findings Signed-off-by: Stefan Klotz * make regex validation method static Signed-off-by: Stefan Klotz * use WhiteListType.NONE for filename validation via mgmt-api Signed-off-by: Stefan Klotz --- .../repository/ArtifactManagement.java | 55 ++------ .../repository/RegexCharacterCollection.java | 67 +++++++++ .../repository/model/ArtifactUpload.java | 129 ++++++++++++++++++ .../hawkbit/repository/RegexCharTest.java | 109 +++++++++++++++ .../repository/jpa/JpaArtifactManagement.java | 29 ++-- .../jpa/ArtifactManagementTest.java | 31 ++++- .../jpa/ControllerManagementTest.java | 9 +- .../jpa/SoftwareModuleManagementTest.java | 17 ++- .../repository/jpa/SystemManagementTest.java | 7 +- .../repository/test/util/TestdataFactory.java | 4 +- .../resource/DdiArtifactDownloadTest.java | 17 +-- .../rest/resource/DdiDeploymentBaseTest.java | 25 ++-- .../resource/MgmtSoftwareModuleResource.java | 5 +- .../MgmtSoftwareModuleResourceTest.java | 53 ++++--- .../RootControllerDocumentationTest.java | 16 ++- .../AbstractApiRestDocumentation.java | 4 +- .../SoftwaremodulesDocumentationTest.java | 15 +- .../upload/AbstractFileTransferHandler.java | 63 +++++---- .../FileTransferHandlerStreamVariable.java | 30 ++-- .../FileTransferHandlerVaadinUpload.java | 66 ++++----- .../upload/UploadDropAreaLayout.java | 3 +- .../src/main/resources/messages.properties | 7 + 22 files changed, 536 insertions(+), 225 deletions(-) create mode 100644 hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RegexCharacterCollection.java create mode 100644 hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/ArtifactUpload.java create mode 100644 hawkbit-repository/hawkbit-repository-api/src/test/java/org/eclipse/hawkbit/repository/RegexCharTest.java diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/ArtifactManagement.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/ArtifactManagement.java index 02822011b..b03b3d133 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/ArtifactManagement.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/ArtifactManagement.java @@ -8,9 +8,10 @@ */ package org.eclipse.hawkbit.repository; -import java.io.InputStream; import java.util.Optional; +import javax.validation.ConstraintViolationException; +import javax.validation.Valid; import javax.validation.constraints.NotEmpty; import javax.validation.constraints.NotNull; @@ -23,6 +24,7 @@ import org.eclipse.hawkbit.repository.exception.EntityNotFoundException; import org.eclipse.hawkbit.repository.exception.InvalidMD5HashException; import org.eclipse.hawkbit.repository.exception.InvalidSHA1HashException; import org.eclipse.hawkbit.repository.model.Artifact; +import org.eclipse.hawkbit.repository.model.ArtifactUpload; import org.eclipse.hawkbit.repository.model.SoftwareModule; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; @@ -45,50 +47,8 @@ public interface ArtifactManagement { * Persists artifact binary as provided by given InputStream. assign the * artifact in addition to given {@link SoftwareModule}. * - * @param inputStream - * to read from for artifact binary - * @param moduleId - * to assign the new artifact to - * @param filename - * of the artifact - * @param overrideExisting - * to true if the artifact binary can be overridden - * if it already exists - * @param filesize - * the size of the file in bytes. - * - * @return uploaded {@link Artifact} - * - * @throws ArtifactUploadFailedException - * if upload fails - * @throws EntityNotFoundException - * if given software module does not exist - */ - @PreAuthorize(SpringEvalExpressions.HAS_AUTH_CREATE_REPOSITORY) - Artifact create(@NotNull InputStream inputStream, long moduleId, final String filename, - final boolean overrideExisting, final long filesize); - - /** - * Persists artifact binary as provided by given InputStream. assign the - * artifact in addition to given {@link SoftwareModule}. - * - * @param stream - * to read from for artifact binary - * @param moduleId - * to assign the new artifact to - * @param filename - * of the artifact - * @param providedSha1Sum - * optional sha1 checksum to check the new file against - * @param providedMd5Sum - * optional md5 checksum to check the new file against - * @param overrideExisting - * to true if the artifact binary can be overridden - * if it already exists - * @param contentType - * the contentType of the file - * @param filesize - * the size of the file in bytes. + * @param artifactUpload + * {@link ArtifactUpload} containing the upload information * * @return uploaded {@link Artifact} * @@ -102,10 +62,11 @@ public interface ArtifactManagement { * if check against provided MD5 checksum failed * @throws InvalidSHA1HashException * if check against provided SHA1 checksum failed + * @throws ConstraintViolationException + * if {@link ArtifactUpload} contains invalid values */ @PreAuthorize(SpringEvalExpressions.HAS_AUTH_CREATE_REPOSITORY) - Artifact create(@NotNull InputStream stream, long moduleId, @NotEmpty String filename, String providedMd5Sum, - String providedSha1Sum, boolean overrideExisting, String contentType, long filesize); + Artifact create(@NotNull @Valid ArtifactUpload artifactUpload); /** * Garbage collects artifact binaries if only referenced by given diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RegexCharacterCollection.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RegexCharacterCollection.java new file mode 100644 index 000000000..1d1f48793 --- /dev/null +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RegexCharacterCollection.java @@ -0,0 +1,67 @@ +/** + * Copyright (c) 2015 Bosch Software Innovations GmbH and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + */ +package org.eclipse.hawkbit.repository; + +import java.util.Arrays; +import java.util.EnumSet; +import java.util.Optional; +import java.util.regex.Pattern; +import java.util.stream.Collectors; + +/** + * Collection of regular expression characters to check strings + */ +public class RegexCharacterCollection { + + private final EnumSet characters; + private final Pattern findAnyCharacter; + + public RegexCharacterCollection(final RegexChar... characters) { + this.characters = EnumSet.copyOf(Arrays.asList(characters)); + this.findAnyCharacter = getPatternFindAnyCharacter(); + } + + public static boolean stringContainsCharacter(final String stringToCheck, + final RegexCharacterCollection regexCharacterCollection) { + return regexCharacterCollection.findAnyCharacter.matcher(stringToCheck).matches(); + } + + private Pattern getPatternFindAnyCharacter() { + final String regexCharacters = characters.stream().map(RegexChar::getRegExp) + .collect(Collectors.joining()); + final String regularExpression = String.format(".*[%s]+.*", regexCharacters); + return Pattern.compile(regularExpression); + } + + public enum RegexChar { + WHITESPACE("\\s", "character.whitespace"), DIGITS("0-9", "character.digits"), QUOTATION_MARKS("'\"", + "character.quotationMarks"), SLASHES("\\/\\\\", "character.slashes"), GREATER_THAN( + ">"), LESS_THAN("<"), EQUALS_SYMBOL("="), EXCLAMATION_MARK("!"), QUESTION_MARK("?"), COLON(":"); + + private final String regExp; + private final String l18nReferenceDescription; + + RegexChar(final String character) { + this(character, null); + } + + RegexChar(final String regExp, final String l18nReferenceDescription) { + this.regExp = regExp; + this.l18nReferenceDescription = l18nReferenceDescription; + } + + public String getRegExp() { + return regExp; + } + + public Optional getL18nReferenceDescription() { + return Optional.ofNullable(l18nReferenceDescription); + } + } +} diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/ArtifactUpload.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/ArtifactUpload.java new file mode 100644 index 000000000..401cdce03 --- /dev/null +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/ArtifactUpload.java @@ -0,0 +1,129 @@ +/** + * Copyright (c) 2015 Bosch Software Innovations GmbH and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + */ +package org.eclipse.hawkbit.repository.model; + +import java.io.InputStream; + +import javax.validation.constraints.NotEmpty; +import javax.validation.constraints.NotNull; + +import org.hibernate.validator.constraints.SafeHtml; +import org.hibernate.validator.constraints.SafeHtml.WhiteListType; + +/** + * Use to create a new artifact. + * + */ +public class ArtifactUpload { + + @NotNull + private final InputStream inputStream; + + private final long moduleId; + + @NotEmpty + @SafeHtml(whitelistType = WhiteListType.NONE, message = "Invalid characters in string") + private final String filename; + + private final String providedMd5Sum; + + private final String providedSha1Sum; + + private final boolean overrideExisting; + + private final String contentType; + + private final long filesize; + + /** + * Constructor + * + * @param inputStream + * to read from for artifact binary + * @param moduleId + * to assign the new artifact to + * @param filename + * of the artifact + * @param overrideExisting + * to true if the artifact binary can be overridden + * if it already exists + * @param filesize + * the size of the file in bytes. + */ + public ArtifactUpload(final InputStream inputStream, final long moduleId, final String filename, + final boolean overrideExisting, final long filesize) { + this(inputStream, moduleId, filename, null, null, overrideExisting, null, filesize); + } + + /** + * Constructor + * + * @param inputStream + * to read from for artifact binary + * @param moduleId + * to assign the new artifact to + * @param filename + * of the artifact + * @param providedSha1Sum + * optional sha1 checksum to check the new file against + * @param providedMd5Sum + * optional md5 checksum to check the new file against + * @param overrideExisting + * to true if the artifact binary can be overridden + * if it already exists + * @param contentType + * the contentType of the file + * @param filesize + * the size of the file in bytes. + */ + public ArtifactUpload(final InputStream inputStream, final long moduleId, final String filename, + final String providedMd5Sum, final String providedSha1Sum, final boolean overrideExisting, + final String contentType, final long filesize) { + this.inputStream = inputStream; + this.moduleId = moduleId; + this.filename = filename; + this.providedMd5Sum = providedMd5Sum; + this.providedSha1Sum = providedSha1Sum; + this.overrideExisting = overrideExisting; + this.contentType = contentType; + this.filesize = filesize; + } + + public InputStream getInputStream() { + return inputStream; + } + + public long getModuleId() { + return moduleId; + } + + public String getFilename() { + return filename; + } + + public String getProvidedMd5Sum() { + return providedMd5Sum; + } + + public String getProvidedSha1Sum() { + return providedSha1Sum; + } + + public boolean overrideExisting() { + return overrideExisting; + } + + public String getContentType() { + return contentType; + } + + public long getFilesize() { + return filesize; + } +} diff --git a/hawkbit-repository/hawkbit-repository-api/src/test/java/org/eclipse/hawkbit/repository/RegexCharTest.java b/hawkbit-repository/hawkbit-repository-api/src/test/java/org/eclipse/hawkbit/repository/RegexCharTest.java new file mode 100644 index 000000000..4d554147a --- /dev/null +++ b/hawkbit-repository/hawkbit-repository-api/src/test/java/org/eclipse/hawkbit/repository/RegexCharTest.java @@ -0,0 +1,109 @@ +/** + * Copyright (c) 2015 Bosch Software Innovations GmbH and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + */ +package org.eclipse.hawkbit.repository; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.eclipse.hawkbit.repository.RegexCharacterCollection.RegexChar; +import org.junit.Test; + +import io.qameta.allure.Description; +import io.qameta.allure.Feature; +import io.qameta.allure.Story; + +@Feature("Unit Tests - Repository") +@Story("Regular expression helper") +public class RegexCharTest { + + private static final String TEST_STRING = getPrintableAsciiCharacters(); + + private static final int INDEX_FIRST_PRINTABLE_ASCII_CHAR = 32; + private static final int INDEX_LAST_PRINTABLE_ASCII_CHAR = 127; + + @Test + @Description("Verifies every RegexChar can be used to exclusively find the desired characters in a String.") + public void allRegexCharsOnlyFindExpectedChars() { + for (final RegexChar character : RegexChar.values()) { + switch (character) { + case DIGITS: + assertRegexCharExclusivelyFindsGivenCharacters(character, "0", "1", "2", "3", "4", "5", "6", "7", "8", "9"); + break; + case WHITESPACE: + assertRegexCharExclusivelyFindsGivenCharacters(character, " ", "\t"); + break; + case SLASHES: + assertRegexCharExclusivelyFindsGivenCharacters(character, "/", "\\"); + break; + case QUOTATION_MARKS: + assertRegexCharExclusivelyFindsGivenCharacters(character, "\"", "'"); + break; + default: + assertRegexCharExclusivelyFindsGivenCharacters(character, character.getRegExp()); + break; + } + } + } + + @Test + @Description("Verifies that combinations of RegexChars can be used to find the desired characters in a String.") + public void combinedRegexCharsFindExpectedChars() { + final RegexCharacterCollection greaterAndLessThan = new RegexCharacterCollection(RegexChar.GREATER_THAN, + RegexChar.LESS_THAN); + final RegexCharacterCollection equalsAndQuestionMark = new RegexCharacterCollection(RegexChar.EQUALS_SYMBOL, + RegexChar.QUESTION_MARK); + final RegexCharacterCollection colonAndWhitespace = new RegexCharacterCollection(RegexChar.COLON, + RegexChar.WHITESPACE); + + assertRegexCharsExclusivelyFindsGivenCharacters(greaterAndLessThan, "<", ">"); + assertRegexCharsExclusivelyFindsGivenCharacters(equalsAndQuestionMark, "=", "?"); + assertRegexCharsExclusivelyFindsGivenCharacters(colonAndWhitespace, ":", " ", "\t"); + } + + private void assertRegexCharExclusivelyFindsGivenCharacters(final RegexChar characterToVerify, + final String... charactersExpectedToBeFoundByRegex) { + assertRegexCharsExclusivelyFindsGivenCharacters(new RegexCharacterCollection(characterToVerify), + charactersExpectedToBeFoundByRegex); + } + + private void assertRegexCharsExclusivelyFindsGivenCharacters(final RegexCharacterCollection regexToVerify, + final String... charactersExpectedToBeFoundByRegex) { + String notMatchingString = TEST_STRING; + for(final String character : charactersExpectedToBeFoundByRegex) { + notMatchingString = notMatchingString.replace(character, ""); + } + for(final String character : charactersExpectedToBeFoundByRegex) { + assertThat(RegexCharacterCollection.stringContainsCharacter("", regexToVerify)).isFalse(); + assertThat(RegexCharacterCollection.stringContainsCharacter(notMatchingString, regexToVerify)).isFalse(); + assertThat(RegexCharacterCollection.stringContainsCharacter(character, regexToVerify)).isTrue(); + assertThat(RegexCharacterCollection + .stringContainsCharacter(insertStringIntoString(notMatchingString, character, 0), regexToVerify)) + .isTrue(); + assertThat(RegexCharacterCollection.stringContainsCharacter( + insertStringIntoString(notMatchingString, character, notMatchingString.length()), regexToVerify)).isTrue(); + assertThat(RegexCharacterCollection.stringContainsCharacter( + insertStringIntoString(notMatchingString, character, notMatchingString.length() / 2), regexToVerify)).isTrue(); + + } + } + + private static String getPrintableAsciiCharacters() { + final StringBuilder stringBuilder = new StringBuilder(); + for (int i = INDEX_FIRST_PRINTABLE_ASCII_CHAR; i < INDEX_LAST_PRINTABLE_ASCII_CHAR; i++) { + stringBuilder.append((char) i); + } + stringBuilder.append("\t"); + return stringBuilder.toString(); + } + + private static String insertStringIntoString(final String baseString, final String stringToInsert, + final int position) { + final StringBuilder stringBuilder = new StringBuilder(baseString); + return stringBuilder.insert(position, stringToInsert).toString(); + } +} diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaArtifactManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaArtifactManagement.java index ce32098c2..06da6f156 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaArtifactManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaArtifactManagement.java @@ -8,7 +8,6 @@ */ package org.eclipse.hawkbit.repository.jpa; -import java.io.InputStream; import java.util.Optional; import org.eclipse.hawkbit.artifact.repository.ArtifactRepository; @@ -30,6 +29,7 @@ import org.eclipse.hawkbit.repository.jpa.model.JpaArtifact; import org.eclipse.hawkbit.repository.jpa.model.JpaSoftwareModule; import org.eclipse.hawkbit.repository.jpa.utils.QuotaHelper; import org.eclipse.hawkbit.repository.model.Artifact; +import org.eclipse.hawkbit.repository.model.ArtifactUpload; import org.eclipse.hawkbit.repository.model.SoftwareModule; import org.eclipse.hawkbit.tenancy.TenantAware; import org.slf4j.Logger; @@ -95,22 +95,24 @@ public class JpaArtifactManagement implements ArtifactManagement { @Transactional @Retryable(include = { ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY)) - public Artifact create(final InputStream stream, final long moduleId, final String filename, - final String providedMd5Sum, final String providedSha1Sum, final boolean overrideExisting, - final String contentType, final long filesize) { + public Artifact create(final ArtifactUpload artifactUpload) { + final String filename = artifactUpload.getFilename(); + final long moduleId = artifactUpload.getModuleId(); AbstractDbArtifact result = null; final SoftwareModule softwareModule = getModuleAndThrowExceptionIfThatFails(moduleId); - final Artifact existing = checkForExistingArtifact(filename, overrideExisting, softwareModule); + final Artifact existing = checkForExistingArtifact(filename, artifactUpload.overrideExisting(), + softwareModule); assertArtifactQuota(moduleId, 1); - assertMaxArtifactSizeQuota(filename, moduleId, filesize); - assertMaxArtifactStorageQuota(filename, filesize); + assertMaxArtifactSizeQuota(filename, moduleId, artifactUpload.getFilesize()); + assertMaxArtifactStorageQuota(filename, artifactUpload.getFilesize()); try { - result = artifactRepository.store(tenantAware.getCurrentTenant(), stream, filename, contentType, - new DbArtifactHash(providedSha1Sum, providedMd5Sum)); + result = artifactRepository.store(tenantAware.getCurrentTenant(), artifactUpload.getInputStream(), filename, + artifactUpload.getContentType(), + new DbArtifactHash(artifactUpload.getProvidedSha1Sum(), artifactUpload.getProvidedMd5Sum())); } catch (final ArtifactStoreException e) { throw new ArtifactUploadFailedException(e); } catch (final HashNotMatchException e) { @@ -248,15 +250,6 @@ public class JpaArtifactManagement implements ArtifactManagement { return localArtifactRepository.save(artifact); } - @Override - @Transactional - @Retryable(include = { - ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY)) - public Artifact create(final InputStream inputStream, final long moduleId, final String filename, - final boolean overrideExisting, final long filesize) { - return create(inputStream, moduleId, filename, null, null, overrideExisting, null, filesize); - } - @Override public long count() { return localArtifactRepository.count(); diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/ArtifactManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/ArtifactManagementTest.java index 0c180df05..725eb91e7 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/ArtifactManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/ArtifactManagementTest.java @@ -20,6 +20,8 @@ import java.net.URISyntaxException; import java.security.NoSuchAlgorithmException; import java.util.List; +import javax.validation.ConstraintViolationException; + import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.RandomStringUtils; import org.eclipse.hawkbit.im.authentication.SpPermission; @@ -31,6 +33,7 @@ import org.eclipse.hawkbit.repository.exception.QuotaExceededException; import org.eclipse.hawkbit.repository.jpa.model.JpaArtifact; import org.eclipse.hawkbit.repository.jpa.model.JpaSoftwareModule; import org.eclipse.hawkbit.repository.model.Artifact; +import org.eclipse.hawkbit.repository.model.ArtifactUpload; import org.eclipse.hawkbit.repository.model.SoftwareModule; import org.eclipse.hawkbit.repository.test.matcher.Expect; import org.eclipse.hawkbit.repository.test.matcher.ExpectEvents; @@ -74,11 +77,14 @@ public class ArtifactManagementTest extends AbstractJpaIntegrationTest { final String artifactData = "test"; final int artifactSize = artifactData.length(); - verifyThrownExceptionBy(() -> artifactManagement.create(IOUtils.toInputStream(artifactData, "UTF-8"), - NOT_EXIST_IDL, "xxx", null, null, false, null, artifactSize), "SoftwareModule"); + verifyThrownExceptionBy( + () -> artifactManagement.create(new ArtifactUpload(IOUtils.toInputStream(artifactData, "UTF-8"), + NOT_EXIST_IDL, "xxx", null, null, false, null, artifactSize)), + "SoftwareModule"); - verifyThrownExceptionBy(() -> artifactManagement.create(IOUtils.toInputStream(artifactData, "UTF-8"), 1234L, - "xxx", false, artifactSize), "SoftwareModule"); + verifyThrownExceptionBy(() -> artifactManagement.create( + new ArtifactUpload(IOUtils.toInputStream(artifactData, "UTF-8"), 1234L, "xxx", false, artifactSize)), + "SoftwareModule"); verifyThrownExceptionBy(() -> artifactManagement.delete(NOT_EXIST_IDL), "Artifact"); @@ -137,6 +143,21 @@ public class ArtifactManagementTest extends AbstractJpaIntegrationTest { } + @Test + @Description("Verifies that artifact management does not create artifacts with illegal filename.") + public void entityQueryWithIllegalFilenameThrowsException() throws URISyntaxException { + final String illegalFilename = ".xml"; + final String artifactData = "test"; + final int artifactSize = artifactData.length(); + + final long smID = softwareModuleRepository + .save(new JpaSoftwareModule(osType, "smIllegalFilenameTest", "1.0", null, null)).getId(); + assertThatExceptionOfType(ConstraintViolationException.class).isThrownBy(() -> artifactManagement.create( + new ArtifactUpload(IOUtils.toInputStream(artifactData, "UTF-8"), smID, illegalFilename, false, + artifactSize))); + assertThat(softwareModuleManagement.get(smID).get().getArtifacts()).hasSize(0); + } + @Test @Description("Verifies that the quota specifying the maximum number of artifacts per software module is enforced.") public void createArtifactsUntilQuotaIsExceeded() throws NoSuchAlgorithmException, IOException { @@ -408,7 +429,7 @@ public class ArtifactManagementTest extends AbstractJpaIntegrationTest { private Artifact createArtifactForSoftwareModule(final String filename, final long moduleId, final int artifactSize, final InputStream inputStream) throws IOException { - return artifactManagement.create(inputStream, moduleId, filename, false, artifactSize); + return artifactManagement.create(new ArtifactUpload(inputStream, moduleId, filename, false, artifactSize)); } private static byte[] randomBytes(final int len) { diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/ControllerManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/ControllerManagementTest.java index 560594595..92f77c2dd 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/ControllerManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/ControllerManagementTest.java @@ -46,6 +46,7 @@ import org.eclipse.hawkbit.repository.model.Action; import org.eclipse.hawkbit.repository.model.Action.Status; import org.eclipse.hawkbit.repository.model.ActionStatus; import org.eclipse.hawkbit.repository.model.Artifact; +import org.eclipse.hawkbit.repository.model.ArtifactUpload; import org.eclipse.hawkbit.repository.model.DistributionSet; import org.eclipse.hawkbit.repository.model.SoftwareModule; import org.eclipse.hawkbit.repository.model.SoftwareModuleMetadata; @@ -444,10 +445,10 @@ public class ControllerManagementTest extends AbstractJpaIntegrationTest { Target savedTarget = testdataFactory.createTarget(); // create two artifacts with identical SHA1 hash - final Artifact artifact = artifactManagement.create(new ByteArrayInputStream(random), - ds.findFirstModuleByType(osType).get().getId(), "file1", false, artifactSize); - final Artifact artifact2 = artifactManagement.create(new ByteArrayInputStream(random), - ds2.findFirstModuleByType(osType).get().getId(), "file1", false, artifactSize); + final Artifact artifact = artifactManagement.create(new ArtifactUpload(new ByteArrayInputStream(random), + ds.findFirstModuleByType(osType).get().getId(), "file1", false, artifactSize)); + final Artifact artifact2 = artifactManagement.create(new ArtifactUpload(new ByteArrayInputStream(random), + ds2.findFirstModuleByType(osType).get().getId(), "file1", false, artifactSize)); assertThat(artifact.getSha1Hash()).isEqualTo(artifact2.getSha1Hash()); assertThat( diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/SoftwareModuleManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/SoftwareModuleManagementTest.java index 6856eb1a4..74b9577f6 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/SoftwareModuleManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/SoftwareModuleManagementTest.java @@ -30,6 +30,7 @@ import org.eclipse.hawkbit.repository.jpa.model.JpaSoftwareModuleMetadata; import org.eclipse.hawkbit.repository.jpa.model.JpaTarget; import org.eclipse.hawkbit.repository.model.Action; import org.eclipse.hawkbit.repository.model.Artifact; +import org.eclipse.hawkbit.repository.model.ArtifactUpload; import org.eclipse.hawkbit.repository.model.AssignedSoftwareModule; import org.eclipse.hawkbit.repository.model.DistributionSet; import org.eclipse.hawkbit.repository.model.DistributionSetType; @@ -370,7 +371,8 @@ public class SoftwareModuleManagementTest extends AbstractJpaIntegrationTest { SoftwareModule moduleX = createSoftwareModuleWithArtifacts(osType, "modulex", "v1.0", 0); // [STEP2]: Create newArtifactX and add it to SoftwareModuleX - artifactManagement.create(new ByteArrayInputStream(source), moduleX.getId(), "artifactx", false, artifactSize); + artifactManagement.create(new ArtifactUpload(new ByteArrayInputStream(source), moduleX.getId(), "artifactx", + false, artifactSize)); moduleX = softwareModuleManagement.get(moduleX.getId()).get(); final Artifact artifactX = moduleX.getArtifacts().iterator().next(); @@ -378,7 +380,8 @@ public class SoftwareModuleManagementTest extends AbstractJpaIntegrationTest { SoftwareModule moduleY = createSoftwareModuleWithArtifacts(osType, "moduley", "v1.0", 0); // [STEP4]: Assign the same ArtifactX to SoftwareModuleY - artifactManagement.create(new ByteArrayInputStream(source), moduleY.getId(), "artifactx", false, artifactSize); + artifactManagement.create(new ArtifactUpload(new ByteArrayInputStream(source), moduleY.getId(), "artifactx", + false, artifactSize)); moduleY = softwareModuleManagement.get(moduleY.getId()).get(); final Artifact artifactY = moduleY.getArtifacts().iterator().next(); @@ -413,14 +416,16 @@ public class SoftwareModuleManagementTest extends AbstractJpaIntegrationTest { // [STEP1]: Create SoftwareModuleX and add a new ArtifactX SoftwareModule moduleX = createSoftwareModuleWithArtifacts(osType, "modulex", "v1.0", 0); - artifactManagement.create(new ByteArrayInputStream(source), moduleX.getId(), "artifactx", false, artifactSize); + artifactManagement.create(new ArtifactUpload(new ByteArrayInputStream(source), moduleX.getId(), "artifactx", + false, artifactSize)); moduleX = softwareModuleManagement.get(moduleX.getId()).get(); final Artifact artifactX = moduleX.getArtifacts().iterator().next(); // [STEP2]: Create SoftwareModuleY and add the same ArtifactX SoftwareModule moduleY = createSoftwareModuleWithArtifacts(osType, "moduley", "v1.0", 0); - artifactManagement.create(new ByteArrayInputStream(source), moduleY.getId(), "artifactx", false, artifactSize); + artifactManagement.create(new ArtifactUpload(new ByteArrayInputStream(source), moduleY.getId(), "artifactx", + false, artifactSize)); moduleY = softwareModuleManagement.get(moduleY.getId()).get(); final Artifact artifactY = moduleY.getArtifacts().iterator().next(); @@ -468,8 +473,8 @@ public class SoftwareModuleManagementTest extends AbstractJpaIntegrationTest { final int artifactSize = 5 * 1024; for (int i = 0; i < numberArtifacts; i++) { - artifactManagement.create(new RandomGeneratedInputStream(artifactSize), softwareModule.getId(), - "file" + (i + 1), false, artifactSize); + artifactManagement.create(new ArtifactUpload(new RandomGeneratedInputStream(artifactSize), + softwareModule.getId(), "file" + (i + 1), false, artifactSize)); } // Verify correct Creation of SoftwareModule and corresponding artifacts diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/SystemManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/SystemManagementTest.java index 4de21ee05..459afe417 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/SystemManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/SystemManagementTest.java @@ -15,6 +15,7 @@ import java.util.List; import java.util.Random; import org.eclipse.hawkbit.im.authentication.SpPermission.SpringEvalExpressions; +import org.eclipse.hawkbit.repository.model.ArtifactUpload; import org.eclipse.hawkbit.repository.model.DistributionSet; import org.eclipse.hawkbit.repository.model.SoftwareModule; import org.eclipse.hawkbit.repository.model.Target; @@ -134,13 +135,15 @@ public class SystemManagementTest extends AbstractJpaIntegrationTest { private void createTestArtifact(final byte[] random) { final SoftwareModule sm = testdataFactory.createSoftwareModuleOs(); - artifactManagement.create(new ByteArrayInputStream(random), sm.getId(), "file1", false, random.length); + artifactManagement.create( + new ArtifactUpload(new ByteArrayInputStream(random), sm.getId(), "file1", false, random.length)); } private void createDeletedTestArtifact(final byte[] random) { final DistributionSet ds = testdataFactory.createDistributionSet("deleted garbage", true); ds.getModules().stream().forEach(module -> { - artifactManagement.create(new ByteArrayInputStream(random), module.getId(), "file1", false, random.length); + artifactManagement.create(new ArtifactUpload(new ByteArrayInputStream(random), module.getId(), "file1", + false, random.length)); softwareModuleManagement.delete(module.getId()); }); } diff --git a/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/TestdataFactory.java b/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/TestdataFactory.java index 3699f9ec4..a4ccd88a7 100644 --- a/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/TestdataFactory.java +++ b/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/TestdataFactory.java @@ -41,6 +41,7 @@ import org.eclipse.hawkbit.repository.model.Action; import org.eclipse.hawkbit.repository.model.Action.Status; import org.eclipse.hawkbit.repository.model.ActionStatus; import org.eclipse.hawkbit.repository.model.Artifact; +import org.eclipse.hawkbit.repository.model.ArtifactUpload; import org.eclipse.hawkbit.repository.model.BaseEntity; import org.eclipse.hawkbit.repository.model.DistributionSet; import org.eclipse.hawkbit.repository.model.DistributionSetTag; @@ -466,7 +467,8 @@ public class TestdataFactory { final String artifactData = "some test data" + i; final InputStream stubInputStream = IOUtils.toInputStream(artifactData, Charset.forName("UTF-8")); artifacts.add( - artifactManagement.create(stubInputStream, moduleId, "filename" + i, false, artifactData.length())); + artifactManagement.create(new ArtifactUpload(stubInputStream, moduleId, "filename" + i, false, + artifactData.length()))); } diff --git a/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiArtifactDownloadTest.java b/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiArtifactDownloadTest.java index 4b0d8973f..ab6dfafef 100644 --- a/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiArtifactDownloadTest.java +++ b/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiArtifactDownloadTest.java @@ -32,6 +32,7 @@ import org.apache.commons.lang3.RandomUtils; import org.eclipse.hawkbit.ddi.rest.resource.DdiArtifactDownloadTest.DownloadTestConfiguration; import org.eclipse.hawkbit.repository.event.remote.DownloadProgressEvent; import org.eclipse.hawkbit.repository.model.Artifact; +import org.eclipse.hawkbit.repository.model.ArtifactUpload; import org.eclipse.hawkbit.repository.model.DistributionSet; import org.eclipse.hawkbit.repository.model.Target; import org.eclipse.hawkbit.repository.test.util.TestdataFactory; @@ -84,8 +85,8 @@ public class DdiArtifactDownloadTest extends AbstractDDiApiIntegrationTest { // create artifact final int artifactSize = 5 * 1024; final byte random[] = RandomUtils.nextBytes(artifactSize); - final Artifact artifact = artifactManagement.create(new ByteArrayInputStream(random), - ds.findFirstModuleByType(osType).get().getId(), "file1", false, artifactSize); + final Artifact artifact = artifactManagement.create(new ArtifactUpload(new ByteArrayInputStream(random), + ds.findFirstModuleByType(osType).get().getId(), "file1", false, artifactSize)); // no artifact available mvc.perform(get("/controller/v1/{controllerId}/softwaremodules/{softwareModuleId}/artifacts/123455", @@ -171,8 +172,8 @@ public class DdiArtifactDownloadTest extends AbstractDDiApiIntegrationTest { // create artifact final int artifactSize = 5 * 1024 * 1024; final byte random[] = RandomUtils.nextBytes(artifactSize); - final Artifact artifact = artifactManagement.create(new ByteArrayInputStream(random), - ds.findFirstModuleByType(osType).get().getId(), "file1", false, artifactSize); + final Artifact artifact = artifactManagement.create(new ArtifactUpload(new ByteArrayInputStream(random), + ds.findFirstModuleByType(osType).get().getId(), "file1", false, artifactSize)); // download fails as artifact is not yet assigned mvc.perform(get("/controller/v1/{controllerId}/softwaremodules/{softwareModuleId}/artifacts/{filename}", @@ -211,8 +212,8 @@ public class DdiArtifactDownloadTest extends AbstractDDiApiIntegrationTest { // create artifact final int artifactSize = 5 * 1024; final byte random[] = RandomUtils.nextBytes(artifactSize); - final Artifact artifact = artifactManagement.create(new ByteArrayInputStream(random), getOsModule(ds), "file1", - false, artifactSize); + final Artifact artifact = artifactManagement.create( + new ArtifactUpload(new ByteArrayInputStream(random), getOsModule(ds), "file1", false, artifactSize)); // download final MvcResult result = mvc.perform(get( @@ -241,8 +242,8 @@ public class DdiArtifactDownloadTest extends AbstractDDiApiIntegrationTest { // create artifact final byte random[] = RandomUtils.nextBytes(resultLength); - final Artifact artifact = artifactManagement.create(new ByteArrayInputStream(random), getOsModule(ds), "file1", - false, resultLength); + final Artifact artifact = artifactManagement.create( + new ArtifactUpload(new ByteArrayInputStream(random), getOsModule(ds), "file1", false, resultLength)); assertThat(random.length).isEqualTo(resultLength); diff --git a/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiDeploymentBaseTest.java b/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiDeploymentBaseTest.java index 5cdcc68e1..7746ed107 100644 --- a/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiDeploymentBaseTest.java +++ b/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiDeploymentBaseTest.java @@ -40,6 +40,7 @@ import org.eclipse.hawkbit.repository.model.Action.ActionType; import org.eclipse.hawkbit.repository.model.Action.Status; import org.eclipse.hawkbit.repository.model.ActionStatus; import org.eclipse.hawkbit.repository.model.Artifact; +import org.eclipse.hawkbit.repository.model.ArtifactUpload; import org.eclipse.hawkbit.repository.model.DistributionSet; import org.eclipse.hawkbit.repository.model.RepositoryModelConstants; import org.eclipse.hawkbit.repository.model.Target; @@ -112,10 +113,10 @@ public class DdiDeploymentBaseTest extends AbstractDDiApiIntegrationTest { final int artifactSize = 5 * 1024; final byte random[] = RandomUtils.nextBytes(artifactSize); - final Artifact artifact = artifactManagement.create(new ByteArrayInputStream(random), getOsModule(ds), "test1", - false, artifactSize); - final Artifact artifactSignature = artifactManagement.create(new ByteArrayInputStream(random), getOsModule(ds), - "test1.signature", false, artifactSize); + final Artifact artifact = artifactManagement.create( + new ArtifactUpload(new ByteArrayInputStream(random), getOsModule(ds), "test1", false, artifactSize)); + final Artifact artifactSignature = artifactManagement.create(new ArtifactUpload( + new ByteArrayInputStream(random), getOsModule(ds), "test1.signature", false, artifactSize)); final Target savedTarget = testdataFactory.createTarget("4712"); @@ -270,10 +271,10 @@ public class DdiDeploymentBaseTest extends AbstractDDiApiIntegrationTest { final int artifactSize = 5 * 1024; final byte random[] = RandomUtils.nextBytes(artifactSize); - final Artifact artifact = artifactManagement.create(new ByteArrayInputStream(random), getOsModule(ds), "test1", - false, artifactSize); - final Artifact artifactSignature = artifactManagement.create(new ByteArrayInputStream(random), getOsModule(ds), - "test1.signature", false, artifactSize); + final Artifact artifact = artifactManagement.create( + new ArtifactUpload(new ByteArrayInputStream(random), getOsModule(ds), "test1", false, artifactSize)); + final Artifact artifactSignature = artifactManagement.create(new ArtifactUpload( + new ByteArrayInputStream(random), getOsModule(ds), "test1.signature", false, artifactSize)); softwareModuleManagement.createMetaData(entityFactory.softwareModuleMetadata().create(getOsModule(ds)) .key(visibleMetadataOsKey).value(visibleMetadataOsValue).targetVisible(true)); @@ -394,10 +395,10 @@ public class DdiDeploymentBaseTest extends AbstractDDiApiIntegrationTest { final int artifactSize = 5 * 1024; final byte random[] = RandomUtils.nextBytes(artifactSize); - final Artifact artifact = artifactManagement.create(new ByteArrayInputStream(random), getOsModule(ds), "test1", - false, artifactSize); - final Artifact artifactSignature = artifactManagement.create(new ByteArrayInputStream(random), getOsModule(ds), - "test1.signature", false, artifactSize); + final Artifact artifact = artifactManagement.create( + new ArtifactUpload(new ByteArrayInputStream(random), getOsModule(ds), "test1", false, artifactSize)); + final Artifact artifactSignature = artifactManagement.create(new ArtifactUpload( + new ByteArrayInputStream(random), getOsModule(ds), "test1.signature", false, artifactSize)); final Target savedTarget = testdataFactory.createTarget("4712"); diff --git a/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtSoftwareModuleResource.java b/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtSoftwareModuleResource.java index 5e51c6f2c..343706c2d 100644 --- a/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtSoftwareModuleResource.java +++ b/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtSoftwareModuleResource.java @@ -28,6 +28,7 @@ import org.eclipse.hawkbit.repository.OffsetBasedPageRequest; import org.eclipse.hawkbit.repository.SoftwareModuleManagement; import org.eclipse.hawkbit.repository.exception.EntityNotFoundException; import org.eclipse.hawkbit.repository.model.Artifact; +import org.eclipse.hawkbit.repository.model.ArtifactUpload; import org.eclipse.hawkbit.repository.model.SoftwareModule; import org.eclipse.hawkbit.repository.model.SoftwareModuleMetadata; import org.slf4j.Logger; @@ -81,9 +82,9 @@ public class MgmtSoftwareModuleResource implements MgmtSoftwareModuleRestApi { } try (InputStream in = file.getInputStream()) { - final Artifact result = artifactManagement.create(in, softwareModuleId, fileName, + final Artifact result = artifactManagement.create(new ArtifactUpload(in, softwareModuleId, fileName, md5Sum == null ? null : md5Sum.toLowerCase(), sha1Sum == null ? null : sha1Sum.toLowerCase(), false, - file.getContentType(), file.getSize()); + file.getContentType(), file.getSize())); final MgmtArtifact reponse = MgmtSoftwareModuleMapper.toResponse(result); MgmtSoftwareModuleMapper.addLinks(result, reponse); diff --git a/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtSoftwareModuleResourceTest.java b/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtSoftwareModuleResourceTest.java index fecb1ae64..7e7ca9854 100644 --- a/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtSoftwareModuleResourceTest.java +++ b/hawkbit-rest/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtSoftwareModuleResourceTest.java @@ -12,6 +12,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.hasSize; import static org.junit.Assert.assertTrue; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; @@ -38,6 +39,7 @@ import org.eclipse.hawkbit.mgmt.rest.api.MgmtRestConstants; import org.eclipse.hawkbit.repository.Constants; import org.eclipse.hawkbit.repository.exception.QuotaExceededException; import org.eclipse.hawkbit.repository.model.Artifact; +import org.eclipse.hawkbit.repository.model.ArtifactUpload; import org.eclipse.hawkbit.repository.model.DistributionSet; import org.eclipse.hawkbit.repository.model.SoftwareModule; import org.eclipse.hawkbit.repository.model.SoftwareModuleMetadata; @@ -204,6 +206,21 @@ public class MgmtSoftwareModuleResourceTest extends AbstractManagementApiIntegra .accept(MediaType.APPLICATION_JSON)).andDo(MockMvcResultPrinter.print()) .andExpect(status().isForbidden()); } + + @Test + @Description("Verifies that artifact with invalid filename cannot be uploaded to prevent cross site scripting.") + public void uploadArtifactFailsIfFilenameInvalide() throws Exception { + final SoftwareModule sm = testdataFactory.createSoftwareModule("quota", "quota"); + final String illegalFilename = ".xml"; + + final byte[] randomBytes = randomBytes(5 * 1024); + final MockMultipartFile file = new MockMultipartFile("file", illegalFilename, null, randomBytes); + + mvc.perform(fileUpload("/rest/v1/softwaremodules/{smId}/artifacts", sm.getId()).file(file) + .accept(MediaType.APPLICATION_JSON)).andDo(MockMvcResultPrinter.print()) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.message", containsString("The filename might contain unsafe HTML code."))); + } private void assertArtifact(final SoftwareModule sm, final byte[] random) throws IOException { // check result in db... @@ -426,10 +443,10 @@ public class MgmtSoftwareModuleResourceTest extends AbstractManagementApiIntegra final int artifactSize = 5 * 1024; final byte random[] = randomBytes(artifactSize); - final Artifact artifact = artifactManagement.create(new ByteArrayInputStream(random), sm.getId(), "file1", - false, artifactSize); - final Artifact artifact2 = artifactManagement.create(new ByteArrayInputStream(random), sm.getId(), "file2", - false, artifactSize); + final Artifact artifact = artifactManagement + .create(new ArtifactUpload(new ByteArrayInputStream(random), sm.getId(), "file1", false, artifactSize)); + final Artifact artifact2 = artifactManagement + .create(new ArtifactUpload(new ByteArrayInputStream(random), sm.getId(), "file2", false, artifactSize)); downloadAndVerify(sm, random, artifact); downloadAndVerify(sm, random, artifact2); @@ -458,8 +475,8 @@ public class MgmtSoftwareModuleResourceTest extends AbstractManagementApiIntegra final int artifactSize = 5 * 1024; final byte random[] = randomBytes(artifactSize); - final Artifact artifact = artifactManagement.create(new ByteArrayInputStream(random), sm.getId(), "file1", - false, artifactSize); + final Artifact artifact = artifactManagement + .create(new ArtifactUpload(new ByteArrayInputStream(random), sm.getId(), "file1", false, artifactSize)); // perform test mvc.perform(get("/rest/v1/softwaremodules/{smId}/artifacts/{artId}", sm.getId(), artifact.getId()) @@ -485,10 +502,10 @@ public class MgmtSoftwareModuleResourceTest extends AbstractManagementApiIntegra final int artifactSize = 5 * 1024; final byte random[] = randomBytes(artifactSize); - final Artifact artifact = artifactManagement.create(new ByteArrayInputStream(random), sm.getId(), "file1", - false, artifactSize); - final Artifact artifact2 = artifactManagement.create(new ByteArrayInputStream(random), sm.getId(), "file2", - false, artifactSize); + final Artifact artifact = artifactManagement + .create(new ArtifactUpload(new ByteArrayInputStream(random), sm.getId(), "file1", false, artifactSize)); + final Artifact artifact2 = artifactManagement + .create(new ArtifactUpload(new ByteArrayInputStream(random), sm.getId(), "file2", false, artifactSize)); mvc.perform(get("/rest/v1/softwaremodules/{smId}/artifacts", sm.getId()).accept(MediaType.APPLICATION_JSON)) .andDo(MockMvcResultPrinter.print()).andExpect(status().isOk()) @@ -527,7 +544,8 @@ public class MgmtSoftwareModuleResourceTest extends AbstractManagementApiIntegra .andDo(MockMvcResultPrinter.print()).andExpect(status().isNotFound()); // SM does not exist - artifactManagement.create(new ByteArrayInputStream(random), sm.getId(), "file1", false, artifactSize); + artifactManagement + .create(new ArtifactUpload(new ByteArrayInputStream(random), sm.getId(), "file1", false, artifactSize)); mvc.perform(get("/rest/v1/softwaremodules/1234567890/artifacts")).andDo(MockMvcResultPrinter.print()) .andExpect(status().isNotFound()); @@ -838,7 +856,8 @@ public class MgmtSoftwareModuleResourceTest extends AbstractManagementApiIntegra final int artifactSize = 5 * 1024; final byte random[] = RandomStringUtils.random(artifactSize).getBytes(); - artifactManagement.create(new ByteArrayInputStream(random), sm.getId(), "file1", false, artifactSize); + artifactManagement + .create(new ArtifactUpload(new ByteArrayInputStream(random), sm.getId(), "file1", false, artifactSize)); assertThat(softwareModuleManagement.findAll(PAGE)).as("Softwaremoudle size is wrong").hasSize(1); assertThat(artifactManagement.count()).isEqualTo(1); @@ -861,7 +880,8 @@ public class MgmtSoftwareModuleResourceTest extends AbstractManagementApiIntegra final Long appTypeSmId = ds1.findFirstModuleByType(appType).get().getId(); - artifactManagement.create(new ByteArrayInputStream(random), appTypeSmId, "file1", false, artifactSize); + artifactManagement.create( + new ArtifactUpload(new ByteArrayInputStream(random), appTypeSmId, "file1", false, artifactSize)); assertThat(softwareModuleManagement.count()).isEqualTo(3); assertThat(artifactManagement.count()).isEqualTo(1); @@ -889,9 +909,10 @@ public class MgmtSoftwareModuleResourceTest extends AbstractManagementApiIntegra final byte random[] = RandomStringUtils.random(artifactSize).getBytes(); // Create 2 artifacts - final Artifact artifact = artifactManagement.create(new ByteArrayInputStream(random), sm.getId(), "file1", - false, artifactSize); - artifactManagement.create(new ByteArrayInputStream(random), sm.getId(), "file2", false, artifactSize); + final Artifact artifact = artifactManagement + .create(new ArtifactUpload(new ByteArrayInputStream(random), sm.getId(), "file1", false, artifactSize)); + artifactManagement + .create(new ArtifactUpload(new ByteArrayInputStream(random), sm.getId(), "file2", false, artifactSize)); // check repo before delete assertThat(softwareModuleManagement.findAll(PAGE)).hasSize(1); diff --git a/hawkbit-rest/hawkbit-rest-docs/src/test/java/org/eclipse/hawkbit/rest/ddi/documentation/RootControllerDocumentationTest.java b/hawkbit-rest/hawkbit-rest-docs/src/test/java/org/eclipse/hawkbit/rest/ddi/documentation/RootControllerDocumentationTest.java index 701f81bfd..2de1d7b94 100644 --- a/hawkbit-rest/hawkbit-rest-docs/src/test/java/org/eclipse/hawkbit/rest/ddi/documentation/RootControllerDocumentationTest.java +++ b/hawkbit-rest/hawkbit-rest-docs/src/test/java/org/eclipse/hawkbit/rest/ddi/documentation/RootControllerDocumentationTest.java @@ -29,6 +29,7 @@ import org.apache.commons.lang3.RandomStringUtils; import org.eclipse.hawkbit.ddi.rest.api.DdiRestConstants; import org.eclipse.hawkbit.repository.model.Action; import org.eclipse.hawkbit.repository.model.Action.Status; +import org.eclipse.hawkbit.repository.model.ArtifactUpload; import org.eclipse.hawkbit.repository.model.DistributionSet; import org.eclipse.hawkbit.repository.model.SoftwareModule; import org.eclipse.hawkbit.repository.model.Target; @@ -129,8 +130,10 @@ public class RootControllerDocumentationTest extends AbstractApiRestDocumentatio set.getModules().forEach(module -> { final byte random[] = RandomStringUtils.random(5).getBytes(); - artifactManagement.create(new ByteArrayInputStream(random), module.getId(), "binary.tgz", false, 0); - artifactManagement.create(new ByteArrayInputStream(random), module.getId(), "file.signature", false, 0); + artifactManagement.create( + new ArtifactUpload(new ByteArrayInputStream(random), module.getId(), "binary.tgz", false, 0)); + artifactManagement.create( + new ArtifactUpload(new ByteArrayInputStream(random), module.getId(), "file.signature", false, 0)); }); final Target target = targetManagement.create(entityFactory.target().create().controllerId(CONTROLLER_ID)); @@ -250,8 +253,10 @@ public class RootControllerDocumentationTest extends AbstractApiRestDocumentatio set.getModules().forEach(module -> { final byte random[] = RandomStringUtils.random(5).getBytes(); - artifactManagement.create(new ByteArrayInputStream(random), module.getId(), "binary.tgz", false, 0); - artifactManagement.create(new ByteArrayInputStream(random), module.getId(), "file.signature", false, 0); + artifactManagement.create( + new ArtifactUpload(new ByteArrayInputStream(random), module.getId(), "binary.tgz", false, 0)); + artifactManagement.create( + new ArtifactUpload(new ByteArrayInputStream(random), module.getId(), "file.signature", false, 0)); }); softwareModuleManagement.createMetaData( @@ -426,7 +431,8 @@ public class RootControllerDocumentationTest extends AbstractApiRestDocumentatio final SoftwareModule module = (SoftwareModule) set.getModules().toArray()[0]; final byte random[] = RandomStringUtils.random(5).getBytes(); - artifactManagement.create(new ByteArrayInputStream(random), module.getId(), "binaryFile", false, 0); + artifactManagement + .create(new ArtifactUpload(new ByteArrayInputStream(random), module.getId(), "binaryFile", false, 0)); final Target target = targetManagement.create(entityFactory.target().create().controllerId(CONTROLLER_ID)); deploymentManagement.assignDistributionSet(set.getId(), Arrays.asList(target.getTargetWithActionType())); diff --git a/hawkbit-rest/hawkbit-rest-docs/src/test/java/org/eclipse/hawkbit/rest/documentation/AbstractApiRestDocumentation.java b/hawkbit-rest/hawkbit-rest-docs/src/test/java/org/eclipse/hawkbit/rest/documentation/AbstractApiRestDocumentation.java index 7cf3bf903..9811d5297 100644 --- a/hawkbit-rest/hawkbit-rest-docs/src/test/java/org/eclipse/hawkbit/rest/documentation/AbstractApiRestDocumentation.java +++ b/hawkbit-rest/hawkbit-rest-docs/src/test/java/org/eclipse/hawkbit/rest/documentation/AbstractApiRestDocumentation.java @@ -26,6 +26,7 @@ import org.eclipse.hawkbit.ddi.rest.resource.DdiApiConfiguration; import org.eclipse.hawkbit.mgmt.rest.resource.MgmtApiConfiguration; import org.eclipse.hawkbit.repository.model.Action; import org.eclipse.hawkbit.repository.model.Action.Status; +import org.eclipse.hawkbit.repository.model.ArtifactUpload; import org.eclipse.hawkbit.repository.model.DistributionSet; import org.eclipse.hawkbit.repository.model.Target; import org.eclipse.hawkbit.repository.model.TargetUpdateStatus; @@ -178,7 +179,8 @@ public abstract class AbstractApiRestDocumentation extends AbstractRestIntegrati distributionSet.getModules().forEach(module -> { final byte[] random = RandomStringUtils.random(5).getBytes(); - artifactManagement.create(new ByteArrayInputStream(random), module.getId(), "file1", false, 0); + artifactManagement + .create(new ArtifactUpload(new ByteArrayInputStream(random), module.getId(), "file1", false, 0)); softwareModuleManagement.update(entityFactory.softwareModule().update(module.getId()) .description("Description of the software module")); }); diff --git a/hawkbit-rest/hawkbit-rest-docs/src/test/java/org/eclipse/hawkbit/rest/mgmt/documentation/SoftwaremodulesDocumentationTest.java b/hawkbit-rest/hawkbit-rest-docs/src/test/java/org/eclipse/hawkbit/rest/mgmt/documentation/SoftwaremodulesDocumentationTest.java index 1597faeae..ddb6c2083 100644 --- a/hawkbit-rest/hawkbit-rest-docs/src/test/java/org/eclipse/hawkbit/rest/mgmt/documentation/SoftwaremodulesDocumentationTest.java +++ b/hawkbit-rest/hawkbit-rest-docs/src/test/java/org/eclipse/hawkbit/rest/mgmt/documentation/SoftwaremodulesDocumentationTest.java @@ -31,6 +31,7 @@ import org.eclipse.hawkbit.im.authentication.SpPermission; import org.eclipse.hawkbit.mgmt.rest.api.MgmtRestConstants; import org.eclipse.hawkbit.repository.Constants; import org.eclipse.hawkbit.repository.model.Artifact; +import org.eclipse.hawkbit.repository.model.ArtifactUpload; import org.eclipse.hawkbit.repository.model.SoftwareModule; import org.eclipse.hawkbit.rest.documentation.AbstractApiRestDocumentation; import org.eclipse.hawkbit.rest.documentation.ApiModelPropertiesGeneric; @@ -244,7 +245,7 @@ public class SoftwaremodulesDocumentationTest extends AbstractApiRestDocumentati final byte random[] = RandomStringUtils.random(5).getBytes(); - artifactManagement.create(new ByteArrayInputStream(random), sm.getId(), "file1", false, 0); + artifactManagement.create(new ArtifactUpload(new ByteArrayInputStream(random), sm.getId(), "file1", false, 0)); mockMvc.perform( get(MgmtRestConstants.SOFTWAREMODULE_V1_REQUEST_MAPPING + "/{softwareModuleId}/artifacts", sm.getId())) @@ -333,8 +334,8 @@ public class SoftwaremodulesDocumentationTest extends AbstractApiRestDocumentati final byte random[] = RandomStringUtils.random(5).getBytes(); - final Artifact artifact = artifactManagement.create(new ByteArrayInputStream(random), sm.getId(), "file1", - false, 0); + final Artifact artifact = artifactManagement + .create(new ArtifactUpload(new ByteArrayInputStream(random), sm.getId(), "file1", false, 0)); mockMvc.perform(delete( MgmtRestConstants.SOFTWAREMODULE_V1_REQUEST_MAPPING + "/{softwareModuleId}/artifacts/{artifactId}", @@ -352,8 +353,8 @@ public class SoftwaremodulesDocumentationTest extends AbstractApiRestDocumentati final byte random[] = RandomStringUtils.random(5).getBytes(); - final Artifact artifact = artifactManagement.create(new ByteArrayInputStream(random), sm.getId(), "file1", - false, 0); + final Artifact artifact = artifactManagement + .create(new ArtifactUpload(new ByteArrayInputStream(random), sm.getId(), "file1", false, 0)); mockMvc.perform( get(MgmtRestConstants.SOFTWAREMODULE_V1_REQUEST_MAPPING + "/{softwareModuleId}/artifacts/{artifactId}", @@ -387,8 +388,8 @@ public class SoftwaremodulesDocumentationTest extends AbstractApiRestDocumentati final byte random[] = RandomStringUtils.random(5).getBytes(); - final Artifact artifact = artifactManagement.create(new ByteArrayInputStream(random), sm.getId(), "file1", - false, 0); + final Artifact artifact = artifactManagement + .create(new ArtifactUpload(new ByteArrayInputStream(random), sm.getId(), "file1", false, 0)); mockMvc.perform(get(MgmtRestConstants.SOFTWAREMODULE_V1_REQUEST_MAPPING + "/{softwareModuleId}/artifacts/{artifactId}/download", sm.getId(), artifact.getId()) diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/AbstractFileTransferHandler.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/AbstractFileTransferHandler.java index ec61a3069..fac756356 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/AbstractFileTransferHandler.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/AbstractFileTransferHandler.java @@ -17,10 +17,13 @@ import java.util.concurrent.ExecutorService; import org.apache.commons.lang3.StringUtils; import org.eclipse.hawkbit.repository.ArtifactManagement; +import org.eclipse.hawkbit.repository.RegexCharacterCollection; +import org.eclipse.hawkbit.repository.RegexCharacterCollection.RegexChar; import org.eclipse.hawkbit.repository.exception.ArtifactUploadFailedException; import org.eclipse.hawkbit.repository.exception.InvalidMD5HashException; import org.eclipse.hawkbit.repository.exception.InvalidSHA1HashException; import org.eclipse.hawkbit.repository.model.Artifact; +import org.eclipse.hawkbit.repository.model.ArtifactUpload; import org.eclipse.hawkbit.repository.model.SoftwareModule; import org.eclipse.hawkbit.ui.artifacts.event.SoftwareModuleEvent; import org.eclipse.hawkbit.ui.artifacts.event.SoftwareModuleEvent.SoftwareModuleEventType; @@ -46,8 +49,6 @@ public abstract class AbstractFileTransferHandler implements Serializable { private static final Logger LOG = LoggerFactory.getLogger(AbstractFileTransferHandler.class); - private volatile boolean duplicateFile; - private volatile boolean uploadInterrupted; private volatile String failureReason; @@ -62,6 +63,9 @@ public abstract class AbstractFileTransferHandler implements Serializable { protected final UINotification uiNotification; + protected static final RegexCharacterCollection ILLEGAL_FILENAME_CHARACTERS = new RegexCharacterCollection( + RegexChar.GREATER_THAN, RegexChar.LESS_THAN, RegexChar.SLASHES); + AbstractFileTransferHandler(final ArtifactManagement artifactManagement, final VaadinMessageSource i18n) { this.artifactManagement = artifactManagement; this.i18n = i18n; @@ -70,25 +74,11 @@ public abstract class AbstractFileTransferHandler implements Serializable { this.uiNotification = SpringContextHelper.getBean(UINotification.class); } - protected boolean isDuplicateFile() { - return duplicateFile; - } - - protected void setDuplicateFile() { - uploadInterrupted = true; - duplicateFile = true; - } - - protected void setUploadInterrupted() { - uploadInterrupted = true; - } - protected boolean isUploadInterrupted() { return uploadInterrupted; } protected void resetState() { - duplicateFile = false; uploadInterrupted = false; failureReason = null; } @@ -108,16 +98,25 @@ public abstract class AbstractFileTransferHandler implements Serializable { new TransferArtifactToRepositoryRunnable(inputStream, fileUploadId, mimeType, UI.getCurrent())); } - private void setFailureReason(final String failureReason) { + private void interruptUploadAndSetReason(final String failureReason) { + uploadInterrupted = true; this.failureReason = failureReason; } - protected void setFailureReasonUploadFailed() { - setFailureReason(i18n.getMessage("message.upload.failed")); + protected void interruptUploadDueToUploadFailed() { + interruptUploadAndSetReason(i18n.getMessage("message.upload.failed")); } - protected void setFailureReasonFileSizeExceeded(final long maxSize) { - setFailureReason(i18n.getMessage("message.uploadedfile.size.exceeded", maxSize)); + protected void interruptUploadDueToDuplicateFile() { + interruptUploadAndSetReason(i18n.getMessage("message.no.duplicateFiles")); + } + + protected void interruptUploadDueToFileSizeExceeded(final long maxSize) { + interruptUploadAndSetReason(i18n.getMessage("message.uploadedfile.size.exceeded", maxSize)); + } + + protected void interruptUploadDueToIllegalFilename() { + interruptUploadAndSetReason(i18n.getMessage("message.uploadedfile.illegalFilename")); } protected boolean isFileAlreadyContainedInSoftwareModule(final FileUploadId newFileUploadId, @@ -171,19 +170,17 @@ public abstract class AbstractFileTransferHandler implements Serializable { new SoftwareModuleEvent(SoftwareModuleEventType.ARTIFACTS_CHANGED, fileUploadId.getSoftwareModuleId())); } - protected void publishUploadFailedEvent(final FileUploadId fileUploadId, final String failureReason, + protected void publishUploadFailedAndFinishedEvent(final FileUploadId fileUploadId, final Exception uploadException) { - LOG.info("Upload failed for file {} due to {}", fileUploadId, - StringUtils.isBlank(failureReason) ? uploadException.getMessage() : failureReason); + LOG.info("Upload failed for file {} due to reason: {}, exception: {}", fileUploadId, failureReason, + uploadException.getMessage()); + final FileUploadProgress fileUploadProgress = new FileUploadProgress(fileUploadId, FileUploadStatus.UPLOAD_FAILED, - StringUtils.isBlank(failureReason) ? uploadException.getMessage() : failureReason); + StringUtils.isBlank(failureReason) ? i18n.getMessage("message.upload.failed") : failureReason); artifactUploadState.updateFileUploadProgress(fileUploadId, fileUploadProgress); eventBus.publish(this, fileUploadProgress); - } - - protected void publishUploadFailedEvent(final FileUploadId fileUploadId, final Exception uploadException) { - publishUploadFailedEvent(fileUploadId, failureReason, uploadException); + publishUploadFinishedEvent(fileUploadId); } protected void assertStateConsistency(final FileUploadId fileUploadId, final String filenameExtractedFromEvent) { @@ -234,7 +231,8 @@ public abstract class AbstractFileTransferHandler implements Serializable { UI.setCurrent(vaadinUi); streamToRepository(); } catch (final RuntimeException e) { - publishUploadFailedEvent(fileUploadId, i18n.getMessage("message.upload.failed"), e); + interruptUploadDueToUploadFailed(); + publishUploadFailedAndFinishedEvent(fileUploadId, e); LOG.error("Failed to transfer file to repository", e); } finally { tryToCloseIOStream(inputStream); @@ -250,6 +248,7 @@ public abstract class AbstractFileTransferHandler implements Serializable { final Artifact artifact = uploadArtifact(filename).orElseThrow(ArtifactUploadFailedException::new); if (isUploadInterrupted()) { handleUploadFailure(artifact); + publishUploadFinishedEvent(fileUploadId); return; } publishUploadSucceeded(fileUploadId, artifact.getSize()); @@ -259,8 +258,8 @@ public abstract class AbstractFileTransferHandler implements Serializable { private Optional uploadArtifact(final String filename) { try { - return Optional.ofNullable(artifactManagement.create(inputStream, fileUploadId.getSoftwareModuleId(), - filename, null, null, true, mimeType, -1)); + return Optional.ofNullable(artifactManagement.create(new ArtifactUpload(inputStream, + fileUploadId.getSoftwareModuleId(), filename, null, null, true, mimeType, -1))); } catch (final ArtifactUploadFailedException | InvalidSHA1HashException | InvalidMD5HashException e) { LOG.error("Failed to transfer file to repository", e); return Optional.empty(); diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/FileTransferHandlerStreamVariable.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/FileTransferHandlerStreamVariable.java index a871c31ae..36de30312 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/FileTransferHandlerStreamVariable.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/FileTransferHandlerStreamVariable.java @@ -14,8 +14,8 @@ import java.io.PipedInputStream; import java.io.PipedOutputStream; import org.eclipse.hawkbit.repository.ArtifactManagement; +import org.eclipse.hawkbit.repository.RegexCharacterCollection; import org.eclipse.hawkbit.repository.model.SoftwareModule; -import org.eclipse.hawkbit.ui.artifacts.upload.FileUploadProgress.FileUploadStatus; import org.eclipse.hawkbit.ui.utils.VaadinMessageSource; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -63,12 +63,13 @@ public class FileTransferHandlerStreamVariable extends AbstractFileTransferHandl public void streamingStarted(final StreamingStartEvent event) { assertStateConsistency(fileUploadId, event.getFileName()); - if (isFileAlreadyContainedInSoftwareModule(fileUploadId, selectedSoftwareModule)) { + if (RegexCharacterCollection.stringContainsCharacter(event.getFileName(), ILLEGAL_FILENAME_CHARACTERS)) { + LOG.info("Filename contains illegal characters {} for upload {}", fileUploadId.getFilename(), fileUploadId); + interruptUploadDueToIllegalFilename(); + } else if (isFileAlreadyContainedInSoftwareModule(fileUploadId, selectedSoftwareModule)) { LOG.info("File {} already contained in Software Module {}", fileUploadId.getFilename(), selectedSoftwareModule); - getUploadState().updateFileUploadProgress(fileUploadId, - new FileUploadProgress(fileUploadId, FileUploadStatus.UPLOAD_FAILED)); - setDuplicateFile(); + interruptUploadDueToDuplicateFile(); } } @@ -90,10 +91,8 @@ public class FileTransferHandlerStreamVariable extends AbstractFileTransferHandl LOG.error("Creating piped Stream failed {}.", e); tryToCloseIOStream(outputStream); tryToCloseIOStream(inputStream); - setFailureReasonUploadFailed(); - setUploadInterrupted(); - getUploadState().updateFileUploadProgress(fileUploadId, - new FileUploadProgress(fileUploadId, FileUploadStatus.UPLOAD_FAILED)); + interruptUploadDueToUploadFailed(); + publishUploadFailedAndFinishedEvent(fileUploadId, e); return ByteStreams.nullOutputStream(); } return outputStream; @@ -120,8 +119,7 @@ public class FileTransferHandlerStreamVariable extends AbstractFileTransferHandl if (event.getBytesReceived() > maxSize || event.getContentLength() > maxSize) { LOG.error("User tried to upload more than was allowed ({}).", maxSize); - setFailureReasonFileSizeExceeded(maxSize); - setUploadInterrupted(); + interruptUploadDueToFileSizeExceeded(maxSize); return; } if (isUploadInterrupted()) { @@ -153,14 +151,10 @@ public class FileTransferHandlerStreamVariable extends AbstractFileTransferHandl public void streamingFailed(final StreamingErrorEvent event) { assertStateConsistency(fileUploadId, event.getFileName()); - if (isDuplicateFile()) { - publishUploadFailedEvent(fileUploadId, getI18n().getMessage("message.no.duplicateFiles"), - event.getException()); - } else { - publishUploadFailedEvent(fileUploadId, event.getException()); + if (!isUploadInterrupted()) { + interruptUploadDueToUploadFailed(); } - setUploadInterrupted(); - publishUploadFinishedEvent(fileUploadId); + publishUploadFailedAndFinishedEvent(fileUploadId, event.getException()); } @Override diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/FileTransferHandlerVaadinUpload.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/FileTransferHandlerVaadinUpload.java index f6fe169c7..027f91cb7 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/FileTransferHandlerVaadinUpload.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/FileTransferHandlerVaadinUpload.java @@ -12,12 +12,11 @@ import java.io.IOException; import java.io.OutputStream; import java.io.PipedInputStream; import java.io.PipedOutputStream; -import java.util.Optional; import org.eclipse.hawkbit.repository.ArtifactManagement; +import org.eclipse.hawkbit.repository.RegexCharacterCollection; import org.eclipse.hawkbit.repository.SoftwareModuleManagement; import org.eclipse.hawkbit.repository.model.SoftwareModule; -import org.eclipse.hawkbit.ui.artifacts.upload.FileUploadProgress.FileUploadStatus; import org.eclipse.hawkbit.ui.utils.VaadinMessageSource; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -71,49 +70,41 @@ public class FileTransferHandlerVaadinUpload extends AbstractFileTransferHandler public void uploadStarted(final StartedEvent event) { // reset internal state here because instance is reused for next upload! resetState(); - this.fileUploadId = null; - assertThatOneSoftwareModuleIsSelected(); - - // selected software module at the time of this callback is - // considered - SoftwareModule softwareModule = null; - final Optional selectedSoftwareModuleId = getUploadState().getSelectedBaseSwModuleId(); - final Long softwareModuleId = selectedSoftwareModuleId.orElse(null); - if (softwareModuleId != null) { - softwareModule = softwareModuleManagement.get(softwareModuleId).orElse(null); - } + final SoftwareModule softwareModule = getSelectedSoftwareModule(); this.fileUploadId = new FileUploadId(event.getFilename(), softwareModule); if (getUploadState().isFileInUploadState(this.fileUploadId)) { - setFailureReasonUploadFailed(); // actual interrupt will happen a bit late so setting the below // flag - setDuplicateFile(); + interruptUploadDueToDuplicateFile(); event.getUpload().interruptUpload(); } else { LOG.info("Uploading file {}", fileUploadId); publishUploadStarted(fileUploadId); - if (isFileAlreadyContainedInSoftwareModule(fileUploadId, softwareModule)) { + if (RegexCharacterCollection.stringContainsCharacter(event.getFilename(), ILLEGAL_FILENAME_CHARACTERS)) { + LOG.info("Filename contains illegal characters {} for upload {}", fileUploadId.getFilename(), + fileUploadId); + interruptUploadDueToIllegalFilename(); + event.getUpload().interruptUpload(); + }else if (isFileAlreadyContainedInSoftwareModule(fileUploadId, softwareModule)) { LOG.info("File {} already contained in Software Module {}", fileUploadId.getFilename(), softwareModule); - getUploadState().updateFileUploadProgress(fileUploadId, - new FileUploadProgress(fileUploadId, FileUploadStatus.UPLOAD_FAILED)); - setDuplicateFile(); + interruptUploadDueToDuplicateFile(); event.getUpload().interruptUpload(); } } } - private void assertThatOneSoftwareModuleIsSelected() { - // FileUpload button should be disabled if no SoftwareModul or more - // than one is selected! - if (getUploadState().isNoSoftwareModuleSelected()) { - throw new IllegalStateException("No SoftwareModul selected"); - } else if (getUploadState().isMoreThanOneSoftwareModulesSelected()) { + private SoftwareModule getSelectedSoftwareModule() { + if (getUploadState().isMoreThanOneSoftwareModulesSelected()) { throw new IllegalStateException("More than one SoftwareModul selected but only one is allowed"); } + final long selectedId = getUploadState().getSelectedBaseSwModuleId() + .orElseThrow(() -> new IllegalStateException("No SoftwareModul selected")); + return softwareModuleManagement.get(selectedId) + .orElseThrow(() -> new IllegalStateException("SoftwareModul with unknown ID selected")); } /** @@ -141,11 +132,8 @@ public class FileTransferHandlerVaadinUpload extends AbstractFileTransferHandler LOG.error("Creating piped Stream failed {}.", e); tryToCloseIOStream(outputStream); tryToCloseIOStream(inputStream); - setFailureReasonUploadFailed(); - setUploadInterrupted(); - getUploadState().updateFileUploadProgress(fileUploadId, - new FileUploadProgress(fileUploadId, FileUploadStatus.UPLOAD_FAILED)); - uiNotification.displayWarning("try again"); + interruptUploadDueToUploadFailed(); + publishUploadFailedAndFinishedEvent(fileUploadId, e); return ByteStreams.nullOutputStream(); } return outputStream; @@ -160,8 +148,7 @@ public class FileTransferHandlerVaadinUpload extends AbstractFileTransferHandler public void updateProgress(final long readBytes, final long contentLength) { if (readBytes > maxSize || contentLength > maxSize) { LOG.error("User tried to upload more than was allowed ({}).", maxSize); - setFailureReasonFileSizeExceeded(maxSize); - setUploadInterrupted(); + interruptUploadDueToFileSizeExceeded(maxSize); return; } if (isUploadInterrupted()) { @@ -209,14 +196,15 @@ public class FileTransferHandlerVaadinUpload extends AbstractFileTransferHandler public void uploadFailed(final FailedEvent event) { assertStateConsistency(fileUploadId, event.getFilename()); - if (isDuplicateFile()) { - publishUploadFailedEvent(fileUploadId, getI18n().getMessage("message.no.duplicateFiles"), - event.getReason()); - } else { - publishUploadFailedEvent(fileUploadId, event.getReason()); + if (!isUploadInterrupted()) { + interruptUploadDueToUploadFailed(); } - setUploadInterrupted(); - publishUploadFinishedEvent(fileUploadId); + publishUploadFailedAndFinishedEvent(fileUploadId, event.getReason()); } + @Override + protected void resetState() { + super.resetState(); + this.fileUploadId = null; + } } diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/UploadDropAreaLayout.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/UploadDropAreaLayout.java index 1b5c1d745..d3d55265a 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/UploadDropAreaLayout.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/artifacts/upload/UploadDropAreaLayout.java @@ -190,8 +190,7 @@ public class UploadDropAreaLayout extends AbstractComponent { } } if (duplicateFound) { - uiNotification.displayValidationError( - i18n.getMessage("message.no.duplicateFiles")); + uiNotification.displayValidationError(i18n.getMessage("message.no.duplicateFiles")); } } diff --git a/hawkbit-ui/src/main/resources/messages.properties b/hawkbit-ui/src/main/resources/messages.properties index 645c1199d..b57b09272 100644 --- a/hawkbit-ui/src/main/resources/messages.properties +++ b/hawkbit-ui/src/main/resources/messages.properties @@ -435,6 +435,7 @@ message.swModule.deleted = {0} Software Module(s) deleted message.error.swModule.notDeleted = Upload is running for Software Module(s) message.upload.failed = Streaming Failed message.uploadedfile.size.exceeded = File size exceeded. Allowed size {0} bytes +message.uploadedfile.illegalFilename = Filename contains illegal characters message.artifact.deleted = Artifact with file {0} deleted successfully @@ -672,3 +673,9 @@ message.confirm.delete.entity = Are you sure you want to delete {0} {1}{2}? caption.entity.assign.action.confirmbox = Confirm Assignment message.confirm.assign.entity = Are you sure you want to assign distribution {0} to {1} {2}? message.confirm.assign.multiple.entities = Are you sure you want to assign {0} {1} to distribution {2}? + +# character descriptions +character.whitespace = whitespace +character.digits = digits +character.quotationMarks = quotation marks +character.slashes = slashes