diff --git a/hawkbit-core/src/main/java/org/eclipse/hawkbit/exception/SpServerError.java b/hawkbit-core/src/main/java/org/eclipse/hawkbit/exception/SpServerError.java index e443260a6..737b6b50f 100644 --- a/hawkbit-core/src/main/java/org/eclipse/hawkbit/exception/SpServerError.java +++ b/hawkbit-core/src/main/java/org/eclipse/hawkbit/exception/SpServerError.java @@ -44,6 +44,11 @@ public enum SpServerError { */ SP_REPO_CONCURRENT_MODIFICATION("hawkbit.server.error.repo.concurrentModification", "The given entity has been changed by another user/session"), + /** + * + */ + SP_TARGET_ATTRIBUTES_INVALID("hawkbit.server.error.repo.invalidTargetAttributes", "The given target attributes are invalid"), + /** * */ diff --git a/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java b/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java index 7001f8500..493597a1e 100644 --- a/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java +++ b/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java @@ -252,31 +252,11 @@ public class AmqpMessageHandlerService extends BaseAmqpService { private void updateAttributes(final Message message) { final DmfAttributeUpdate attributeUpdate = convertMessage(message, DmfAttributeUpdate.class); final String thingId = getStringHeaderKey(message, MessageHeaderKey.THING_ID, "ThingId is null"); - // reject messages with invalid attributes - if (attributeUpdate.getAttributes().entrySet().stream() - .anyMatch(e -> !AmqpMessageHandlerService.isAttributeEntryValid(e))) { - throw new MessageConversionException( - "The received UPDATE_ATTRIBUTES message contains attribute entries which violate the existing length constraints (keys must not exceed 32 characters, values must not exceed 128 characters)."); - } + controllerManagement.updateControllerAttributes(thingId, attributeUpdate.getAttributes(), getUpdateMode(attributeUpdate)); } - private static boolean isAttributeEntryValid(final Entry e) { - if (e == null) { - return true; - } - return isAttributeKeyValid(e.getKey()) && isAttributeValueValid(e.getValue()); - } - - private static boolean isAttributeKeyValid(final String key) { - return key != null && key.length() <= 32; - } - - private static boolean isAttributeValueValid(final String value) { - return value == null || value.length() <= 128; - } - /** * Method to update the action status of an action through the event. * diff --git a/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/DelayedRequeueExceptionStrategy.java b/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/DelayedRequeueExceptionStrategy.java index 13cc8c914..483b19324 100644 --- a/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/DelayedRequeueExceptionStrategy.java +++ b/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/DelayedRequeueExceptionStrategy.java @@ -15,6 +15,7 @@ import javax.validation.ConstraintViolationException; import org.eclipse.hawkbit.repository.exception.CancelActionNotAllowedException; import org.eclipse.hawkbit.repository.exception.EntityNotFoundException; import org.eclipse.hawkbit.repository.exception.InvalidTargetAddressException; +import org.eclipse.hawkbit.repository.exception.InvalidTargetAttributeException; import org.eclipse.hawkbit.repository.exception.QuotaExceededException; import org.eclipse.hawkbit.repository.exception.TenantNotExistException; import org.slf4j.Logger; @@ -79,7 +80,15 @@ public class DelayedRequeueExceptionStrategy extends ConditionalRejectingErrorHa } private static boolean invalidContent(final Throwable cause) { - return cause instanceof ConstraintViolationException || cause instanceof InvalidTargetAddressException - || cause instanceof MessageConversionException || cause instanceof MessageHandlingException; + return isRepositoryException(cause) || isMessageException(cause); + } + + private static boolean isRepositoryException(final Throwable cause) { + return cause instanceof ConstraintViolationException || cause instanceof InvalidTargetAttributeException; + } + + private static boolean isMessageException(final Throwable cause) { + return cause instanceof InvalidTargetAddressException || cause instanceof MessageConversionException + || cause instanceof MessageHandlingException; } } diff --git a/hawkbit-dmf/hawkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/integration/AmqpMessageHandlerServiceIntegrationTest.java b/hawkbit-dmf/hawkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/integration/AmqpMessageHandlerServiceIntegrationTest.java index 41e6887d8..68bf97577 100644 --- a/hawkbit-dmf/hawkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/integration/AmqpMessageHandlerServiceIntegrationTest.java +++ b/hawkbit-dmf/hawkbit-dmf-amqp/src/test/java/org/eclipse/hawkbit/integration/AmqpMessageHandlerServiceIntegrationTest.java @@ -785,6 +785,33 @@ public class AmqpMessageHandlerServiceIntegrationTest extends AmqpServiceIntegra verifyOneDeadLetterMessage(); } + @Test + @Description("Verifies that sending an UPDATE_ATTRIBUTES message with invalid attributes is handled correctly.") + public void updateAttributesWithInvalidValues() { + // setup + final String target = "ControllerAttributeTestTarget"; + registerAndAssertTargetWithExistingTenant(target); + final String keyTooLong = "123456789012345678901234567890123"; + final String keyValid = "12345678901234567890123456789012"; + final String valueTooLong = "123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789"; + final String valueValid = "12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678"; + + sendUpdateAttributesMessageWithGivenAttributes(target, keyTooLong, valueValid); + + sendUpdateAttributesMessageWithGivenAttributes(target, keyTooLong, valueTooLong); + + sendUpdateAttributesMessageWithGivenAttributes(target, keyValid, valueTooLong); + + verifyNumberOfDeadLetterMessages(3); + } + + private void sendUpdateAttributesMessageWithGivenAttributes(String target, String key, String value) { + final DmfAttributeUpdate controllerAttribute = new DmfAttributeUpdate(); + controllerAttribute.getAttributes().put(key, value); + final Message message = createUpdateAttributesMessage(target, TENANT_EXIST, controllerAttribute); + getDmfClient().send(message); + } + private Long registerTargetAndSendActionStatus(final DmfActionStatus sendActionStatus, final String controllerId) { final DistributionSetAssignmentResult assignmentResult = registerTargetAndAssignDistributionSet(controllerId); final Long actionId = assignmentResult.getActions().get(0); @@ -859,8 +886,12 @@ public class AmqpMessageHandlerServiceIntegrationTest extends AmqpServiceIntegra } private void verifyOneDeadLetterMessage() { + verifyNumberOfDeadLetterMessages(1); + } + + private void verifyNumberOfDeadLetterMessages(int numberOfInvocations) { assertEmptyReceiverQueueCount(); createConditionFactory() - .until(() -> Mockito.verify(getDeadletterListener(), Mockito.times(1)).handleMessage(Mockito.any())); + .until(() -> Mockito.verify(getDeadletterListener(), Mockito.times(numberOfInvocations)).handleMessage(Mockito.any())); } } diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/ControllerManagement.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/ControllerManagement.java index 553d00af3..77a4745c8 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/ControllerManagement.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/ControllerManagement.java @@ -25,6 +25,7 @@ import org.eclipse.hawkbit.repository.exception.CancelActionNotAllowedException; import org.eclipse.hawkbit.repository.exception.EntityAlreadyExistsException; import org.eclipse.hawkbit.repository.exception.EntityNotFoundException; import org.eclipse.hawkbit.repository.exception.QuotaExceededException; +import org.eclipse.hawkbit.repository.exception.InvalidTargetAttributeException; import org.eclipse.hawkbit.repository.model.Action; import org.eclipse.hawkbit.repository.model.Action.Status; import org.eclipse.hawkbit.repository.model.ActionStatus; @@ -334,7 +335,9 @@ public interface ControllerManagement { * @throws EntityNotFoundException * if target that has to be updated could not be found * @throws QuotaExceededException - * if maximum number of attribzes per target is exceeded + * if maximum number of attributes per target is exceeded + * @throws InvalidTargetAttributeException + * if attributes violate constraints */ @PreAuthorize(SpringEvalExpressions.IS_CONTROLLER) Target updateControllerAttributes(@NotEmpty String controllerId, @NotNull Map attributes, diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/exception/InvalidTargetAttributeException.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/exception/InvalidTargetAttributeException.java new file mode 100644 index 000000000..ca704156c --- /dev/null +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/exception/InvalidTargetAttributeException.java @@ -0,0 +1,26 @@ +/** + * Copyright (c) 2018 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.exception; + +import org.eclipse.hawkbit.exception.AbstractServerRtException; +import org.eclipse.hawkbit.exception.SpServerError; + +public class InvalidTargetAttributeException extends AbstractServerRtException { + + private static final long serialVersionUID = 1L; + private static final SpServerError THIS_ERROR = SpServerError.SP_TARGET_ATTRIBUTES_INVALID; + + /** + * Default constructor. + */ + public InvalidTargetAttributeException() { + super(THIS_ERROR); + } +} diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/Target.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/Target.java index 3b88d2e42..ca7bdf883 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/Target.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/model/Target.java @@ -35,6 +35,16 @@ public interface Target extends NamedEntity { */ int ADDRESS_MAX_SIZE = 512; + /** + * Maximum length of key of controller attribute + */ + int CONTROLLER_ATTRIBUTE_KEY_SIZE = 32; + + /** + * Maximum length of value of controller attribute + */ + int CONTROLLER_ATTRIBUTE_VALUE_SIZE = 128; + /** * @return business identifier of the {@link Target} */ diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaControllerManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaControllerManagement.java index e97cb81f8..f5433b2af 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaControllerManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaControllerManagement.java @@ -44,6 +44,7 @@ import org.eclipse.hawkbit.repository.event.remote.TargetPollEvent; import org.eclipse.hawkbit.repository.event.remote.entity.CancelTargetAssignmentEvent; import org.eclipse.hawkbit.repository.exception.CancelActionNotAllowedException; import org.eclipse.hawkbit.repository.exception.EntityNotFoundException; +import org.eclipse.hawkbit.repository.exception.InvalidTargetAttributeException; import org.eclipse.hawkbit.repository.jpa.builder.JpaActionStatusCreate; import org.eclipse.hawkbit.repository.jpa.configuration.Constants; import org.eclipse.hawkbit.repository.jpa.executor.AfterTransactionCommitExecutor; @@ -96,6 +97,9 @@ import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Sets; +import static org.eclipse.hawkbit.repository.model.Target.CONTROLLER_ATTRIBUTE_KEY_SIZE; +import static org.eclipse.hawkbit.repository.model.Target.CONTROLLER_ATTRIBUTE_VALUE_SIZE; + /** * JPA based {@link ControllerManagement} implementation. * @@ -657,6 +661,15 @@ public class JpaControllerManagement implements ControllerManagement { ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY)) public Target updateControllerAttributes(final String controllerId, final Map data, final UpdateMode mode) { + + /* + Constraints on attribute keys & values are not validated by EclipseLink. Hence, they are validated here. + */ + if (data.entrySet().stream() + .anyMatch(e -> !isAttributeEntryValid(e))) { + throw new InvalidTargetAttributeException(); + } + final JpaTarget target = (JpaTarget) targetRepository.findByControllerId(controllerId) .orElseThrow(() -> new EntityNotFoundException(Target.class, controllerId)); @@ -689,15 +702,27 @@ public class JpaControllerManagement implements ControllerManagement { return targetRepository.save(target); } + private static boolean isAttributeEntryValid(final Map.Entry e) { + return isAttributeKeyValid(e.getKey()) && isAttributeValueValid(e.getValue()); + } + + private static boolean isAttributeKeyValid(final String key) { + return key != null && key.length() <= CONTROLLER_ATTRIBUTE_KEY_SIZE; + } + + private static boolean isAttributeValueValid(final String value) { + return value == null || value.length() <= CONTROLLER_ATTRIBUTE_VALUE_SIZE; + } + private static void copy(final Map src, final Map trg) { if (src == null || src.isEmpty()) { return; } - src.entrySet().forEach(e -> { - if (e.getValue() != null) { - trg.put(e.getKey(), e.getValue()); + src.forEach((key, value) -> { + if (value != null) { + trg.put(key, value); } else { - trg.remove(e.getKey()); + trg.remove(key); } }); } 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 7697b26f0..e42e0afbc 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 @@ -16,6 +16,7 @@ import static org.junit.Assert.fail; import java.io.ByteArrayInputStream; import java.net.URISyntaxException; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -38,6 +39,7 @@ import org.eclipse.hawkbit.repository.event.remote.entity.SoftwareModuleUpdatedE import org.eclipse.hawkbit.repository.event.remote.entity.TargetCreatedEvent; import org.eclipse.hawkbit.repository.event.remote.entity.TargetUpdatedEvent; import org.eclipse.hawkbit.repository.exception.CancelActionNotAllowedException; +import org.eclipse.hawkbit.repository.exception.InvalidTargetAttributeException; import org.eclipse.hawkbit.repository.exception.QuotaExceededException; import org.eclipse.hawkbit.repository.model.Action; import org.eclipse.hawkbit.repository.model.Action.Status; @@ -835,6 +837,39 @@ public class ControllerManagementTest extends AbstractJpaIntegrationTest { controllerManagement.updateControllerAttributes(controllerId, testData, null); } + @Test + @Description("Checks if invalid values of attribute-key and attribute-value are handled correctly") + public void updateTargetAttributesFailsForInvalidAttributes() { + final String keyTooLong = "123456789012345678901234567890123"; + final String keyValid = "12345678901234567890123456789012"; + final String valueTooLong = "123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789"; + final String valueValid = "12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678"; + final String keyNull = null; + + final String controllerId = "targetId123"; + testdataFactory.createTarget(controllerId); + + assertThatExceptionOfType(InvalidTargetAttributeException.class) + .isThrownBy(() -> controllerManagement.updateControllerAttributes(controllerId, + Collections.singletonMap(keyTooLong, valueValid), null)) + .as("Attribute with key too long should not be created"); + + assertThatExceptionOfType(InvalidTargetAttributeException.class) + .isThrownBy(() -> controllerManagement.updateControllerAttributes(controllerId, + Collections.singletonMap(keyTooLong, valueTooLong), null)) + .as("Attribute with key too long and value too long should not be created"); + + assertThatExceptionOfType(InvalidTargetAttributeException.class) + .isThrownBy(() -> controllerManagement.updateControllerAttributes(controllerId, + Collections.singletonMap(keyValid, valueTooLong), null)) + .as("Attribute with value too long should not be created"); + + assertThatExceptionOfType(InvalidTargetAttributeException.class) + .isThrownBy(() -> controllerManagement.updateControllerAttributes(controllerId, + Collections.singletonMap(keyNull, valueValid), null)) + .as("Attribute with key NULL should not be created"); + } + @Test @Description("Controller providing status entries fails if providing more than permitted by quota.") @ExpectEvents({ @Expect(type = TargetCreatedEvent.class, count = 1), diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/TargetManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/TargetManagementTest.java index f13b0f39b..fa7dcdd49 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/TargetManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/TargetManagementTest.java @@ -17,6 +17,7 @@ import java.net.URI; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -69,7 +70,7 @@ public class TargetManagementTest extends AbstractJpaIntegrationTest { private static final String WHITESPACE_ERROR = "target with whitespaces in controller id should not be created"; @Test - @Description("Verifies that management get access react as specfied on calls for non existing entities by means " + @Description("Verifies that management get access react as specified on calls for non existing entities by means " + "of Optional not present.") @ExpectEvents({ @Expect(type = TargetCreatedEvent.class, count = 0) }) public void nonExistingEntityAccessReturnsNotPresent() { @@ -78,7 +79,7 @@ public class TargetManagementTest extends AbstractJpaIntegrationTest { } @Test - @Description("Verifies that management queries react as specfied on calls for non existing entities " + @Description("Verifies that management queries react as specified on calls for non existing entities " + " by means of throwing EntityNotFoundException.") @ExpectEvents({ @Expect(type = TargetCreatedEvent.class, count = 1), @Expect(type = TargetTagCreatedEvent.class, count = 1) }) @@ -87,8 +88,8 @@ public class TargetManagementTest extends AbstractJpaIntegrationTest { final Target target = testdataFactory.createTarget(); verifyThrownExceptionBy( - () -> targetManagement.assignTag(Arrays.asList(target.getControllerId()), NOT_EXIST_IDL), "TargetTag"); - verifyThrownExceptionBy(() -> targetManagement.assignTag(Arrays.asList(NOT_EXIST_ID), tag.getId()), "Target"); + () -> targetManagement.assignTag(Collections.singletonList(target.getControllerId()), NOT_EXIST_IDL), "TargetTag"); + verifyThrownExceptionBy(() -> targetManagement.assignTag(Collections.singletonList(NOT_EXIST_ID), tag.getId()), "Target"); verifyThrownExceptionBy(() -> targetManagement.findByTag(PAGE, NOT_EXIST_IDL), "TargetTag"); verifyThrownExceptionBy(() -> targetManagement.findByRsqlAndTag(PAGE, "name==*", NOT_EXIST_IDL), "TargetTag"); @@ -103,7 +104,7 @@ public class TargetManagementTest extends AbstractJpaIntegrationTest { "DistributionSet"); verifyThrownExceptionBy(() -> targetManagement.deleteByControllerID(NOT_EXIST_ID), "Target"); - verifyThrownExceptionBy(() -> targetManagement.delete(Arrays.asList(NOT_EXIST_IDL)), "Target"); + verifyThrownExceptionBy(() -> targetManagement.delete(Collections.singletonList(NOT_EXIST_IDL)), "Target"); verifyThrownExceptionBy(() -> targetManagement.findByTargetFilterQueryAndNonDS(PAGE, NOT_EXIST_IDL, "name==*"), "DistributionSet"); @@ -122,9 +123,9 @@ public class TargetManagementTest extends AbstractJpaIntegrationTest { "DistributionSet"); verifyThrownExceptionBy( - () -> targetManagement.toggleTagAssignment(Arrays.asList(target.getControllerId()), NOT_EXIST_ID), + () -> targetManagement.toggleTagAssignment(Collections.singletonList(target.getControllerId()), NOT_EXIST_ID), "TargetTag"); - verifyThrownExceptionBy(() -> targetManagement.toggleTagAssignment(Arrays.asList(NOT_EXIST_ID), tag.getName()), + verifyThrownExceptionBy(() -> targetManagement.toggleTagAssignment(Collections.singletonList(NOT_EXIST_ID), tag.getName()), "Target"); verifyThrownExceptionBy(() -> targetManagement.unAssignTag(NOT_EXIST_ID, tag.getId()), "Target"); @@ -142,20 +143,14 @@ public class TargetManagementTest extends AbstractJpaIntegrationTest { // retrieve security token only with READ_TARGET_SEC_TOKEN permission final String securityTokenWithReadPermission = securityRule.runAs(WithSpringAuthorityRule - .withUser("OnlyTargetReadPermission", false, SpPermission.READ_TARGET_SEC_TOKEN.toString()), () -> { - return createdTarget.getSecurityToken(); - }); + .withUser("OnlyTargetReadPermission", false, SpPermission.READ_TARGET_SEC_TOKEN), createdTarget::getSecurityToken); // retrieve security token as system code execution - final String securityTokenAsSystemCode = systemSecurityContext.runAsSystem(() -> { - return createdTarget.getSecurityToken(); - }); + final String securityTokenAsSystemCode = systemSecurityContext.runAsSystem(createdTarget::getSecurityToken); // retrieve security token without any permissions final String securityTokenWithoutPermission = securityRule - .runAs(WithSpringAuthorityRule.withUser("NoPermission", false), () -> { - return createdTarget.getSecurityToken(); - }); + .runAs(WithSpringAuthorityRule.withUser("NoPermission", false), createdTarget::getSecurityToken); assertThat(createdTarget.getSecurityToken()).isEqualTo("token"); assertThat(securityTokenWithReadPermission).isNotNull(); @@ -237,7 +232,7 @@ public class TargetManagementTest extends AbstractJpaIntegrationTest { assertThatExceptionOfType(ConstraintViolationException.class) .isThrownBy(() -> targetManagement .create(entityFactory.target().create().controllerId("a").name(INVALID_TEXT_HTML))) - .as("target with invalidname should not be created"); + .as("target with invalid name should not be created"); assertThatExceptionOfType(ConstraintViolationException.class) .isThrownBy(() -> targetManagement.update(entityFactory.target().update(target.getControllerId()) @@ -394,12 +389,12 @@ public class TargetManagementTest extends AbstractJpaIntegrationTest { public void deleteAndCreateTargets() { Target target = targetManagement.create(entityFactory.target().create().controllerId("targetId123")); assertThat(targetManagement.count()).as("target count is wrong").isEqualTo(1); - targetManagement.delete(Arrays.asList(target.getId())); + targetManagement.delete(Collections.singletonList(target.getId())); assertThat(targetManagement.count()).as("target count is wrong").isEqualTo(0); target = createTargetWithAttributes("4711"); assertThat(targetManagement.count()).as("target count is wrong").isEqualTo(1); - targetManagement.delete(Arrays.asList(target.getId())); + targetManagement.delete(Collections.singletonList(target.getId())); assertThat(targetManagement.count()).as("target count is wrong").isEqualTo(0); final List targets = new ArrayList<>(); @@ -426,7 +421,7 @@ public class TargetManagementTest extends AbstractJpaIntegrationTest { } @Test - @Description("Finds a target by given ID and checks if all data is in the reponse (including the data defined as lazy).") + @Description("Finds a target by given ID and checks if all data is in the response (including the data defined as lazy).") @ExpectEvents({ @Expect(type = DistributionSetCreatedEvent.class, count = 2), @Expect(type = TargetCreatedEvent.class, count = 1), @Expect(type = TargetUpdatedEvent.class, count = 5), @Expect(type = ActionCreatedEvent.class, count = 2), @Expect(type = ActionUpdatedEvent.class, count = 1), @@ -561,8 +556,8 @@ public class TargetManagementTest extends AbstractJpaIntegrationTest { Long modifiedAt = savedTarget.getLastModifiedAt(); assertThat(createdAt).as("CreatedAt compared with modifiedAt").isEqualTo(modifiedAt); - assertNotNull("The createdAt attribut of the target should no be null", savedTarget.getCreatedAt()); - assertNotNull("The lastModifiedAt attribut of the target should no be null", savedTarget.getLastModifiedAt()); + assertNotNull("The createdAt attribute of the target should no be null", savedTarget.getCreatedAt()); + assertNotNull("The lastModifiedAt attribute of the target should no be null", savedTarget.getLastModifiedAt()); Thread.sleep(1); savedTarget = targetManagement.update( @@ -586,7 +581,7 @@ public class TargetManagementTest extends AbstractJpaIntegrationTest { @Test @WithUser(allSpPermissions = true) - @Description("Create multiple tragets as bulk operation and delete them in bulk.") + @Description("Create multiple targets as bulk operation and delete them in bulk.") @ExpectEvents({ @Expect(type = TargetCreatedEvent.class, count = 101), @Expect(type = TargetUpdatedEvent.class, count = 100), @Expect(type = TargetDeletedEvent.class, count = 51) }) @@ -644,14 +639,14 @@ public class TargetManagementTest extends AbstractJpaIntegrationTest { targetManagement.delete(targetsIdsToDelete); final List targetsLeft = targetManagement.findAll(new PageRequest(0, 200)).getContent(); - assertThat(firstList.spliterator().getExactSizeIfKnown() - numberToDelete).as("Size of splited list") + assertThat(firstList.spliterator().getExactSizeIfKnown() - numberToDelete).as("Size of split list") .isEqualTo(targetsLeft.spliterator().getExactSizeIfKnown()); assertThat(targetsLeft).as("Not all undeleted found").doesNotContain(deletedTargets); } @Test - @Description("Tests the assigment of tags to the a single target.") + @Description("Tests the assignment of tags to the a single target.") @ExpectEvents({ @Expect(type = TargetCreatedEvent.class, count = 2), @Expect(type = TargetTagCreatedEvent.class, count = 7), @Expect(type = TargetUpdatedEvent.class, count = 7) }) @@ -661,11 +656,11 @@ public class TargetManagementTest extends AbstractJpaIntegrationTest { final int noT1Tags = 3; final List t1Tags = testdataFactory.createTargetTags(noT1Tags, "tag1"); - t1Tags.forEach(tag -> targetManagement.assignTag(Arrays.asList(t1.getControllerId()), tag.getId())); + t1Tags.forEach(tag -> targetManagement.assignTag(Collections.singletonList(t1.getControllerId()), tag.getId())); final Target t2 = testdataFactory.createTarget("id-2"); final List t2Tags = testdataFactory.createTargetTags(noT2Tags, "tag2"); - t2Tags.forEach(tag -> targetManagement.assignTag(Arrays.asList(t2.getControllerId()), tag.getId())); + t2Tags.forEach(tag -> targetManagement.assignTag(Collections.singletonList(t2.getControllerId()), tag.getId())); final Target t11 = targetManagement.getByControllerID(t1.getControllerId()).get(); assertThat(targetTagManagement.findByTarget(PAGE, t11.getControllerId()).getContent()).as("Tag size is wrong") @@ -681,7 +676,7 @@ public class TargetManagementTest extends AbstractJpaIntegrationTest { } @Test - @Description("Tests the assigment of tags to multiple targets.") + @Description("Tests the assignment of tags to multiple targets.") @ExpectEvents({ @Expect(type = TargetCreatedEvent.class, count = 50), @Expect(type = TargetTagCreatedEvent.class, count = 4), @Expect(type = TargetUpdatedEvent.class, count = 80) }) diff --git a/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiConfigDataTest.java b/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiConfigDataTest.java index d9d3de514..9984de002 100644 --- a/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiConfigDataTest.java +++ b/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiConfigDataTest.java @@ -18,10 +18,12 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers. import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; +import java.util.Collections; import java.util.HashMap; import java.util.Map; import org.eclipse.hawkbit.exception.SpServerError; +import org.eclipse.hawkbit.repository.exception.InvalidTargetAttributeException; import org.eclipse.hawkbit.repository.exception.QuotaExceededException; import org.eclipse.hawkbit.repository.model.Target; import org.eclipse.hawkbit.rest.util.JsonBuilder; @@ -46,6 +48,11 @@ import ru.yandex.qatools.allure.annotations.Stories; @Stories("Config Data Resource") public class DdiConfigDataTest extends AbstractDDiApiIntegrationTest { + private static final String KEY_TOO_LONG = "123456789012345678901234567890123"; + private static final String KEY_VALID = "12345678901234567890123456789012"; + private static final String VALUE_TOO_LONG = "123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789"; + private static final String VALUE_VALID = "12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678"; + @Test @Description("We verify that the config data (i.e. device attributes like serial number, hardware revision etc.) " + "are requested only once from the device.") @@ -94,7 +101,7 @@ public class DdiConfigDataTest extends AbstractDDiApiIntegrationTest { // initial final Map attributes = new HashMap<>(); - attributes.put("dsafsdf", "sdsds"); + attributes.put(KEY_VALID, VALUE_VALID); mvc.perform(put("/{tenant}/controller/v1/4717/configData", tenantAware.getCurrentTenant()) .content(JsonBuilder.configData("", attributes, "closed")).contentType(MediaType.APPLICATION_JSON)) @@ -136,7 +143,7 @@ public class DdiConfigDataTest extends AbstractDDiApiIntegrationTest { @Test @Description("We verify that the config data (i.e. device attributes like serial number, hardware revision etc.) " - + "resource behaves as exptected in cae of invalid request attempts.") + + "resource behaves as expected in case of invalid request attempts.") public void badConfigData() throws Exception { testdataFactory.createTarget("4712"); @@ -169,6 +176,43 @@ public class DdiConfigDataTest extends AbstractDDiApiIntegrationTest { .andDo(MockMvcResultPrinter.print()).andExpect(status().isBadRequest()); } + @Test + @Description("Verifies that invalid config data attributes are handled correctly.") + public void putConfigDataWithInvalidAttributes() throws Exception { + // create a target + final String controllerId = "4718"; + testdataFactory.createTarget(controllerId); + final String configDataPath = "/{tenant}/controller/v1/" + controllerId + "/configData"; + + putAndVerifyConfigDataWithKeyTooLong(configDataPath); + + putAndVerifyConfigDataWithValueTooLong(configDataPath); + } + + @Step + private void putAndVerifyConfigDataWithKeyTooLong(final String configDataPath) throws Exception { + + final Map attributes = Collections.singletonMap(KEY_TOO_LONG, VALUE_VALID); + + mvc.perform(put(configDataPath, tenantAware.getCurrentTenant()) + .content(JsonBuilder.configData("", attributes, "closed")).contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.exceptionClass", equalTo(InvalidTargetAttributeException.class.getName()))) + .andExpect(jsonPath("$.errorCode", equalTo(SpServerError.SP_TARGET_ATTRIBUTES_INVALID.getKey()))); + } + + @Step + private void putAndVerifyConfigDataWithValueTooLong(final String configDataPath) throws Exception { + + final Map attributes = Collections.singletonMap(KEY_VALID, VALUE_TOO_LONG); + + mvc.perform(put(configDataPath, tenantAware.getCurrentTenant()) + .content(JsonBuilder.configData("", attributes, "closed")).contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.exceptionClass", equalTo(InvalidTargetAttributeException.class.getName()))) + .andExpect(jsonPath("$.errorCode", equalTo(SpServerError.SP_TARGET_ATTRIBUTES_INVALID.getKey()))); + } + @Test @Description("Verify that config data (device attributes) can be updated by the controller using different update modes (merge, replace, remove).") public void putConfigDataWithDifferentUpdateModes() throws Exception { diff --git a/hawkbit-rest/hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/exception/ResponseExceptionHandler.java b/hawkbit-rest/hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/exception/ResponseExceptionHandler.java index b12401fef..817733a9d 100644 --- a/hawkbit-rest/hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/exception/ResponseExceptionHandler.java +++ b/hawkbit-rest/hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/exception/ResponseExceptionHandler.java @@ -75,6 +75,7 @@ public class ResponseExceptionHandler { ERROR_TO_HTTP_STATUS.put(SpServerError.SP_REPO_OPERATION_NOT_SUPPORTED, HttpStatus.GONE); ERROR_TO_HTTP_STATUS.put(SpServerError.SP_REPO_CONCURRENT_MODIFICATION, HttpStatus.CONFLICT); ERROR_TO_HTTP_STATUS.put(SpServerError.SP_MAINTENANCE_SCHEDULE_INVALID, HttpStatus.BAD_REQUEST); + ERROR_TO_HTTP_STATUS.put(SpServerError.SP_TARGET_ATTRIBUTES_INVALID, HttpStatus.BAD_REQUEST); }