From 8ebfb40f07ba7ecc9857cb25a096b2b7079157da Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Thu, 16 Jul 2026 18:57:34 -0300 Subject: [PATCH 1/3] fix(s3,dynamodb,athena,route53,cloudwatch,ses,firehose): correct output/filter/pagination divergences (bug-hunt) --- crates/fakecloud-athena/src/service/mod.rs | 93 +++++++++++++ .../fakecloud-athena/src/service/queries.rs | 57 +++++++- crates/fakecloud-cloudwatch/src/service.rs | 54 +++++++- crates/fakecloud-cloudwatch/src/tests.rs | 50 +++++++ .../fakecloud-dynamodb/src/service/queries.rs | 15 +++ .../fakecloud-dynamodb/src/service/tests.rs | 66 ++++++++++ crates/fakecloud-firehose/src/service.rs | 79 ++++++++++- crates/fakecloud-route53/src/service/mod.rs | 76 +++++++++++ .../fakecloud-route53/src/service/records.rs | 50 +++---- .../fakecloud-s3/src/service/objects/read.rs | 8 +- crates/fakecloud-s3/src/service/tests.rs | 61 +++++++++ .../src/service/contact_lists.rs | 76 +++++++++++ crates/fakecloud-ses/src/service/misc.rs | 5 +- crates/fakecloud-ses/src/service/sending.rs | 12 +- crates/fakecloud-ses/src/service/tests.rs | 124 +++++++++++++++++- 15 files changed, 770 insertions(+), 56 deletions(-) diff --git a/crates/fakecloud-athena/src/service/mod.rs b/crates/fakecloud-athena/src/service/mod.rs index 52a873371..62ea019fd 100644 --- a/crates/fakecloud-athena/src/service/mod.rs +++ b/crates/fakecloud-athena/src/service/mod.rs @@ -1227,6 +1227,99 @@ mod tests { serde_json::from_slice(resp.body.expect_bytes()).unwrap() } + #[test] + fn get_query_results_paginates_large_result_set() { + // Regression: GetQueryResults honors MaxResults + NextToken. Before + // the fix it returned only the first MaxResults rows and emitted no + // NextToken, leaving the rest of a large result set unreachable. + let svc = AthenaService::new(SharedAthenaState::default()); + let id = "qe-paginate"; + { + let mut state = svc.state.write(); + let account = super::account_mut(&mut state, "123456789012"); + let now = Utc::now(); + account.query_executions.insert( + id.to_string(), + crate::state::QueryExecution { + query_execution_id: id.to_string(), + query: "SELECT n FROM t".to_string(), + statement_type: "DML".to_string(), + work_group: "primary".to_string(), + state: "SUCCEEDED".to_string(), + state_change_reason: None, + submission_time: now, + completion_time: Some(now), + query_execution_context: None, + result_configuration: None, + engine_version: None, + data_scanned_bytes: 0, + engine_execution_time_ms: 1, + query_planning_time_ms: 1, + total_execution_time_ms: 2, + result_rows: (0..5).map(|n| vec![n.to_string()]).collect(), + result_columns: vec![("n".to_string(), "integer".to_string())], + }, + ); + } + + let mut data_rows: Vec = Vec::new(); + let mut next: Option = None; + let mut pages = 0; + loop { + pages += 1; + let mut body = json!({ "QueryExecutionId": id, "MaxResults": 3 }); + if let Some(t) = &next { + body["NextToken"] = json!(t); + } + let resp = svc + .get_query_results(&req("GetQueryResults", body)) + .unwrap(); + let parsed = parse_json(&resp); + let rows = parsed["ResultSet"]["Rows"].as_array().unwrap(); + // The column-header row is emitted only on the first page. + let start = if pages == 1 { 1 } else { 0 }; + for row in &rows[start..] { + data_rows.push(row["Data"][0]["VarCharValue"].as_str().unwrap().to_string()); + } + match parsed.get("NextToken").and_then(|v| v.as_str()) { + Some(t) => next = Some(t.to_string()), + None => break, + } + assert!(pages < 10, "pagination did not terminate"); + } + + assert!(pages >= 2, "large result set must span multiple pages"); + assert_eq!(data_rows, vec!["0", "1", "2", "3", "4"]); + + // MaxResults=1 (header consumes the only slot on page 1) must still + // terminate and surface every data row rather than loop forever. + let mut data_rows: Vec = Vec::new(); + let mut next: Option = None; + let mut pages = 0; + loop { + pages += 1; + let mut body = json!({ "QueryExecutionId": id, "MaxResults": 1 }); + if let Some(t) = &next { + body["NextToken"] = json!(t); + } + let resp = svc + .get_query_results(&req("GetQueryResults", body)) + .unwrap(); + let parsed = parse_json(&resp); + let rows = parsed["ResultSet"]["Rows"].as_array().unwrap(); + let start = if pages == 1 { 1 } else { 0 }; + for row in &rows[start..] { + data_rows.push(row["Data"][0]["VarCharValue"].as_str().unwrap().to_string()); + } + match parsed.get("NextToken").and_then(|v| v.as_str()) { + Some(t) => next = Some(t.to_string()), + None => break, + } + assert!(pages < 100, "MaxResults=1 pagination did not terminate"); + } + assert_eq!(data_rows, vec!["0", "1", "2", "3", "4"]); + } + #[test] fn substitute_parameters_replaces_placeholders() { let out = substitute_parameters( diff --git a/crates/fakecloud-athena/src/service/queries.rs b/crates/fakecloud-athena/src/service/queries.rs index 57fa7cc48..f358a5fe6 100644 --- a/crates/fakecloud-athena/src/service/queries.rs +++ b/crates/fakecloud-athena/src/service/queries.rs @@ -266,7 +266,14 @@ impl AthenaService { ) -> Result { let body = req.json_body(); let id = require_str(&body, "QueryExecutionId")?; + // Smithy: MaxResults targets MaxRowsCount @range(1,1000); + // NextToken targets Token @length(1,1024). let max_results = validate_max_results(&body, 1, 1000)?; + validate_opt_string_len(&body, "NextToken", 1, 1024)?; + let next_token = body + .get("NextToken") + .and_then(Value::as_str) + .map(str::to_owned); let mut state = self.state.write(); let account = account_mut(&mut state, &req.account_id); let q = account @@ -297,22 +304,58 @@ impl AthenaService { }) }) .collect(); - let header_row = json!({ - "Data": q.result_columns.iter().map(|(n, _)| json!({"VarCharValue": n})).collect::>(), - }); - let mut rows = vec![header_row]; - for row in q.result_rows.iter().take(max_results.saturating_sub(1)) { + // Pagination: the header row (column names) is emitted only on the + // first page (NextToken absent) and counts against MaxResults there. + // The NextToken is a numeric offset into the data rows; subsequent + // pages carry data rows only. Without this, result sets larger than + // MaxResults were unreachable past page 1 (bug-hunt 2026-07-16). + let offset: usize = match next_token.as_deref() { + None => 0, + Some(tok) => tok + .parse() + .map_err(|_| invalid_request("Invalid NextToken"))?, + }; + let total = q.result_rows.len(); + let start = offset.min(total); + // The column-header row is emitted only on the first invocation (no + // NextToken supplied). Keying this on token absence rather than + // `offset == 0` avoids an infinite loop when MaxResults is 1: the + // first page returns only the header with NextToken "0", and the + // resumed page must then advance past offset 0 rather than treating + // itself as the first page again. + let include_header = next_token.is_none(); + let data_budget = if include_header { + max_results.saturating_sub(1) + } else { + max_results + }; + let end = start.saturating_add(data_budget).min(total); + + let mut rows = Vec::new(); + if include_header { + rows.push(json!({ + "Data": q.result_columns.iter().map(|(n, _)| json!({"VarCharValue": n})).collect::>(), + })); + } + for row in &q.result_rows[start..end] { rows.push(json!({ "Data": row.iter().map(|v| json!({"VarCharValue": v})).collect::>(), })); } - Ok(AwsResponse::ok_json(json!({ + let mut response = json!({ "ResultSet": { "Rows": rows, "ResultSetMetadata": {"ColumnInfo": column_info}, }, "UpdateCount": 0, - }))) + }); + if end < total { + response + .as_object_mut() + .unwrap() + .insert("NextToken".to_string(), Value::String(end.to_string())); + } + Ok(AwsResponse::ok_json(response)) } pub(super) fn get_query_runtime_statistics( diff --git a/crates/fakecloud-cloudwatch/src/service.rs b/crates/fakecloud-cloudwatch/src/service.rs index 7a6ca2776..aa17183cb 100644 --- a/crates/fakecloud-cloudwatch/src/service.rs +++ b/crates/fakecloud-cloudwatch/src/service.rs @@ -532,6 +532,45 @@ pub(crate) fn parse_dimensions_query(req: &AwsRequest, prefix: &str) -> BTreeMap out } +/// Parse `{prefix}.member.N.Name` / `.Value` query params into ListMetrics' +/// `DimensionFilter` list, where `Value` is OPTIONAL (per the Smithy model). +/// A name-only filter matches any metric carrying a dimension with that name +/// (any value); a name+value filter is an exact match. +/// +/// Distinct from [`parse_dimensions_query`], which drops a name-only entry — +/// correct for the put/statistics APIs (exact dimension sets) but wrong for +/// ListMetrics, where a name-only filter must still narrow the results +/// instead of silently returning every metric in the namespace. +pub(crate) fn parse_dimension_filters( + req: &AwsRequest, + prefix: &str, +) -> Vec<(String, Option)> { + let mut dims: BTreeMap, Option)> = BTreeMap::new(); + let needle = format!("{prefix}.member."); + for (k, v) in req.query_params.iter() { + let Some(rest) = k.strip_prefix(&needle) else { + continue; + }; + let mut parts = rest.splitn(2, '.'); + let Some(idx_str) = parts.next() else { + continue; + }; + let Ok(idx) = idx_str.parse::() else { + continue; + }; + let field = parts.next().unwrap_or(""); + let entry = dims.entry(idx).or_default(); + match field { + "Name" => entry.0 = Some(v.clone()), + "Value" => entry.1 = Some(v.clone()), + _ => {} + } + } + dims.into_values() + .filter_map(|(name, value)| name.map(|n| (n, value))) + .collect() +} + /// Validate the length of an optional string param against `[min, max]`. /// Returns a 4xx on violation. AWS measures length in characters; the /// conformance probe only sends ASCII so byte length is equivalent here. @@ -954,7 +993,7 @@ impl CloudWatchService { validate_enum(req, "RecentlyActive", &["PT3H"])?; let namespace = optional_query_param(req, "Namespace"); let metric_name = optional_query_param(req, "MetricName"); - let dim_filter = parse_dimensions_query(req, "Dimensions"); + let dim_filter = parse_dimension_filters(req, "Dimensions"); // ListMetrics has no MaxResults param — AWS caps each page at 500 and // round-trips a NextToken. const LIST_METRICS_PAGE: usize = 500; @@ -981,13 +1020,14 @@ impl CloudWatchService { } } // ListMetrics filters by dimension containment (a metric - // matches if it carries all the requested name/value - // pairs), unlike the exact-set match used by the - // statistics APIs. + // matches if it carries all the requested filters), + // unlike the exact-set match used by the statistics + // APIs. A name-only DimensionFilter matches any value. if !dim_filter.is_empty() - && !dim_filter - .iter() - .all(|(k, v)| d.dimensions.get(k) == Some(v)) + && !dim_filter.iter().all(|(k, v)| match v { + Some(val) => d.dimensions.get(k) == Some(val), + None => d.dimensions.contains_key(k), + }) { continue; } diff --git a/crates/fakecloud-cloudwatch/src/tests.rs b/crates/fakecloud-cloudwatch/src/tests.rs index 9e5f2806a..0945c66a3 100644 --- a/crates/fakecloud-cloudwatch/src/tests.rs +++ b/crates/fakecloud-cloudwatch/src/tests.rs @@ -825,6 +825,56 @@ async fn put_metric_data_json_and_query_roundtrip() { assert!(body_of(&xml_list).contains("Latency")); } +#[tokio::test] +async fn list_metrics_name_only_dimension_filter_narrows_results() { + // Regression: a DimensionFilter with only a Name (no Value) must still + // narrow the results. Before the fix such a filter was dropped and every + // metric in the namespace was returned. + let svc = service(); + call_json( + &svc, + "PutMetricData", + serde_json::json!({ + "Namespace": "NS", + "MetricData": [{ + "MetricName": "M1", + "Value": 1.0, + "Dimensions": [{"Name": "Host", "Value": "a"}] + }] + }), + ) + .await; + call_json( + &svc, + "PutMetricData", + serde_json::json!({ + "Namespace": "NS", + "MetricData": [{ + "MetricName": "M2", + "Value": 2.0, + "Dimensions": [{"Name": "Region", "Value": "us"}] + }] + }), + ) + .await; + + let resp = call( + &svc, + "ListMetrics", + &[("Namespace", "NS"), ("Dimensions.member.1.Name", "Host")], + ) + .await; + let body = body_of(&resp); + assert!( + body.contains("M1"), + "M1 (has dimension Host) expected: {body}" + ); + assert!( + !body.contains("M2"), + "M2 (no Host dimension) must be filtered out: {body}" + ); +} + #[tokio::test] async fn get_metric_statistics_json_roundtrip() { let svc = service(); diff --git a/crates/fakecloud-dynamodb/src/service/queries.rs b/crates/fakecloud-dynamodb/src/service/queries.rs index cc143b59f..0c03f7572 100644 --- a/crates/fakecloud-dynamodb/src/service/queries.rs +++ b/crates/fakecloud-dynamodb/src/service/queries.rs @@ -134,6 +134,21 @@ impl DynamoDbService { let mut matched: Vec<&HashMap> = items_to_scan .iter() .filter(|item| { + // Sparse index: an item only appears in a GSI/LSI when it + // carries every one of that index's key attributes. AWS never + // returns (or counts) an item missing the index hash/range key + // on an index query, so skip those before evaluating the key + // condition (mirrors the scan() guard, bug-hunt 2026-07-16). + if index_name.is_some() { + if !item.contains_key(hash_key_name.as_str()) { + return false; + } + if let Some(ref rk) = range_key_name { + if !item.contains_key(rk.as_str()) { + return false; + } + } + } evaluate_key_condition(&key_condition, item, &expr_attr_names, &expr_attr_values) }) .collect(); diff --git a/crates/fakecloud-dynamodb/src/service/tests.rs b/crates/fakecloud-dynamodb/src/service/tests.rs index e7e97afd8..7223ff0da 100644 --- a/crates/fakecloud-dynamodb/src/service/tests.rs +++ b/crates/fakecloud-dynamodb/src/service/tests.rs @@ -1983,6 +1983,72 @@ fn gsi_query_last_evaluated_key_includes_table_pk() { ); } +#[test] +fn gsi_query_excludes_items_missing_index_sort_key() { + // Regression: an item lacking the index's sort key is not part of a + // sparse GSI and must not appear in (or be counted by) an index Query. + // Before the fix, query() filtered only by the key condition (which + // constrains the hash key) and returned the phantom item. + let svc = make_service(); + create_gsi_table(&svc); + + // Two full items carry both GSI key attributes. + for i in 0..2 { + let req = make_request( + "PutItem", + json!({ + "TableName": "gsi-table", + "Item": { + "pk": { "S": format!("full{i}") }, + "gsi_pk": { "S": "shared" }, + "gsi_sk": { "S": format!("sort{i}") } + } + }), + ); + svc.put_item(&req).unwrap(); + } + // One item carries the GSI hash key but NOT the GSI sort key, so it is + // absent from the sparse index. + let req = make_request( + "PutItem", + json!({ + "TableName": "gsi-table", + "Item": { + "pk": { "S": "phantom" }, + "gsi_pk": { "S": "shared" } + } + }), + ); + svc.put_item(&req).unwrap(); + + let req = make_request( + "Query", + json!({ + "TableName": "gsi-table", + "IndexName": "gsi-index", + "KeyConditionExpression": "gsi_pk = :v", + "ExpressionAttributeValues": { ":v": { "S": "shared" } } + }), + ); + let resp = svc.query(&req).unwrap(); + let body: Value = serde_json::from_slice(resp.body.expect_bytes()).unwrap(); + assert_eq!( + body["Count"], 2, + "phantom item missing gsi_sk must be excluded" + ); + assert_eq!(body["ScannedCount"], 2); + let pks: Vec<&str> = body["Items"] + .as_array() + .unwrap() + .iter() + .map(|it| it["pk"]["S"].as_str().unwrap()) + .collect(); + assert!( + !pks.contains(&"phantom"), + "phantom must not be returned: {pks:?}" + ); +} + #[test] fn gsi_query_pagination_returns_all_items() { let svc = make_service(); diff --git a/crates/fakecloud-firehose/src/service.rs b/crates/fakecloud-firehose/src/service.rs index 2b37e423d..b9bca2962 100644 --- a/crates/fakecloud-firehose/src/service.rs +++ b/crates/fakecloud-firehose/src/service.rs @@ -755,6 +755,23 @@ impl FirehoseService { let name = body["DeliveryStreamName"] .as_str() .ok_or_else(|| missing("DeliveryStreamName"))?; + // AWS: Limit @range(1,50) caps the page (default 50); + // ExclusiveStartTagKey resumes after that key. Tags are ordered by + // key (BTreeMap iteration). Without this, large tag sets were + // returned whole with HasMoreTags hardcoded to false (bug-hunt + // 2026-07-16). + let limit = match body["Limit"].as_i64() { + Some(l) => { + if !(1..=50).contains(&l) { + return Err(invalid_argument(format!( + "Limit must be between 1 and 50, got {l}" + ))); + } + l as usize + } + None => 50, + }; + let exclusive_start = body["ExclusiveStartTagKey"].as_str(); let accounts = self.state.read(); let state = accounts .get(&req.account_id) @@ -763,14 +780,20 @@ impl FirehoseService { .streams(&req.region) .and_then(|s| s.get(name)) .ok_or_else(|| not_found(name))?; - let tags: Vec = stream + let remaining: Vec<(&String, &String)> = stream .tags .iter() + .filter(|(k, _)| exclusive_start.is_none_or(|start| k.as_str() > start)) + .collect(); + let has_more = remaining.len() > limit; + let tags: Vec = remaining + .into_iter() + .take(limit) .map(|(k, v)| json!({"Key": k, "Value": v})) .collect(); Ok(AwsResponse::ok_json(json!({ "Tags": tags, - "HasMoreTags": false, + "HasMoreTags": has_more, }))) } @@ -1382,4 +1405,56 @@ mod tests { }; assert_eq!(err.code(), "InvalidArgumentException"); } + + #[test] + fn list_tags_for_delivery_stream_paginates() { + // Regression: ListTagsForDeliveryStream honors Limit + + // ExclusiveStartTagKey and sets HasMoreTags when a page is truncated, + // instead of returning every tag with HasMoreTags hardcoded false. + let svc = service(); + create(&svc, "tagged"); + svc.tag_delivery_stream(&request( + "TagDeliveryStream", + json!({ + "DeliveryStreamName": "tagged", + "Tags": [ + {"Key": "a", "Value": "1"}, + {"Key": "b", "Value": "2"}, + {"Key": "c", "Value": "3"} + ] + }), + )) + .unwrap(); + + // Page 1: Limit 2 -> first two keys, more remain. + let resp = svc + .list_tags_for_delivery_stream(&request( + "ListTagsForDeliveryStream", + json!({ "DeliveryStreamName": "tagged", "Limit": 2 }), + )) + .unwrap(); + let body: Value = serde_json::from_slice(resp.body.expect_bytes()).unwrap(); + let tags = body["Tags"].as_array().unwrap(); + assert_eq!(tags.len(), 2); + assert_eq!(tags[0]["Key"], "a"); + assert_eq!(tags[1]["Key"], "b"); + assert_eq!(body["HasMoreTags"], true); + + // Page 2: resume after "b". + let resp = svc + .list_tags_for_delivery_stream(&request( + "ListTagsForDeliveryStream", + json!({ + "DeliveryStreamName": "tagged", + "Limit": 2, + "ExclusiveStartTagKey": "b" + }), + )) + .unwrap(); + let body: Value = serde_json::from_slice(resp.body.expect_bytes()).unwrap(); + let tags = body["Tags"].as_array().unwrap(); + assert_eq!(tags.len(), 1); + assert_eq!(tags[0]["Key"], "c"); + assert_eq!(body["HasMoreTags"], false); + } } diff --git a/crates/fakecloud-route53/src/service/mod.rs b/crates/fakecloud-route53/src/service/mod.rs index 949888d87..5d7257ed8 100644 --- a/crates/fakecloud-route53/src/service/mod.rs +++ b/crates/fakecloud-route53/src/service/mod.rs @@ -1025,6 +1025,82 @@ mod tests { } } + #[test] + fn list_resource_record_sets_uses_reversed_dns_order() { + // Regression: Route 53 orders record sets by reversed-label DNS name + // (then type), not plain forward ASCII. The apex (`example.com.`) + // sorts first under `com.example`, ahead of `a.example.com.`. + fn named(name: &str) -> crate::model::ResourceRecordSet { + crate::model::ResourceRecordSet { + name: name.to_string(), + record_type: "A".to_string(), + ttl: Some(60), + resource_records: Some(crate::model::ResourceRecords { + resource_record: vec![crate::model::ResourceRecord { + value: "1.2.3.4".to_string(), + }], + }), + ..Default::default() + } + } + let (svc, zid) = svc_with_zone(vec![ + named("z.example.com."), + named("a.example.com."), + named("example.com."), + named("b.sub.example.com."), + ]); + let route = Route { + action: "ListResourceRecordSets", + id: Some(zid.clone()), + second_id: None, + }; + let req = AwsRequest { + service: "route53".to_string(), + action: "ListResourceRecordSets".to_string(), + region: "us-east-1".to_string(), + account_id: DEFAULT_ACCOUNT.to_string(), + request_id: "rid".to_string(), + headers: HeaderMap::new(), + query_params: std::collections::HashMap::new(), + body: Bytes::new(), + body_stream: parking_lot::Mutex::new(None), + path_segments: vec![ + "2013-04-01".into(), + "hostedzone".into(), + zid.clone(), + "rrset".into(), + ], + raw_path: format!("/2013-04-01/hostedzone/{zid}/rrset"), + raw_query: String::new(), + method: http::Method::GET, + is_query_protocol: false, + access_key_id: None, + principal: None, + }; + let resp = svc.list_resource_record_sets(&req, &route).unwrap(); + let body = std::str::from_utf8(resp.body.expect_bytes()) + .unwrap() + .to_string(); + // Collect values in document order. + let mut names = Vec::new(); + let mut rest = body.as_str(); + while let Some(start) = rest.find("") { + rest = &rest[start + "".len()..]; + let end = rest.find("").unwrap(); + names.push(rest[..end].to_string()); + rest = &rest[end + "".len()..]; + } + assert_eq!( + names, + vec![ + "example.com.".to_string(), + "a.example.com.".to_string(), + "b.sub.example.com.".to_string(), + "z.example.com.".to_string(), + ] + ); + } + fn empty_lookup() -> AliasLookup<'static> { AliasLookup { elbv2: None, diff --git a/crates/fakecloud-route53/src/service/records.rs b/crates/fakecloud-route53/src/service/records.rs index 943900c0a..5bebc5599 100644 --- a/crates/fakecloud-route53/src/service/records.rs +++ b/crates/fakecloud-route53/src/service/records.rs @@ -120,9 +120,11 @@ impl Route53Service { .ok_or_else(|| no_such_hosted_zone(&id))?; drop(state); - // Route 53 returns record sets ordered by (name, type) and paginates - // with maxitems + the StartRecordName/Type/Identifier cursor. Names, - // record types, and set identifiers are XML-safe DNS/enum values. + // Route 53 orders record sets by reversed-label DNS name + // (`www.example.com.` sorts under `com.example.www`) then by record + // type, and paginates with maxitems + the StartRecordName/Type/ + // Identifier cursor. Names, record types, and set identifiers are + // XML-safe DNS/enum values. let max_items = req .query_params .get("maxitems") @@ -136,32 +138,36 @@ impl Route53Service { let mut sorted: Vec<&crate::model::ResourceRecordSet> = zone.resource_record_sets.iter().collect(); sorted.sort_by(|a, b| { - a.name - .to_ascii_lowercase() - .cmp(&b.name.to_ascii_lowercase()) + reverse_dns_key(&a.name) + .cmp(&reverse_dns_key(&b.name)) .then(a.record_type.cmp(&b.record_type)) .then(a.set_identifier.cmp(&b.set_identifier)) }); let start_idx = match &start_name { None => 0, - Some(sn) => sorted - .iter() - .position(|r| match r.name.to_ascii_lowercase().cmp(sn) { - std::cmp::Ordering::Greater => true, - std::cmp::Ordering::Less => false, - std::cmp::Ordering::Equal => match &start_type { - None => true, - Some(st) => match r.record_type.cmp(st) { - std::cmp::Ordering::Greater => true, - std::cmp::Ordering::Less => false, - std::cmp::Ordering::Equal => start_ident - .as_deref() - .is_none_or(|si| r.set_identifier.as_deref().unwrap_or("") >= si), + Some(sn) => { + let start_key = reverse_dns_key(sn); + sorted + .iter() + .position(|r| match reverse_dns_key(&r.name).cmp(&start_key) { + std::cmp::Ordering::Greater => true, + std::cmp::Ordering::Less => false, + std::cmp::Ordering::Equal => match &start_type { + None => true, + Some(st) => match r.record_type.cmp(st) { + std::cmp::Ordering::Greater => true, + std::cmp::Ordering::Less => false, + std::cmp::Ordering::Equal => { + start_ident.as_deref().is_none_or(|si| { + r.set_identifier.as_deref().unwrap_or("") >= si + }) + } + }, }, - }, - }) - .unwrap_or(sorted.len()), + }) + .unwrap_or(sorted.len()) + } }; let page: Vec<&crate::model::ResourceRecordSet> = sorted diff --git a/crates/fakecloud-s3/src/service/objects/read.rs b/crates/fakecloud-s3/src/service/objects/read.rs index c2c3b65bd..cd13452f2 100644 --- a/crates/fakecloud-s3/src/service/objects/read.rs +++ b/crates/fakecloud-s3/src/service/objects/read.rs @@ -647,10 +647,10 @@ impl S3Service { let attr = attr.trim(); match attr { "ETag" => { - body_parts.push(format!( - ""{}"", - xml_escape(&obj.etag) - )); + // GetObjectAttributes returns the ETag WITHOUT the + // surrounding quotes that GetObject/HeadObject wrap it in + // (single-part `md5` and multipart `md5-N` alike). + body_parts.push(format!("{}", xml_escape(&obj.etag))); } "StorageClass" => { body_parts.push(format!( diff --git a/crates/fakecloud-s3/src/service/tests.rs b/crates/fakecloud-s3/src/service/tests.rs index f96dc75ff..123e37d93 100644 --- a/crates/fakecloud-s3/src/service/tests.rs +++ b/crates/fakecloud-s3/src/service/tests.rs @@ -3908,6 +3908,67 @@ async fn get_object_attributes_basic() { assert_eq!(resp.status, StatusCode::OK); } +#[tokio::test] +async fn get_object_attributes_etag_is_unquoted() { + // Regression: GetObjectAttributes returns the ETag WITHOUT the + // surrounding quotes that GetObject/HeadObject wrap it in. + let svc = make_service(); + seed_bucket(&svc, "goaetag"); + let req = make_request(Method::PUT, "/goaetag/k", &[], b"hi"); + svc.put_object("123456789012", &req, "goaetag", "k") + .await + .unwrap(); + + let mut req = make_request(Method::GET, "/goaetag/k", &[("attributes", "")], b""); + req.headers + .insert("x-amz-object-attributes", "ETag".parse().unwrap()); + let resp = svc + .get_object_attributes("123456789012", &req, "goaetag", "k") + .unwrap(); + let body = std::str::from_utf8(resp.body.expect_bytes()).unwrap(); + let md5 = compute_md5(b"hi"); + assert!( + body.contains(&format!("{md5}")), + "ETag must be unquoted: {body}" + ); + assert!(!body.contains("""), "ETag must not be quoted: {body}"); +} + +#[test] +fn get_object_attributes_multipart_etag_is_unquoted() { + // Regression: a multipart ETag (`md5-N`) is also returned unquoted. + let svc = make_service(); + seed_bucket(&svc, "goampu"); + { + let mut mas = svc.state.write(); + let state = mas.default_mut(); + let b = state.buckets.get_mut("goampu").expect("bucket seeded"); + let obj = S3Object { + key: "k".to_string(), + body: fakecloud_persistence::BodyRef::Memory(Bytes::from_static(b"x")), + content_type: "application/octet-stream".to_string(), + etag: "d41d8cd98f00b204e9800998ecf8427e-3".to_string(), + size: 1, + parts_count: Some(3), + last_modified: chrono::Utc::now(), + ..Default::default() + }; + b.objects.insert("k".to_string(), obj); + } + let mut req = make_request(Method::GET, "/goampu/k", &[("attributes", "")], b""); + req.headers + .insert("x-amz-object-attributes", "ETag".parse().unwrap()); + let resp = svc + .get_object_attributes("123456789012", &req, "goampu", "k") + .unwrap(); + let body = std::str::from_utf8(resp.body.expect_bytes()).unwrap(); + assert!( + body.contains("d41d8cd98f00b204e9800998ecf8427e-3"), + "multipart ETag must be unquoted: {body}" + ); + assert!(!body.contains("""), "ETag must not be quoted: {body}"); +} + #[test] fn get_object_attributes_bucket_not_found() { let svc = make_service(); diff --git a/crates/fakecloud-ses/src/service/contact_lists.rs b/crates/fakecloud-ses/src/service/contact_lists.rs index 8d80d9faa..c4caf699e 100644 --- a/crates/fakecloud-ses/src/service/contact_lists.rs +++ b/crates/fakecloud-ses/src/service/contact_lists.rs @@ -361,11 +361,32 @@ impl SesV2Service { )); } + // ListContacts Filter (optional): FilteredStatus (OPT_IN/OPT_OUT) + // and/or TopicFilter (TopicName + UseDefaultIfPreferenceUnavailable). + // Parsed from the request body; a GET or empty body means no filter. + let filter_body: Value = serde_json::from_slice(&req.body).unwrap_or(Value::Null); + let filter = &filter_body["Filter"]; + let filtered_status = filter["FilteredStatus"].as_str(); + let topic_filter_name = filter["TopicFilter"]["TopicName"].as_str(); + let use_default = filter["TopicFilter"]["UseDefaultIfPreferenceUnavailable"] + .as_bool() + .unwrap_or(false); + let list_def = state.contact_lists.get(list_name).unwrap(); + let contacts: Vec = state .contacts .get(list_name) .map(|m| { m.values() + .filter(|c| { + contact_matches_filter( + c, + list_def, + filtered_status, + topic_filter_name, + use_default, + ) + }) .map(|c| { let topic_prefs: Vec = c .topic_preferences @@ -491,3 +512,58 @@ impl SesV2Service { Ok(AwsResponse::json(StatusCode::OK, "{}")) } } + +/// Apply a `ListContacts` `Filter` to a single contact. A `None` facet +/// (status or topic) is unconstrained. +/// +/// * With a `TopicFilter`: resolve the contact's effective subscription +/// status for that topic — an explicit `UnsubscribeAll` forces `OPT_OUT`, +/// otherwise the contact's explicit topic preference is used, falling back +/// to the topic's default only when `UseDefaultIfPreferenceUnavailable`. +/// The contact matches when it has a resolved status and (if +/// `FilteredStatus` is set) that status equals the requested one. +/// * Without a `TopicFilter`: match by the contact's overall status +/// (`OPT_OUT` when `UnsubscribeAll`, else `OPT_IN`). +fn contact_matches_filter( + contact: &Contact, + list: &ContactList, + filtered_status: Option<&str>, + topic_name: Option<&str>, + use_default_if_unavailable: bool, +) -> bool { + match topic_name { + Some(topic) => { + let status: Option<&str> = if contact.unsubscribe_all { + Some("OPT_OUT") + } else if let Some(pref) = contact + .topic_preferences + .iter() + .find(|tp| tp.topic_name == topic) + { + Some(pref.subscription_status.as_str()) + } else if use_default_if_unavailable { + list.topics + .iter() + .find(|t| t.topic_name == topic) + .map(|t| t.default_subscription_status.as_str()) + } else { + None + }; + match status { + None => false, + Some(s) => filtered_status.is_none_or(|want| s == want), + } + } + None => match filtered_status { + None => true, + Some(want) => { + let status = if contact.unsubscribe_all { + "OPT_OUT" + } else { + "OPT_IN" + }; + status == want + } + }, + } +} diff --git a/crates/fakecloud-ses/src/service/misc.rs b/crates/fakecloud-ses/src/service/misc.rs index b80fb31a3..abdd73cd0 100644 --- a/crates/fakecloud-ses/src/service/misc.rs +++ b/crates/fakecloud-ses/src/service/misc.rs @@ -454,9 +454,8 @@ impl SesV2Service { // Verify template exists, then gate on the template's // FromEmailAddress matching a verified identity. Real SES v2 - // raises `MailFromDomainNotVerifiedException` from the - // SendCustomVerificationEmail action when the from-address has - // no matching verified email/domain identity. + // raises `MessageRejected` when the from-address has no matching + // verified email/domain identity. let from_email = { let accounts = self.state.read(); let empty = SesState::new(&req.account_id, &req.region); diff --git a/crates/fakecloud-ses/src/service/sending.rs b/crates/fakecloud-ses/src/service/sending.rs index 7124108fb..1e14d9dbb 100644 --- a/crates/fakecloud-ses/src/service/sending.rs +++ b/crates/fakecloud-ses/src/service/sending.rs @@ -178,8 +178,10 @@ impl SesV2Service { /// Reject sends where the sender is not a verified identity. Mirrors /// real SES: every From address must either match a verified email /// identity exactly, or its domain must match a verified domain - /// identity. Real SES v2 surfaces this as - /// `MailFromDomainNotVerifiedException` (HTTP 400). + /// identity. Real SES surfaces an unverified sender identity as + /// `MessageRejected` (HTTP 400) — matching the v1 path and unlike + /// `MailFromDomainNotVerifiedException`, which is reserved for a custom + /// MAIL FROM domain that has not been verified. pub(super) fn reject_unverified_sender( &self, account_id: &str, @@ -205,8 +207,10 @@ impl SesV2Service { } else { Some(Self::json_error( StatusCode::BAD_REQUEST, - "MailFromDomainNotVerifiedException", - "Mail-From domain not verified.", + "MessageRejected", + &format!( + "Email address is not verified. The following identities failed the check in region us-east-1: {email}" + ), )) } } diff --git a/crates/fakecloud-ses/src/service/tests.rs b/crates/fakecloud-ses/src/service/tests.rs index b861c4fcb..697038fa2 100644 --- a/crates/fakecloud-ses/src/service/tests.rs +++ b/crates/fakecloud-ses/src/service/tests.rs @@ -740,7 +740,7 @@ async fn test_send_email_rejects_unverified_sender() { let svc = SesV2Service::new(state); // No identity registered for sender@example.com → v2 surfaces this - // as MailFromDomainNotVerifiedException. + // as MessageRejected (matching real SES and the v1 path). let req = make_request( Method::POST, "/v2/email/outbound-emails", @@ -753,7 +753,7 @@ async fn test_send_email_rejects_unverified_sender() { let resp = svc.handle(req).await.unwrap(); assert_eq!(resp.status, StatusCode::BAD_REQUEST); let body: Value = serde_json::from_slice(resp.body.expect_bytes()).unwrap(); - assert_eq!(body["__type"], "MailFromDomainNotVerifiedException"); + assert_eq!(body["__type"], "MessageRejected"); } #[tokio::test] @@ -794,7 +794,7 @@ async fn send_email_v2_rejects_unverified_from_in_sandbox() { let resp = svc.handle(req).await.unwrap(); assert_eq!(resp.status, StatusCode::BAD_REQUEST); let body: Value = serde_json::from_slice(resp.body.expect_bytes()).unwrap(); - assert_eq!(body["__type"], "MailFromDomainNotVerifiedException"); + assert_eq!(body["__type"], "MessageRejected"); } #[tokio::test] @@ -1306,6 +1306,119 @@ async fn test_contact_lifecycle() { assert_eq!(resp.status, StatusCode::NOT_FOUND); } +#[tokio::test] +async fn list_contacts_applies_filtered_status() { + // Regression: ListContacts must apply Filter.FilteredStatus rather than + // returning every contact. A contact's overall status is OPT_OUT when + // UnsubscribeAll is set, else OPT_IN. + let state = make_state(); + let svc = SesV2Service::new(state); + + svc.handle(make_request( + Method::POST, + "/v2/email/contact-lists", + r#"{"ContactListName": "flist"}"#, + )) + .await + .unwrap(); + svc.handle(make_request( + Method::POST, + "/v2/email/contact-lists/flist/contacts", + r#"{"EmailAddress": "in@example.com", "UnsubscribeAll": false}"#, + )) + .await + .unwrap(); + svc.handle(make_request( + Method::POST, + "/v2/email/contact-lists/flist/contacts", + r#"{"EmailAddress": "out@example.com", "UnsubscribeAll": true}"#, + )) + .await + .unwrap(); + + // No filter -> both contacts. + let resp = svc + .handle(make_request( + Method::GET, + "/v2/email/contact-lists/flist/contacts", + "{}", + )) + .await + .unwrap(); + let body: Value = serde_json::from_slice(resp.body.expect_bytes()).unwrap(); + assert_eq!(body["Contacts"].as_array().unwrap().len(), 2); + + // FilteredStatus OPT_OUT -> only the opted-out contact. + let resp = svc + .handle(make_request( + Method::POST, + "/v2/email/contact-lists/flist/contacts/list", + r#"{"Filter": {"FilteredStatus": "OPT_OUT"}}"#, + )) + .await + .unwrap(); + let body: Value = serde_json::from_slice(resp.body.expect_bytes()).unwrap(); + let contacts = body["Contacts"].as_array().unwrap(); + assert_eq!(contacts.len(), 1); + assert_eq!(contacts[0]["EmailAddress"], "out@example.com"); +} + +#[tokio::test] +async fn list_contacts_applies_topic_filter() { + // Regression: ListContacts must apply Filter.TopicFilter, matching only + // contacts with the requested subscription status for that topic. + let state = make_state(); + let svc = SesV2Service::new(state); + + svc.handle(make_request( + Method::POST, + "/v2/email/contact-lists", + r#"{ + "ContactListName": "tlist", + "Topics": [{ + "TopicName": "news", + "DisplayName": "News", + "Description": "d", + "DefaultSubscriptionStatus": "OPT_OUT" + }] + }"#, + )) + .await + .unwrap(); + // Explicitly subscribed to "news". + svc.handle(make_request( + Method::POST, + "/v2/email/contact-lists/tlist/contacts", + r#"{ + "EmailAddress": "sub@example.com", + "TopicPreferences": [{"TopicName": "news", "SubscriptionStatus": "OPT_IN"}] + }"#, + )) + .await + .unwrap(); + // No preference for "news". + svc.handle(make_request( + Method::POST, + "/v2/email/contact-lists/tlist/contacts", + r#"{"EmailAddress": "nopref@example.com"}"#, + )) + .await + .unwrap(); + + let resp = svc + .handle(make_request( + Method::POST, + "/v2/email/contact-lists/tlist/contacts/list", + r#"{"Filter": {"FilteredStatus": "OPT_IN", "TopicFilter": {"TopicName": "news"}}}"#, + )) + .await + .unwrap(); + let body: Value = serde_json::from_slice(resp.body.expect_bytes()).unwrap(); + let contacts = body["Contacts"].as_array().unwrap(); + assert_eq!(contacts.len(), 1); + assert_eq!(contacts[0]["EmailAddress"], "sub@example.com"); +} + #[tokio::test] async fn test_duplicate_contact() { let state = make_state(); @@ -2772,10 +2885,7 @@ async fn test_send_custom_verification_email_unverified_sender() { let resp = svc.handle(req).await.unwrap(); assert_eq!(resp.status, StatusCode::BAD_REQUEST); let body: Value = serde_json::from_slice(resp.body.expect_bytes()).unwrap(); - assert_eq!( - body["__type"].as_str(), - Some("MailFromDomainNotVerifiedException") - ); + assert_eq!(body["__type"].as_str(), Some("MessageRejected")); } #[tokio::test] From 7f686a791ee804a7a008e266a16385832eca0734 Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Thu, 16 Jul 2026 22:31:29 -0300 Subject: [PATCH 2/3] test(s3,ses): update e2e assertions for unquoted GetObjectAttributes ETag + MessageRejected unverified sender --- crates/fakecloud-e2e/tests/s3.rs | 16 +++++++++------- crates/fakecloud-e2e/tests/ses.rs | 8 ++++++-- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/crates/fakecloud-e2e/tests/s3.rs b/crates/fakecloud-e2e/tests/s3.rs index 81a86799a..1b52a686e 100644 --- a/crates/fakecloud-e2e/tests/s3.rs +++ b/crates/fakecloud-e2e/tests/s3.rs @@ -3656,17 +3656,19 @@ async fn s3_get_object_attributes() { ); let json = output.stdout_json(); - // ETag must round-trip with the surrounding quote chars AWS itself - // emits — SDK clients compare this against PutObject's ETag, which is - // also quoted, so unquoted values would fail integrity checks. + // GetObjectAttributes returns the ETag WITHOUT the surrounding quote + // characters — unlike GetObject/HeadObject, which quote it. This matches + // AWS S3, which emits the raw entity tag for this operation. Compare + // against PutObject's (quoted) ETag with its quotes stripped. let etag = json["ETag"].as_str().expect("Expected ETag in response"); + let put_etag_unquoted = put_etag.trim_matches('"'); assert_eq!( - etag, put_etag, - "ETag mismatch: got {etag}, expected {put_etag}" + etag, put_etag_unquoted, + "ETag mismatch: got {etag}, expected {put_etag_unquoted}" ); assert!( - etag.starts_with('"') && etag.ends_with('"'), - "ETag must be quoted: {etag}" + !etag.starts_with('"') && !etag.ends_with('"'), + "GetObjectAttributes ETag must be unquoted: {etag}" ); // ObjectSize should match data length diff --git a/crates/fakecloud-e2e/tests/ses.rs b/crates/fakecloud-e2e/tests/ses.rs index 8d28f82e2..cdf62d0f7 100644 --- a/crates/fakecloud-e2e/tests/ses.rs +++ b/crates/fakecloud-e2e/tests/ses.rs @@ -1877,9 +1877,13 @@ async fn ses_send_custom_verification_email_unverified_sender() { .await .unwrap_err(); let dbg = format!("{err:?}"); + // An unverified sender is rejected with MessageRejected ("Email address is + // not verified") — matching real SES and the v1 path. + // MailFromDomainNotVerifiedException is reserved for an unverified custom + // MAIL FROM domain, a different condition. assert!( - dbg.contains("MailFromDomainNotVerifiedException"), - "expected MailFromDomainNotVerifiedException, got {dbg}" + dbg.contains("MessageRejected"), + "expected MessageRejected, got {dbg}" ); } From b014e90cc6079eb388f31476a158e97330557854 Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Fri, 17 Jul 2026 01:45:36 -0300 Subject: [PATCH 3/3] test(ses): unverified v2 sender returns MessageRejected in verification-gate e2e --- crates/fakecloud-e2e/tests/ses_verification_gate.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/fakecloud-e2e/tests/ses_verification_gate.rs b/crates/fakecloud-e2e/tests/ses_verification_gate.rs index a67df14ff..b2ee279ef 100644 --- a/crates/fakecloud-e2e/tests/ses_verification_gate.rs +++ b/crates/fakecloud-e2e/tests/ses_verification_gate.rs @@ -223,8 +223,10 @@ async fn sandbox_rejects_unverified_sender() { let client = server.sesv2_client().await; set_sandbox(&server, true).await; - // No identities at all — the From-address gate must trip with the - // dedicated `MailFromDomainNotVerifiedException` SES uses for v2. + // No identities at all — the From-address gate must trip. AWS SES returns + // `MessageRejected` ("Email address is not verified...") for an unverified + // sender on both v1 and v2; `MailFromDomainNotVerifiedException` is reserved + // for a configured-but-unverified custom MAIL FROM domain. let err = client .send_email() .from_email_address("noreply@unverified.test") @@ -253,8 +255,8 @@ async fn sandbox_rejects_unverified_sender() { let dbg = format!("{err:?}"); assert!( - dbg.contains("MailFromDomainNotVerified"), - "expected MailFromDomainNotVerifiedException, got: {dbg}" + dbg.contains("MessageRejected"), + "expected MessageRejected, got: {dbg}" ); }