Fix artifact filename validation (#770)
* use validated ArtifactUpload object when creating a new artifact Signed-off-by: Stefan Klotz <stefan.klotz@bosch-si.com> * rename method Signed-off-by: Stefan Klotz <stefan.klotz@bosch-si.com> * add regular expression classes Signed-off-by: Stefan Klotz <stefan.klotz@bosch-si.com> * add filename validation to UI upload button Signed-off-by: Stefan Klotz <stefan.klotz@bosch-si.com> * move filename validation to uploadStarted Signed-off-by: Stefan Klotz <stefan.klotz@bosch-si.com> * clean up code for UI error handling during artifact upload, assert filename validation Signed-off-by: Stefan Klotz <stefan.klotz@bosch-si.com> * update visibilities Signed-off-by: Stefan Klotz <stefan.klotz@bosch-si.com> * clean up code Signed-off-by: Stefan Klotz <stefan.klotz@bosch-si.com> * clean up code Signed-off-by: Stefan Klotz <stefan.klotz@bosch-si.com> * change RegexChar class to enum and use i18n Signed-off-by: Stefan Klotz <stefan.klotz@bosch-si.com> * typo, use StringBuilder Signed-off-by: Stefan Klotz <stefan.klotz@bosch-si.com> * typo Signed-off-by: Stefan Klotz <stefan.klotz@bosch-si.com> * use dedicated class for collections of regular expression characters Signed-off-by: Stefan Klotz <stefan.klotz@bosch-si.com> * remove Optional, remove stringBuilder Signed-off-by: Stefan Klotz <stefan.klotz@bosch-si.com> * PR findings Signed-off-by: Stefan Klotz <stefan.klotz@bosch-si.com> * make regex validation method static Signed-off-by: Stefan Klotz <stefan.klotz@bosch-si.com> * use WhiteListType.NONE for filename validation via mgmt-api Signed-off-by: Stefan Klotz <stefan.klotz@bosch-si.com>
This commit is contained in:
Stefan Klotz
committed by
Dominic Schabel
Dominic Schabel
parent
a2c1e5f132
commit
20d84a10eb
@@ -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();
|
||||
|
||||
@@ -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 = "<img src=ernw onerror=alert(1)>.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) {
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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());
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user