From 3118090fdc38de64e0a7aef969b3b1992c1703f5 Mon Sep 17 00:00:00 2001 From: Bert Blommers Date: Fri, 30 Sep 2022 10:07:20 +0000 Subject: [PATCH] DynamoDB: Validate duplicate expression paths (#5504) --- moto/dynamodb/exceptions.py | 7 ++++ moto/dynamodb/parsing/ast_nodes.py | 10 ++++- .../exceptions/test_dynamodb_exceptions.py | 39 +++++++++++++++++++ 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/moto/dynamodb/exceptions.py b/moto/dynamodb/exceptions.py index 01e634421..973a4721f 100644 --- a/moto/dynamodb/exceptions.py +++ b/moto/dynamodb/exceptions.py @@ -262,6 +262,13 @@ class InvalidAttributeTypeError(MockValidationException): super().__init__(self.msg.format(name, expected_type, actual_type)) +class DuplicateUpdateExpression(InvalidUpdateExpression): + def __init__(self, names): + super().__init__( + f"Two document paths overlap with each other; must remove or rewrite one of these paths; path one: [{names[0]}], path two: [{names[1]}]" + ) + + class TooManyAddClauses(InvalidUpdateExpression): msg = 'The "ADD" section can only be used once in an update expression;' diff --git a/moto/dynamodb/parsing/ast_nodes.py b/moto/dynamodb/parsing/ast_nodes.py index 6c7176d69..b784fcdb5 100644 --- a/moto/dynamodb/parsing/ast_nodes.py +++ b/moto/dynamodb/parsing/ast_nodes.py @@ -3,7 +3,7 @@ from abc import abstractmethod from collections import deque from moto.dynamodb.models import DynamoType -from ..exceptions import TooManyAddClauses +from ..exceptions import DuplicateUpdateExpression, TooManyAddClauses class Node(metaclass=abc.ABCMeta): @@ -26,6 +26,14 @@ class Node(metaclass=abc.ABCMeta): nr_of_clauses = len(self.find_clauses(UpdateExpressionAddClause)) if nr_of_clauses > 1: raise TooManyAddClauses() + set_actions = self.find_clauses(UpdateExpressionSetAction) + set_attributes = [ + c.children[0].children[0].children[0] for c in set_actions + ] + # We currently only check for duplicates + # We should also check for partial duplicates, i.e. [attr, attr.sub] is also invalid + if len(set_attributes) != len(set(set_attributes)): + raise DuplicateUpdateExpression(set_attributes) def find_clauses(self, clause_type): clauses = [] diff --git a/tests/test_dynamodb/exceptions/test_dynamodb_exceptions.py b/tests/test_dynamodb/exceptions/test_dynamodb_exceptions.py index e81f0e2e8..047d569b5 100644 --- a/tests/test_dynamodb/exceptions/test_dynamodb_exceptions.py +++ b/tests/test_dynamodb/exceptions/test_dynamodb_exceptions.py @@ -578,6 +578,45 @@ def test_update_item_non_existent_table(): assert err["Message"].should.equal("Requested resource not found") +@mock_dynamodb +@pytest.mark.parametrize( + "expression", + [ + "set example_column = :example_column, example_column = :example_column", + "set example_column = :example_column ADD x :y set example_column = :example_column", + ], +) +def test_update_item_with_duplicate_expressions(expression): + dynamodb = boto3.resource("dynamodb", region_name="us-east-1") + dynamodb.create_table( + TableName="example_table", + KeySchema=[{"AttributeName": "pk", "KeyType": "HASH"}], + AttributeDefinitions=[{"AttributeName": "pk", "AttributeType": "S"}], + BillingMode="PAY_PER_REQUEST", + ) + record = { + "pk": "example_id", + "example_column": "example", + } + table = dynamodb.Table("example_table") + table.put_item(Item=record) + with pytest.raises(ClientError) as exc: + table.update_item( + Key={"pk": "example_id"}, + UpdateExpression=expression, + ExpressionAttributeValues={":example_column": "test"}, + ) + err = exc.value.response["Error"] + err["Code"].should.equal("ValidationException") + err["Message"].should.equal( + "Invalid UpdateExpression: Two document paths overlap with each other; must remove or rewrite one of these paths; path one: [example_column], path two: [example_column]" + ) + + # The item is not updated + item = table.get_item(Key={"pk": "example_id"})["Item"] + item.should.equal({"pk": "example_id", "example_column": "example"}) + + @mock_dynamodb def test_put_item_wrong_datatype(): if settings.TEST_SERVER_MODE: