Fix handle invalid controller attributes (#740)
* Move controller attribute validation from DMF to Repository Rational: Constraint validation on key & value of a map is currently not supported by EclipseLink Maven Plugin. Therefore, this check has to be done by hawkBit. The check is required for both APIs (DMF + DDI). * add tests for attribute validation * update tests Signed-off-by: Jeroen Laverman <jeroen.laverman@bosch-si.com> * Review findings Signed-off-by: Jeroen Laverman <jeroen.laverman@bosch-si.com> * * Add custom exception for invalid target attributes * Add integration tests for DDI and DMF Signed-off-by: Jeroen Laverman <jeroen.laverman@bosch-si.com> * * rename exception Signed-off-by: Jeroen Laverman <jeroen.laverman@bosch-si.com> * * rename test steps Signed-off-by: Jeroen Laverman <jeroen.laverman@bosch-si.com> * Fix value size Signed-off-by: Jeroen Laverman <jeroen.laverman@bosch-si.com> * Ensure constraints are validated correctly Signed-off-by: Jeroen Laverman <jeroen.laverman@bosch-si.com> * Introduce review findings Signed-off-by: Jeroen Laverman <jeroen.laverman@bosch-si.com> * fix sonar finding Signed-off-by: Jeroen Laverman <jeroen.laverman@bosch-si.com>
This commit is contained in:
committed by
Dominic Schabel
parent
cab2a6f774
commit
5fe86954b0
@@ -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<String, String> 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.
|
||||
*
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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()));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user