fix: data plane request validation

crates/core/src/serde_helpers.rs

@@ -75,11 +75,36 @@ where
         return Err(de::Error::custom(format!("{field_name} must not be empty")));
     }
     let mut out = HashMap::with_capacity(raw.len());
-    for (key, value) in raw {
-        validate_placeholder_key(&key, prefix, field_name, include_size_in_too_long)
+    // Pass 1: validate every key first, so a malformed key is reported before
+    // any value error (DynamoDB validates key syntax ahead of value contents).
+    for key in raw.keys() {
+        validate_placeholder_key(key, prefix, field_name, include_size_in_too_long)
             .map_err(de::Error::custom)?;
+    }
+    // Pass 2: per-value check and conversion.
+    for (key, value) in raw {
         check_value(&key, &value).map_err(de::Error::custom)?;
-        let converted = convert(value).map_err(de::Error::custom)?;
+        let converted = convert(value).map_err(|e| {
+            let msg = e.to_string();
+            // Semantic value-validation errors are wrapped with the field name
+            // and the offending key (DynamoDB parity). Wire/type errors (wrong
+            // JSON shape for a datatype) pass through as-is.
+            let is_value_validation = msg.starts_with("One or more parameter values were invalid:")
+                || msg.contains("Supplied AttributeValue is empty")
+                || msg.contains("Supplied AttributeValue has more than one datatypes set")
+                || msg.contains("cannot be converted to a numeric value")
+                || msg.contains("significant digits in a Number")
+                || msg.starts_with("Number overflow")
+                || msg.starts_with("Number underflow")
+                || msg.contains("Input collection contains duplicates");
+            if is_value_validation {
+                de::Error::custom(format!(
+                    "{field_name} contains invalid value: {msg} for key {key}"
+                ))
+            } else {
+                de::Error::custom(msg)
+            }
+        })?;
         out.insert(key, converted);
     }
     Ok(Some(out))
@@ -264,6 +289,162 @@ mod tests {
         );
     }
 
+    #[test]
+    fn values_null_non_boolean_is_validation_error() {
+        // {"NULL":"no"} is a validation error on real DynamoDB, not a parse
+        // (Serialization) error. Must be prefixed and name its key.
+        let msg = values_err(r#"{"values":{":b":{"NULL":"no"}}}"#);
+        assert!(
+            msg.contains(
+                "ExpressionAttributeValues contains invalid value: One or more parameter \
+                 values were invalid: Null attribute value types must have the value of \
+                 true for key :b"
+            ),
+            "{msg}"
+        );
+    }
+
+    #[test]
+    fn values_null_false_is_validation_error() {
+        let msg = values_err(r#"{"values":{":b":{"NULL":false}}}"#);
+        assert!(
+            msg.contains(
+                "ExpressionAttributeValues contains invalid value: One or more parameter \
+                 values were invalid: Null attribute value types must have the value of \
+                 true for key :b"
+            ),
+            "{msg}"
+        );
+    }
+
+    #[test]
+    fn values_null_true_accepted() {
+        let parsed: TestValues =
+            serde_json::from_str(r#"{"values":{":b":{"NULL":true}}}"#).unwrap();
+        assert!(parsed.values.is_some());
+    }
+
+    #[test]
+    fn values_empty_set_wrapped_with_key() {
+        for (av, needle) in [
+            (r#"{"SS":[]}"#, "An string set  may not be empty"),
+            (r#"{"NS":[]}"#, "An number set  may not be empty"),
+            (r#"{"BS":[]}"#, "Binary sets should not be empty"),
+        ] {
+            let msg = values_err(&format!(r#"{{"values":{{":b":{av}}}}}"#));
+            let expected = format!(
+                "ExpressionAttributeValues contains invalid value: One or more \
+                 parameter values were invalid: {needle} for key :b"
+            );
+            assert!(msg.contains(&expected), "av={av} got: {msg}");
+        }
+    }
+
+    #[test]
+    fn values_duplicate_set_wrapped_with_key() {
+        let ss = values_err(r#"{"values":{":b":{"SS":["a","a"]}}}"#);
+        assert!(
+            ss.contains(
+                "ExpressionAttributeValues contains invalid value: One or more parameter \
+                 values were invalid: Input collection [a, a] contains duplicates. for key :b"
+            ),
+            "{ss}"
+        );
+        // Binary duplicates carry the "of type BS" qualifier (DynamoDB parity).
+        let bs = values_err(r#"{"values":{":b":{"BS":["Yg==","Yg=="]}}}"#);
+        assert!(
+            bs.contains(
+                "ExpressionAttributeValues contains invalid value: One or more parameter \
+                 values were invalid: Input collection [Yg==, Yg==]of type BS contains \
+                 duplicates. for key :b"
+            ),
+            "{bs}"
+        );
+    }
+
+    #[test]
+    fn values_invalid_key_reported_before_value_error() {
+        // Map has a malformed value (:b -> unknown type) AND a malformed key (b
+        // without the ':' prefix). The key error must win, matching DynamoDB.
+        let msg = values_err(r#"{"values":{":b":{"a":""},"b":{"S":"a"}}}"#);
+        assert!(
+            msg.contains(
+                r#"ExpressionAttributeValues contains invalid key: Syntax error; key: "b""#
+            ),
+            "{msg}"
+        );
+    }
+
+    #[test]
+    fn values_unsupported_datatype_wrapped_with_key() {
+        // An unrecognized datatype tag is reported as an empty AttributeValue,
+        // wrapped with the field name and key.
+        let msg = values_err(r#"{"values":{":b":{"a":""}}}"#);
+        assert!(
+            msg.contains(
+                "ExpressionAttributeValues contains invalid value: Supplied AttributeValue \
+                 is empty, must contain exactly one of the supported datatypes for key :b"
+            ),
+            "{msg}"
+        );
+    }
+
+    #[test]
+    fn values_multiple_datatypes_wrapped_with_key() {
+        let msg = values_err(r#"{"values":{":b":{"S":"a","N":"1"}}}"#);
+        assert!(
+            msg.contains(
+                "ExpressionAttributeValues contains invalid value: Supplied AttributeValue \
+                 has more than one datatypes set, must contain exactly one of the supported \
+                 datatypes for key :b"
+            ),
+            "{msg}"
+        );
+    }
+
+    #[test]
+    fn values_invalid_number_wrapped_with_key() {
+        // Empty, non-numeric, overflow, underflow, 38-digit, and NS-duplicate
+        // all wrap with the field name and key, matching real DynamoDB.
+        for (av, needle) in [
+            (
+                r#"{"N":""}"#,
+                "The parameter cannot be converted to a numeric value",
+            ),
+            (
+                r#"{"N":"b"}"#,
+                "The parameter cannot be converted to a numeric value: b",
+            ),
+            (
+                r#"{"S":"a","N":""}"#,
+                "The parameter cannot be converted to a numeric value",
+            ),
+            (
+                r#"{"NS":["1","b"]}"#,
+                "The parameter cannot be converted to a numeric value: b",
+            ),
+            (
+                r#"{"NS":["1","1"]}"#,
+                "Input collection contains duplicates",
+            ),
+            (
+                r#"{"N":"1e126"}"#,
+                "Number overflow. Attempting to store a number with magnitude larger",
+            ),
+            (
+                r#"{"N":"1e-131"}"#,
+                "Number underflow. Attempting to store a number with magnitude smaller",
+            ),
+        ] {
+            let msg = values_err(&format!(r#"{{"values":{{":b":{av}}}}}"#));
+            let expected = format!("ExpressionAttributeValues contains invalid value: {needle}");
+            assert!(
+                msg.contains(&expected) && msg.contains("for key :b"),
+                "av={av} got: {msg}"
+            );
+        }
+    }
+
     #[test]
     fn values_key_too_long_omits_size() {
         let key = format!(":{}", "a".repeat(255)); // 256 bytes including ':'

crates/core/src/types/attribute_value.rs

@@ -76,21 +76,50 @@ impl<'de> Visitor<'de> for AttributeValueVisitor {
     }
 
     fn visit_map<A: MapAccess<'de>>(self, mut map: A) -> Result<Self::Value, A::Error> {
-        let (key, value): (String, serde_json::Value) = map
-            .next_entry()?
-            .ok_or_else(|| {
-                de::Error::custom(
-                    "Supplied AttributeValue is empty, must contain exactly one of the supported datatypes",
-                )
-            })?;
+        let mut entries: Vec<(String, serde_json::Value)> = Vec::new();
+        while let Some(entry) = map.next_entry::<String, serde_json::Value>()? {
+            entries.push(entry);
+        }
+        if entries.is_empty() {
+            return Err(de::Error::custom(
+                "Supplied AttributeValue is empty, must contain exactly one of the supported datatypes",
+            ));
+        }
+
+        // DynamoDB validates the content of a number field before it reports the
+        // "more than one datatype" error, so an invalid N/NS value is surfaced
+        // even when several type descriptors are present.
+        for (k, v) in &entries {
+            match k.as_str() {
+                "N" => {
+                    if let Some(s) = v.as_str() {
+                        crate::validation::number::validate_and_normalize_number(s)
+                            .map_err(|e| de::Error::custom(e.message()))?;
+                    }
+                }
+                "NS" => {
+                    if let Some(arr) = v.as_array() {
+                        for elem in arr {
+                            if let Some(s) = elem.as_str() {
+                                crate::validation::number::validate_and_normalize_number(s)
+                                    .map_err(|e| de::Error::custom(e.message()))?;
+                            }
+                        }
+                    }
+                }
+                _ => {}
+            }
+        }
 
         // REQ-TYPE-001: reject if multiple keys
-        if map.next_key::<String>()?.is_some() {
+        if entries.len() > 1 {
             return Err(de::Error::custom(
                 "Supplied AttributeValue has more than one datatypes set, must contain exactly one of the supported datatypes",
             ));
         }
 
+        let (key, value) = entries.into_iter().next().expect("entries is non-empty");
+
         match key.as_str() {
             "S" => {
                 let s = value
@@ -174,11 +203,7 @@ impl<'de> Visitor<'de> for AttributeValueVisitor {
                     })
                     .collect::<Result<_, _>>()?;
                 if set.len() != arr.len() {
-                    let values: Vec<&str> = arr.iter().filter_map(|v| v.as_str()).collect();
-                    let repr = values.join(", ");
-                    return Err(de::Error::custom(format!(
-                        "One or more parameter values were invalid: Input collection [{repr}] contains duplicates."
-                    )));
+                    return Err(de::Error::custom("Input collection contains duplicates"));
                 }
                 Ok(AttributeValue::NS(set))
             }
@@ -210,7 +235,7 @@ impl<'de> Visitor<'de> for AttributeValueVisitor {
                     let values: Vec<&str> = arr.iter().filter_map(|v| v.as_str()).collect();
                     let repr = values.join(", ");
                     return Err(de::Error::custom(format!(
-                        "One or more parameter values were invalid: Input collection [{repr}] contains duplicates."
+                        "One or more parameter values were invalid: Input collection [{repr}]of type BS contains duplicates."
                     )));
                 }
                 Ok(AttributeValue::BS(set))
@@ -222,10 +247,7 @@ impl<'de> Visitor<'de> for AttributeValueVisitor {
                 Ok(AttributeValue::Bool(b))
             }
             "NULL" => {
-                let n = value
-                    .as_bool()
-                    .ok_or_else(|| de::Error::custom("NULL value must be a boolean"))?;
-                if !n {
+                if value.as_bool() != Some(true) {
                     return Err(de::Error::custom(
                         "One or more parameter values were invalid: Null attribute value types must have the value of true",
                     ));
@@ -256,9 +278,9 @@ impl<'de> Visitor<'de> for AttributeValueVisitor {
                     .collect::<Result<_, A::Error>>()?;
                 Ok(AttributeValue::M(map))
             }
-            other => Err(de::Error::custom(format!(
-                "unknown AttributeValue type descriptor: {other}"
-            ))),
+            _other => Err(de::Error::custom(
+                "Supplied AttributeValue is empty, must contain exactly one of the supported datatypes",
+            )),
         }
     }
 }
@@ -421,32 +443,41 @@ mod tests {
 
     #[test]
     fn unknown_type_descriptor_rejected() {
+        // An unrecognized type descriptor means no supported datatype is
+        // present; DynamoDB reports this as an empty AttributeValue.
         let json = r#"{"X":"hello"}"#;
         let err = serde_json::from_str::<AttributeValue>(json).unwrap_err();
-        assert!(err.to_string().contains("unknown"));
+        assert!(
+            err.to_string().contains(
+                "Supplied AttributeValue is empty, must contain exactly one of the supported datatypes"
+            ),
+            "got: {err}"
+        );
     }
 
     #[test]
     fn invalid_number_accepted_at_deserialization() {
-        // Invalid numbers are accepted by the deserializer (stored raw)
-        // and rejected later by the validation layer as ValidationException.
-        let json = r#"{"N":"abc"}"#;
-        let val: AttributeValue = serde_json::from_str(json).unwrap();
-        assert_eq!(val, AttributeValue::N("abc".to_owned()));
+        // Invalid numbers are rejected at deserialization with DynamoDB's
+        // number-validation messages.
+        let err = serde_json::from_str::<AttributeValue>(r#"{"N":"abc"}"#).unwrap_err();
+        assert!(
+            err.to_string()
+                .contains("The parameter cannot be converted to a numeric value: abc"),
+            "got: {err}"
+        );
 
-        let json = r#"{"N":"1E999"}"#;
-        let val: AttributeValue = serde_json::from_str(json).unwrap();
-        assert_eq!(val, AttributeValue::N("1E999".to_owned()));
+        let err = serde_json::from_str::<AttributeValue>(r#"{"N":"1E999"}"#).unwrap_err();
+        assert!(err.to_string().contains("Number overflow"), "got: {err}");
     }
 
     #[test]
     fn invalid_number_in_ns_accepted() {
-        let json = r#"{"NS":["1","abc"]}"#;
-        let val: AttributeValue = serde_json::from_str(json).unwrap();
-        match val {
-            AttributeValue::NS(set) => assert!(set.contains("abc")),
-            _ => panic!("expected NS"),
-        }
+        let err = serde_json::from_str::<AttributeValue>(r#"{"NS":["1","abc"]}"#).unwrap_err();
+        assert!(
+            err.to_string()
+                .contains("The parameter cannot be converted to a numeric value: abc"),
+            "got: {err}"
+        );
     }
 
     #[test]