diff --git a/alternator/conditions.cc b/alternator/conditions.cc index 58540fea4b..3837cd8d3a 100644 --- a/alternator/conditions.cc +++ b/alternator/conditions.cc @@ -159,23 +159,40 @@ static bool check_NE(const rjson::value* v1, const rjson::value& v2) { } // Check if two JSON-encoded values match with the BEGINS_WITH relation -static bool check_BEGINS_WITH(const rjson::value* v1, const rjson::value& v2) { - // BEGINS_WITH requires that its single operand (v2) be a string or - // binary - otherwise it's a validation error. However, problems with - // the stored attribute (v1) will just return false (no match). - if (!v2.IsObject() || v2.MemberCount() != 1) { - throw api_error::validation(format("BEGINS_WITH operator encountered malformed AttributeValue: {}", v2)); - } - auto it2 = v2.MemberBegin(); - if (it2->name != "S" && it2->name != "B") { - throw api_error::validation(format("BEGINS_WITH operator requires String or Binary type in AttributeValue, got {}", it2->name)); - } - - +bool check_BEGINS_WITH(const rjson::value* v1, const rjson::value& v2, + bool v1_from_query, bool v2_from_query) { + bool bad = false; if (!v1 || !v1->IsObject() || v1->MemberCount() != 1) { + if (v1_from_query) { + throw api_error::validation("begins_with() encountered malformed argument"); + } else { + bad = true; + } + } else if (v1->MemberBegin()->name != "S" && v1->MemberBegin()->name != "B") { + if (v1_from_query) { + throw api_error::validation(format("begins_with supports only string or binary type, got: {}", *v1)); + } else { + bad = true; + } + } + if (!v2.IsObject() || v2.MemberCount() != 1) { + if (v2_from_query) { + throw api_error::validation("begins_with() encountered malformed argument"); + } else { + bad = true; + } + } else if (v2.MemberBegin()->name != "S" && v2.MemberBegin()->name != "B") { + if (v2_from_query) { + throw api_error::validation(format("begins_with() supports only string or binary type, got: {}", v2)); + } else { + bad = true; + } + } + if (bad) { return false; } auto it1 = v1->MemberBegin(); + auto it2 = v2.MemberBegin(); if (it1->name != it2->name) { return false; } @@ -279,24 +296,38 @@ static bool check_NOT_NULL(const rjson::value* val) { return val != nullptr; } +// Only types S, N or B (string, number or bytes) may be compared by the +// various comparion operators - lt, le, gt, ge, and between. +static bool check_comparable_type(const rjson::value& v) { + if (!v.IsObject() || v.MemberCount() != 1) { + return false; + } + const rjson::value& type = v.MemberBegin()->name; + return type == "S" || type == "N" || type == "B"; +} + // Check if two JSON-encoded values match with cmp. template -bool check_compare(const rjson::value* v1, const rjson::value& v2, const Comparator& cmp) { - if (!v2.IsObject() || v2.MemberCount() != 1) { - throw api_error::validation( - format("{} requires a single AttributeValue of type String, Number, or Binary", - cmp.diagnostic)); +bool check_compare(const rjson::value* v1, const rjson::value& v2, const Comparator& cmp, + bool v1_from_query, bool v2_from_query) { + bool bad = false; + if (!v1 || !check_comparable_type(*v1)) { + if (v1_from_query) { + throw api_error::validation(format("{} allow only the types String, Number, or Binary", cmp.diagnostic)); + } + bad = true; } - const auto& kv2 = *v2.MemberBegin(); - if (kv2.name != "S" && kv2.name != "N" && kv2.name != "B") { - throw api_error::validation( - format("{} requires a single AttributeValue of type String, Number, or Binary", - cmp.diagnostic)); + if (!check_comparable_type(v2)) { + if (v2_from_query) { + throw api_error::validation(format("{} allow only the types String, Number, or Binary", cmp.diagnostic)); + } + bad = true; } - if (!v1 || !v1->IsObject() || v1->MemberCount() != 1) { + if (bad) { return false; } const auto& kv1 = *v1->MemberBegin(); + const auto& kv2 = *v2.MemberBegin(); if (kv1.name != kv2.name) { return false; } @@ -310,7 +341,8 @@ bool check_compare(const rjson::value* v1, const rjson::value& v2, const Compara if (kv1.name == "B") { return cmp(base64_decode(kv1.value), base64_decode(kv2.value)); } - clogger.error("check_compare panic: LHS type equals RHS type, but one is in {N,S,B} while the other isn't"); + // cannot reach here, as check_comparable_type() verifies the type is one + // of the above options. return false; } @@ -341,56 +373,71 @@ struct cmp_gt { static constexpr const char* diagnostic = "GT operator"; }; -// True if v is between lb and ub, inclusive. Throws if lb > ub. +// True if v is between lb and ub, inclusive. Throws or returns false +// (depending on bounds_from_query parameter) if lb > ub. template -static bool check_BETWEEN(const T& v, const T& lb, const T& ub) { +static bool check_BETWEEN(const T& v, const T& lb, const T& ub, bool bounds_from_query) { if (cmp_lt()(ub, lb)) { - throw api_error::validation( - format("BETWEEN operator requires lower_bound <= upper_bound, but {} > {}", lb, ub)); + if (bounds_from_query) { + throw api_error::validation( + format("BETWEEN operator requires lower_bound <= upper_bound, but {} > {}", lb, ub)); + } else { + return false; + } } return cmp_ge()(v, lb) && cmp_le()(v, ub); } -static bool check_BETWEEN(const rjson::value* v, const rjson::value& lb, const rjson::value& ub) { - if (!v) { +static bool check_BETWEEN(const rjson::value* v, const rjson::value& lb, const rjson::value& ub, + bool v_from_query, bool lb_from_query, bool ub_from_query) { + if ((v && v_from_query && !check_comparable_type(*v)) || + (lb_from_query && !check_comparable_type(lb)) || + (ub_from_query && !check_comparable_type(ub))) { + throw api_error::validation("between allow only the types String, Number, or Binary"); + + } + if (!v || !v->IsObject() || v->MemberCount() != 1 || + !lb.IsObject() || lb.MemberCount() != 1 || + !ub.IsObject() || ub.MemberCount() != 1) { return false; } - if (!v->IsObject() || v->MemberCount() != 1) { - throw api_error::validation(format("BETWEEN operator encountered malformed AttributeValue: {}", *v)); - } - if (!lb.IsObject() || lb.MemberCount() != 1) { - throw api_error::validation(format("BETWEEN operator encountered malformed AttributeValue: {}", lb)); - } - if (!ub.IsObject() || ub.MemberCount() != 1) { - throw api_error::validation(format("BETWEEN operator encountered malformed AttributeValue: {}", ub)); - } const auto& kv_v = *v->MemberBegin(); const auto& kv_lb = *lb.MemberBegin(); const auto& kv_ub = *ub.MemberBegin(); + bool bounds_from_query = lb_from_query && ub_from_query; if (kv_lb.name != kv_ub.name) { - throw api_error::validation( + if (bounds_from_query) { + throw api_error::validation( format("BETWEEN operator requires the same type for lower and upper bound; instead got {} and {}", kv_lb.name, kv_ub.name)); + } else { + return false; + } } if (kv_v.name != kv_lb.name) { // Cannot compare different types, so v is NOT between lb and ub. return false; } if (kv_v.name == "N") { const char* diag = "BETWEEN operator"; - return check_BETWEEN(unwrap_number(*v, diag), unwrap_number(lb, diag), unwrap_number(ub, diag)); + return check_BETWEEN(unwrap_number(*v, diag), unwrap_number(lb, diag), unwrap_number(ub, diag), bounds_from_query); } if (kv_v.name == "S") { return check_BETWEEN(std::string_view(kv_v.value.GetString(), kv_v.value.GetStringLength()), std::string_view(kv_lb.value.GetString(), kv_lb.value.GetStringLength()), - std::string_view(kv_ub.value.GetString(), kv_ub.value.GetStringLength())); + std::string_view(kv_ub.value.GetString(), kv_ub.value.GetStringLength()), + bounds_from_query); } if (kv_v.name == "B") { - return check_BETWEEN(base64_decode(kv_v.value), base64_decode(kv_lb.value), base64_decode(kv_ub.value)); + return check_BETWEEN(base64_decode(kv_v.value), base64_decode(kv_lb.value), base64_decode(kv_ub.value), bounds_from_query); } - throw api_error::validation( - format("BETWEEN operator requires AttributeValueList elements to be of type String, Number, or Binary; instead got {}", + if (v_from_query) { + throw api_error::validation( + format("BETWEEN operator requires AttributeValueList elements to be of type String, Number, or Binary; instead got {}", kv_lb.name)); + } else { + return false; + } } // Verify one Expect condition on one attribute (whose content is "got") @@ -437,19 +484,19 @@ static bool verify_expected_one(const rjson::value& condition, const rjson::valu return check_NE(got, (*attribute_value_list)[0]); case comparison_operator_type::LT: verify_operand_count(attribute_value_list, exact_size(1), *comparison_operator); - return check_compare(got, (*attribute_value_list)[0], cmp_lt{}); + return check_compare(got, (*attribute_value_list)[0], cmp_lt{}, false, true); case comparison_operator_type::LE: verify_operand_count(attribute_value_list, exact_size(1), *comparison_operator); - return check_compare(got, (*attribute_value_list)[0], cmp_le{}); + return check_compare(got, (*attribute_value_list)[0], cmp_le{}, false, true); case comparison_operator_type::GT: verify_operand_count(attribute_value_list, exact_size(1), *comparison_operator); - return check_compare(got, (*attribute_value_list)[0], cmp_gt{}); + return check_compare(got, (*attribute_value_list)[0], cmp_gt{}, false, true); case comparison_operator_type::GE: verify_operand_count(attribute_value_list, exact_size(1), *comparison_operator); - return check_compare(got, (*attribute_value_list)[0], cmp_ge{}); + return check_compare(got, (*attribute_value_list)[0], cmp_ge{}, false, true); case comparison_operator_type::BEGINS_WITH: verify_operand_count(attribute_value_list, exact_size(1), *comparison_operator); - return check_BEGINS_WITH(got, (*attribute_value_list)[0]); + return check_BEGINS_WITH(got, (*attribute_value_list)[0], false, true); case comparison_operator_type::IN: verify_operand_count(attribute_value_list, nonempty(), *comparison_operator); return check_IN(got, *attribute_value_list); @@ -461,7 +508,8 @@ static bool verify_expected_one(const rjson::value& condition, const rjson::valu return check_NOT_NULL(got); case comparison_operator_type::BETWEEN: verify_operand_count(attribute_value_list, exact_size(2), *comparison_operator); - return check_BETWEEN(got, (*attribute_value_list)[0], (*attribute_value_list)[1]); + return check_BETWEEN(got, (*attribute_value_list)[0], (*attribute_value_list)[1], + false, true, true); case comparison_operator_type::CONTAINS: { verify_operand_count(attribute_value_list, exact_size(1), *comparison_operator); @@ -573,7 +621,8 @@ static bool calculate_primitive_condition(const parsed::primitive_condition& con // Shouldn't happen unless we have a bug in the parser throw std::logic_error(format("Wrong number of values {} in BETWEEN primitive_condition", cond._values.size())); } - return check_BETWEEN(&calculated_values[0], calculated_values[1], calculated_values[2]); + return check_BETWEEN(&calculated_values[0], calculated_values[1], calculated_values[2], + cond._values[0].is_constant(), cond._values[1].is_constant(), cond._values[2].is_constant()); case parsed::primitive_condition::type::IN: return check_IN(calculated_values); case parsed::primitive_condition::type::VALUE: @@ -604,13 +653,17 @@ static bool calculate_primitive_condition(const parsed::primitive_condition& con case parsed::primitive_condition::type::NE: return check_NE(&calculated_values[0], calculated_values[1]); case parsed::primitive_condition::type::GT: - return check_compare(&calculated_values[0], calculated_values[1], cmp_gt{}); + return check_compare(&calculated_values[0], calculated_values[1], cmp_gt{}, + cond._values[0].is_constant(), cond._values[1].is_constant()); case parsed::primitive_condition::type::GE: - return check_compare(&calculated_values[0], calculated_values[1], cmp_ge{}); + return check_compare(&calculated_values[0], calculated_values[1], cmp_ge{}, + cond._values[0].is_constant(), cond._values[1].is_constant()); case parsed::primitive_condition::type::LT: - return check_compare(&calculated_values[0], calculated_values[1], cmp_lt{}); + return check_compare(&calculated_values[0], calculated_values[1], cmp_lt{}, + cond._values[0].is_constant(), cond._values[1].is_constant()); case parsed::primitive_condition::type::LE: - return check_compare(&calculated_values[0], calculated_values[1], cmp_le{}); + return check_compare(&calculated_values[0], calculated_values[1], cmp_le{}, + cond._values[0].is_constant(), cond._values[1].is_constant()); default: // Shouldn't happen unless we have a bug in the parser throw std::logic_error(format("Unknown type {} in primitive_condition object", (int)(cond._op))); diff --git a/alternator/conditions.hh b/alternator/conditions.hh index 3f9b564ac3..c1e3b5f77d 100644 --- a/alternator/conditions.hh +++ b/alternator/conditions.hh @@ -52,6 +52,7 @@ bool verify_expected(const rjson::value& req, const rjson::value* previous_item) bool verify_condition(const rjson::value& condition, bool require_all, const rjson::value* previous_item); bool check_CONTAINS(const rjson::value* v1, const rjson::value& v2); +bool check_BEGINS_WITH(const rjson::value* v1, const rjson::value& v2, bool v1_from_query, bool v2_from_query); bool verify_condition_expression( const parsed::condition_expression& condition_expression, diff --git a/alternator/expressions.cc b/alternator/expressions.cc index d7aedd52d0..cabc637604 100644 --- a/alternator/expressions.cc +++ b/alternator/expressions.cc @@ -603,52 +603,8 @@ std::unordered_map function_handlers { } rjson::value v1 = calculate_value(f._parameters[0], caller, previous_item); rjson::value v2 = calculate_value(f._parameters[1], caller, previous_item); - // TODO: There's duplication here with check_BEGINS_WITH(). - // But unfortunately, the two functions differ a bit. - - // If one of v1 or v2 is malformed or has an unsupported type - // (not B or S), what we do depends on whether it came from - // the user's query (is_constant()), or the item. Unsupported - // values in the query result in an error, but if they are in - // the item, we silently return false (no match). - bool bad = false; - if (!v1.IsObject() || v1.MemberCount() != 1) { - bad = true; - if (f._parameters[0].is_constant()) { - throw api_error::validation(format("{}: begins_with() encountered malformed AttributeValue: {}", caller, v1)); - } - } else if (v1.MemberBegin()->name != "S" && v1.MemberBegin()->name != "B") { - bad = true; - if (f._parameters[0].is_constant()) { - throw api_error::validation(format("{}: begins_with() supports only string or binary in AttributeValue: {}", caller, v1)); - } - } - if (!v2.IsObject() || v2.MemberCount() != 1) { - bad = true; - if (f._parameters[1].is_constant()) { - throw api_error::validation(format("{}: begins_with() encountered malformed AttributeValue: {}", caller, v2)); - } - } else if (v2.MemberBegin()->name != "S" && v2.MemberBegin()->name != "B") { - bad = true; - if (f._parameters[1].is_constant()) { - throw api_error::validation(format("{}: begins_with() supports only string or binary in AttributeValue: {}", caller, v2)); - } - } - bool ret = false; - if (!bad) { - auto it1 = v1.MemberBegin(); - auto it2 = v2.MemberBegin(); - if (it1->name == it2->name) { - if (it2->name == "S") { - std::string_view val1 = rjson::to_string_view(it1->value); - std::string_view val2 = rjson::to_string_view(it2->value); - ret = val1.starts_with(val2); - } else /* it2->name == "B" */ { - ret = base64_begins_with(rjson::to_string_view(it1->value), rjson::to_string_view(it2->value)); - } - } - } - return to_bool_json(ret); + return to_bool_json(check_BEGINS_WITH(v1.IsNull() ? nullptr : &v1, v2, + f._parameters[0].is_constant(), f._parameters[1].is_constant())); } }, {"contains", [] (calculate_value_caller caller, const rjson::value* previous_item, const parsed::value::function_call& f) { diff --git a/test/alternator/test_condition_expression.py b/test/alternator/test_condition_expression.py index 4dd60d962a..7631477bf0 100644 --- a/test/alternator/test_condition_expression.py +++ b/test/alternator/test_condition_expression.py @@ -136,7 +136,7 @@ def test_update_condition_eq_different(test_table_s): ConditionExpression='a = :val2', ExpressionAttributeValues={':val1': val1, ':val2': val2}) -# Also check an actual case of same time, but inequality. +# Also check an actual case of same type, but inequality. def test_update_condition_eq_unequal(test_table_s): p = random_string() test_table_s.update_item(Key={'p': p}, @@ -146,6 +146,13 @@ def test_update_condition_eq_unequal(test_table_s): UpdateExpression='SET a = :val1', ConditionExpression='a = :oldval', ExpressionAttributeValues={':val1': 3, ':oldval': 2}) + # If the attribute being compared doesn't exist, it's considered a failed + # condition, not an error: + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET a = :val1', + ConditionExpression='q = :oldval', + ExpressionAttributeValues={':val1': 3, ':oldval': 2}) # Check that set equality is checked correctly. Unlike string equality (for # example), it cannot be done with just naive string comparison of the JSON @@ -269,15 +276,44 @@ def test_update_condition_lt(test_table_s): UpdateExpression='SET z = :newval', ConditionExpression='a < :oldval', ExpressionAttributeValues={':newval': 2, ':oldval': '17'}) - # Trying to compare an unsupported type - e.g., in the following test - # a boolean, is unfortunately caught by boto3 and cannot be tested here... - #test_table_s.update_item(Key={'p': p}, - # AttributeUpdates={'d': {'Value': False, 'Action': 'PUT'}}) - #with pytest.raises(ClientError, match='ConditionalCheckFailedException'): - # test_table_s.update_item(Key={'p': p}, - # UpdateExpression='SET z = :newval', - # ConditionExpression='d < :oldval', - # ExpressionAttributeValues={':newval': 2, ':oldval': True}) + # If the attribute being compared doesn't even exist, this is also + # considered as a false condition - not an error. + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression='q < :oldval', + ExpressionAttributeValues={':newval': 2, ':oldval': '17'}) + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression=':oldval < q', + ExpressionAttributeValues={':newval': 2, ':oldval': '17'}) + # If a comparison parameter comes from a constant specified in the query, + # and it has a type not supported by the comparison (e.g., a list), it's + # not just a failed comparison - it is considered a ValidationException + with pytest.raises(ClientError, match='ValidationException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression='a < :oldval', + ExpressionAttributeValues={':newval': 2, ':oldval': [1,2]}) + with pytest.raises(ClientError, match='ValidationException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression=':oldval < a', + ExpressionAttributeValues={':newval': 2, ':oldval': [1,2]}) + # However, if when the wrong type comes from an item attribute, not the + # query, the comparison is simply false - not a ValidationException. + test_table_s.update_item(Key={'p': p}, AttributeUpdates={'x': {'Value': [1,2,3], 'Action': 'PUT'}}) + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression='x < :oldval', + ExpressionAttributeValues={':newval': 2, ':oldval': 1}) + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression=':oldval < x', + ExpressionAttributeValues={':newval': 2, ':oldval': 1}) assert test_table_s.get_item(Key={'p': p}, ConsistentRead=True)['Item']['z'] == 4 # Test for ConditionExpression with operator "<=" @@ -341,6 +377,44 @@ def test_update_condition_le(test_table_s): UpdateExpression='SET z = :newval', ConditionExpression='a <= :oldval', ExpressionAttributeValues={':newval': 2, ':oldval': '17'}) + # If the attribute being compared doesn't even exist, this is also + # considered as a false condition - not an error. + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression='q <= :oldval', + ExpressionAttributeValues={':newval': 2, ':oldval': '17'}) + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression=':oldval <= q', + ExpressionAttributeValues={':newval': 2, ':oldval': '17'}) + # If a comparison parameter comes from a constant specified in the query, + # and it has a type not supported by the comparison (e.g., a list), it's + # not just a failed comparison - it is considered a ValidationException + with pytest.raises(ClientError, match='ValidationException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression='a <= :oldval', + ExpressionAttributeValues={':newval': 2, ':oldval': [1,2]}) + with pytest.raises(ClientError, match='ValidationException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression=':oldval <= a', + ExpressionAttributeValues={':newval': 2, ':oldval': [1,2]}) + # However, if when the wrong type comes from an item attribute, not the + # query, the comparison is simply false - not a ValidationException. + test_table_s.update_item(Key={'p': p}, AttributeUpdates={'x': {'Value': [1,2,3], 'Action': 'PUT'}}) + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression='x <= :oldval', + ExpressionAttributeValues={':newval': 2, ':oldval': 1}) + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression=':oldval <= x', + ExpressionAttributeValues={':newval': 2, ':oldval': 1}) assert test_table_s.get_item(Key={'p': p}, ConsistentRead=True)['Item']['z'] == 7 # Test for ConditionExpression with operator ">" @@ -404,6 +478,44 @@ def test_update_condition_gt(test_table_s): UpdateExpression='SET z = :newval', ConditionExpression='a > :oldval', ExpressionAttributeValues={':newval': 2, ':oldval': '17'}) + # If the attribute being compared doesn't even exist, this is also + # considered as a false condition - not an error. + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression='q > :oldval', + ExpressionAttributeValues={':newval': 2, ':oldval': '17'}) + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression=':oldval > q', + ExpressionAttributeValues={':newval': 2, ':oldval': '17'}) + # If a comparison parameter comes from a constant specified in the query, + # and it has a type not supported by the comparison (e.g., a list), it's + # not just a failed comparison - it is considered a ValidationException + with pytest.raises(ClientError, match='ValidationException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression='a > :oldval', + ExpressionAttributeValues={':newval': 2, ':oldval': [1,2]}) + with pytest.raises(ClientError, match='ValidationException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression=':oldval > a', + ExpressionAttributeValues={':newval': 2, ':oldval': [1,2]}) + # However, if when the wrong type comes from an item attribute, not the + # query, the comparison is simply false - not a ValidationException. + test_table_s.update_item(Key={'p': p}, AttributeUpdates={'x': {'Value': [1,2,3], 'Action': 'PUT'}}) + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression='x > :oldval', + ExpressionAttributeValues={':newval': 2, ':oldval': 1}) + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression=':oldval > x', + ExpressionAttributeValues={':newval': 2, ':oldval': 1}) assert test_table_s.get_item(Key={'p': p}, ConsistentRead=True)['Item']['z'] == 4 # Test for ConditionExpression with operator ">=" @@ -467,6 +579,44 @@ def test_update_condition_ge(test_table_s): UpdateExpression='SET z = :newval', ConditionExpression='a >= :oldval', ExpressionAttributeValues={':newval': 2, ':oldval': '0'}) + # If the attribute being compared doesn't even exist, this is also + # considered as a false condition - not an error. + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression='q >= :oldval', + ExpressionAttributeValues={':newval': 2, ':oldval': '17'}) + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression=':oldval >= q', + ExpressionAttributeValues={':newval': 2, ':oldval': '17'}) + # If a comparison parameter comes from a constant specified in the query, + # and it has a type not supported by the comparison (e.g., a list), it's + # not just a failed comparison - it is considered a ValidationException + with pytest.raises(ClientError, match='ValidationException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression='a >= :oldval', + ExpressionAttributeValues={':newval': 2, ':oldval': [1,2]}) + with pytest.raises(ClientError, match='ValidationException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression=':oldval >= a', + ExpressionAttributeValues={':newval': 2, ':oldval': [1,2]}) + # However, if when the wrong type comes from an item attribute, not the + # query, the comparison is simply false - not a ValidationException. + test_table_s.update_item(Key={'p': p}, AttributeUpdates={'x': {'Value': [1,2,3], 'Action': 'PUT'}}) + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression='x >= :oldval', + ExpressionAttributeValues={':newval': 2, ':oldval': 1}) + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression=':oldval >= x', + ExpressionAttributeValues={':newval': 2, ':oldval': 1}) assert test_table_s.get_item(Key={'p': p}, ConsistentRead=True)['Item']['z'] == 7 # Test for ConditionExpression with ternary operator "BETWEEN" (checking @@ -548,6 +698,60 @@ def test_update_condition_between(test_table_s): UpdateExpression='SET z = :newval', ConditionExpression='a BETWEEN :oldval1 AND :oldval2', ExpressionAttributeValues={':newval': 2, ':oldval1': '0', ':oldval2': '2'}) + # If the attribute being compared doesn't even exist, this is also + # considered as a false condition - not an error. + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression='q BETWEEN :oldval1 AND :oldval2', + ExpressionAttributeValues={':newval': 2, ':oldval1': b'dog', ':oldval2': b'zebra'}) + # If and operand from the query, and it has a type not supported by the + # comparison (e.g., a list), it's not just a failed condition - it is + # considered a ValidationException + with pytest.raises(ClientError, match='ValidationException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression='a BETWEEN :oldval1 AND :oldval2', + ExpressionAttributeValues={':newval': 2, ':oldval1': [1,2], ':oldval2': [2,3]}) + # However, if when the wrong type comes from an item attribute, not the + # query, the comparison is simply false - not a ValidationException. + test_table_s.update_item(Key={'p': p}, AttributeUpdates={'x': {'Value': [1,2,3], 'Action': 'PUT'}, + 'y': {'Value': [2,3,4], 'Action': 'PUT'}}) + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression='a BETWEEN x and y', + ExpressionAttributeValues={':newval': 2}) + # If the two operands come from the query (":val" references) then if they + # have different types or the wrong order, this is a ValidationException. + # But if one or more of the operands come from the item, this only causes + # a false condition - not a ValidationException. + with pytest.raises(ClientError, match='ValidationException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression='a BETWEEN :oldval1 AND :oldval2', + ExpressionAttributeValues={':newval': 2, ':oldval1': 2, ':oldval2': 1}) + with pytest.raises(ClientError, match='ValidationException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression='a BETWEEN :oldval1 AND :oldval2', + ExpressionAttributeValues={':newval': 2, ':oldval1': 2, ':oldval2': 'dog'}) + test_table_s.update_item(Key={'p': p}, AttributeUpdates={'two': {'Value': 2, 'Action': 'PUT'}}) + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression='a BETWEEN two AND :oldval', + ExpressionAttributeValues={':newval': 2, ':oldval': 1}) + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression='a BETWEEN :oldval AND two', + ExpressionAttributeValues={':newval': 2, ':oldval': 3}) + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET z = :newval', + ConditionExpression='a BETWEEN two AND :oldval', + ExpressionAttributeValues={':newval': 2, ':oldval': 'dog'}) assert test_table_s.get_item(Key={'p': p}, ConsistentRead=True)['Item']['z'] == 9 # Test for ConditionExpression with multi-operand operator "IN", checking @@ -605,6 +809,13 @@ def test_update_condition_in(test_table_s): UpdateExpression='SET c = :val37', ConditionExpression='a IN ()', ExpressionAttributeValues=values) + # If the attribute being compared doesn't even exist, this is also + # considered as a false condition - not an error. + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + UpdateExpression='SET c = :val37', + ConditionExpression='q IN ({})'.format(','.join(values.keys())), + ExpressionAttributeValues=values) # Beyond the above operators, there are also test functions supported - # attribute_exists, attribute_not_exists, attribute_type, begins_with, diff --git a/test/alternator/test_expected.py b/test/alternator/test_expected.py index f8b52e21d9..69ba91855b 100644 --- a/test/alternator/test_expected.py +++ b/test/alternator/test_expected.py @@ -237,6 +237,30 @@ def test_update_expected_1_le(test_table_s): 'AttributeValueList': [2, 3]}} ) +# Comparison operators like le work only on numbers, strings or bytes. +# As noted in issue #8043, if any other type is included in *the query*, +# the result should be a ValidationException, but if the wrong type appears +# in the item, not the query, the result is a failed condition. +def test_update_expected_1_le_validation(test_table_s): + p = random_string() + test_table_s.update_item(Key={'p': p}, + AttributeUpdates={'a': {'Value': 1, 'Action': 'PUT'}, + 'b': {'Value': [1,2], 'Action': 'PUT'}}) + # Bad type (a list) in the query. Result is ValidationException. + with pytest.raises(ClientError, match='ValidationException'): + test_table_s.update_item(Key={'p': p}, + AttributeUpdates={'z': {'Value': 17, 'Action': 'PUT'}}, + Expected={'a': {'ComparisonOperator': 'LE', + 'AttributeValueList': [[1,2,3]]}} + ) + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + AttributeUpdates={'z': {'Value': 17, 'Action': 'PUT'}}, + Expected={'b': {'ComparisonOperator': 'LE', + 'AttributeValueList': [3]}} + ) + assert not 'z' in test_table_s.get_item(Key={'p': p}, ConsistentRead=True)['Item'] + # Tests for Expected with ComparisonOperator = "LT": def test_update_expected_1_lt(test_table_s): p = random_string() @@ -894,6 +918,34 @@ def test_update_expected_1_between(test_table_s): AttributeUpdates={'z': {'Value': 2, 'Action': 'PUT'}}, Expected={'d': {'ComparisonOperator': 'BETWEEN', 'AttributeValueList': [set([1]), set([2])]}}) +# BETWEEN work only on numbers, strings or bytes. As noted in issue #8043, +# if any other type is included in *the query*, the result should be a +# ValidationException, but if the wrong type appears in the item, not the +# query, the result is a failed condition. +# BETWEEN should also generate ValidationException if the two ends of the +# range are not of the same type or not in the correct order, but this +# already is tested in the test above (test_update_expected_1_between). +def test_update_expected_1_between_validation(test_table_s): + p = random_string() + test_table_s.update_item(Key={'p': p}, + AttributeUpdates={'a': {'Value': 1, 'Action': 'PUT'}, + 'b': {'Value': [1,2], 'Action': 'PUT'}}) + # Bad type (a list) in the query. Result is ValidationException. + with pytest.raises(ClientError, match='ValidationException'): + test_table_s.update_item(Key={'p': p}, + AttributeUpdates={'z': {'Value': 17, 'Action': 'PUT'}}, + Expected={'a': {'ComparisonOperator': 'BETWEEN', + 'AttributeValueList': [[1,2,3], [2,3,4]]}} + ) + with pytest.raises(ClientError, match='ConditionalCheckFailedException'): + test_table_s.update_item(Key={'p': p}, + AttributeUpdates={'z': {'Value': 17, 'Action': 'PUT'}}, + Expected={'b': {'ComparisonOperator': 'BETWEEN', + 'AttributeValueList': [1,2]}} + ) + assert not 'z' in test_table_s.get_item(Key={'p': p}, ConsistentRead=True)['Item'] + + ############################################################################## # Instead of ComparisonOperator and AttributeValueList, one can specify either # Value or Exists: diff --git a/test/alternator/test_filter_expression.py b/test/alternator/test_filter_expression.py index 42718d5d9b..f88a2ba65a 100644 --- a/test/alternator/test_filter_expression.py +++ b/test/alternator/test_filter_expression.py @@ -235,6 +235,30 @@ def test_filter_expression_ge(test_table_sn_with_data): expected_items = [item for item in items if item[xn] >= xv] assert(got_items == expected_items) +# Comparison operators such as >= or BETWEEN only work on numbers, strings or +# bytes. When an expression's operands come from the item and has a wrong type +# (e.g., a list), the result is that the item is skipped - aborting the scan +# with a ValidationException is a bug (this was issue #8043). +def test_filter_expression_le_bad_type(test_table_sn_with_data): + table, p, items = test_table_sn_with_data + got_items = full_query(table, KeyConditionExpression='p=:p', FilterExpression='l <= :xv', + ExpressionAttributeValues={':p': p, ':xv': 3}) + assert got_items == [] + got_items = full_query(table, KeyConditionExpression='p=:p', FilterExpression=':xv <= l', + ExpressionAttributeValues={':p': p, ':xv': 3}) + assert got_items == [] +def test_filter_expression_between_bad_type(test_table_sn_with_data): + table, p, items = test_table_sn_with_data + got_items = full_query(table, KeyConditionExpression='p=:p', FilterExpression='s between :xv and l', + ExpressionAttributeValues={':p': p, ':xv': 'cat'}) + assert got_items == [] + got_items = full_query(table, KeyConditionExpression='p=:p', FilterExpression='s between l and :xv', + ExpressionAttributeValues={':p': p, ':xv': 'cat'}) + assert got_items == [] + got_items = full_query(table, KeyConditionExpression='p=:p', FilterExpression='s between i and :xv', + ExpressionAttributeValues={':p': p, ':xv': 'cat'}) + assert got_items == [] + # Test the "BETWEEN/AND" ternary operator on a numeric, string and bytes # attribute. These keywords are case-insensitive. def test_filter_expression_between(test_table_sn_with_data):