From dc0f0c2879ac165a080e7cbf03a47f7ee0dd42ce Mon Sep 17 00:00:00 2001 From: Maurus Decimus <11444311+mdecimus@users.noreply.github.com> Date: Mon, 4 May 2026 16:45:37 +0200 Subject: [PATCH] Fix JMAP references in nested objects not resolved --- CHANGELOG.md | 1 + crates/jmap-proto/src/references/eval.rs | 138 ++++++-------- crates/jmap-proto/src/references/mod.rs | 228 +++++++++++++++++++++++ 3 files changed, 281 insertions(+), 86 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 20369ff1..699ad765 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ If you are upgrading from v0.16.x, replace the binary (or run `docker pull`). If - `mail-parser` panic with certain messages containing corrupted attachments. - Pagination by anchor for queued messages, tasks and metrics. - Spam filter: Use original instead of rewritten `RCPT` on checks. +- JMAP references in nested objects not resolved. ## [0.16.3] - 2026-04-30 diff --git a/crates/jmap-proto/src/references/eval.rs b/crates/jmap-proto/src/references/eval.rs index 4b00b007..c3cb663e 100644 --- a/crates/jmap-proto/src/references/eval.rs +++ b/crates/jmap-proto/src/references/eval.rs @@ -196,64 +196,73 @@ where max_depth: usize, eval_strings: bool, ) -> trc::Result<()> { - let Value::Object(obj) = self else { - return Ok(()); - }; - - for (key, value) in obj.as_mut_vec() { - // Resolve patch with references (e.g. mailboxIds/#idRef) - if depth == 0 - && let Key::Property(property) = key - && let Some(id_ref) = property.as_id_ref() - { - if let Some(id) = response.created_ids.get(id_ref) { - if !property.try_set_id(id.clone()) { + match self { + Value::Element(element) => { + if let Some(id_ref) = element.as_id_ref() { + if let Some(id) = response.created_ids.get(id_ref) { + if !element.try_set_id(id.clone()) { + return Err(trc::JmapEvent::InvalidResultReference + .into_err() + .details("Id reference points to invalid type.")); + } + } else if let Graph::Some { child_id, graph } = graph { + graph + .entry(child_id.to_string()) + .or_insert_with(Vec::new) + .push(id_ref.to_string()); + } else { return Err(trc::JmapEvent::InvalidResultReference .into_err() - .details("Id reference points to invalid type.")); + .details(format_compact!("Id reference {id_ref:?} not found."))); } - } else { - return Err(trc::JmapEvent::InvalidResultReference - .into_err() - .details(format_compact!("Id reference {id_ref:?} not found."))); } - } else if eval_strings - && let Some(id) = key - .as_string_key() - .and_then(|k| k.strip_prefix('#')) - .and_then(|id_ref| response.created_ids.get(id_ref)) - { - *key = Key::Owned(match id { - AnyId::Id(id) => id.to_string(), - AnyId::BlobId(id) => id.to_string(), - }); } - - match value { - Value::Element(element) => { - if let Some(id_ref) = element.as_id_ref() { + Value::Array(items) if depth < max_depth => { + // Resolve references in arrays (e.g. emailIds: [#idRef1, #idRef2]) + for item in items { + item.eval_object_references( + response, + graph, + depth + 1, + max_depth, + eval_strings, + )?; + } + } + Value::Object(items) if depth < max_depth => { + // Resolve references in JMAP sets (e.g. mailboxIds: { "#idRef1": true, "#idRef2": true }) + for (key, value) in items.as_mut_vec() { + if let Key::Property(property) = key + && let Some(id_ref) = property.as_id_ref() + { if let Some(id) = response.created_ids.get(id_ref) { - if !element.try_set_id(id.clone()) { + if !property.try_set_id(id.clone()) { return Err(trc::JmapEvent::InvalidResultReference .into_err() .details("Id reference points to invalid type.")); } - } else if let Graph::Some { child_id, graph } = graph { - graph - .entry(child_id.to_string()) - .or_insert_with(Vec::new) - .push(id_ref.to_string()); } else { return Err(trc::JmapEvent::InvalidResultReference .into_err() .details(format_compact!("Id reference {id_ref:?} not found."))); } + } else if eval_strings + && let Some(id) = key + .as_string_key() + .and_then(|k| k.strip_prefix('#')) + .and_then(|id_ref| response.created_ids.get(id_ref)) + { + *key = Key::Owned(match id { + AnyId::Id(id) => id.to_string(), + AnyId::BlobId(id) => id.to_string(), + }); } - } - Value::Array(items) if depth < max_depth => { - // Resolve references in arrays (e.g. emailIds: [#idRef1, #idRef2]) - for item in items { - item.eval_object_references( + + if matches!( + value, + Value::Element(_) | Value::Array(_) | Value::Object(_) + ) { + value.eval_object_references( response, graph, depth + 1, @@ -262,51 +271,8 @@ where )?; } } - Value::Object(items) if depth < max_depth => { - // Resolve references in JMAP sets (e.g. mailboxIds: { "#idRef1": true, "#idRef2": true }) - let visit_children = depth + 1 < max_depth; - for (key, value) in items.as_mut_vec() { - if let Key::Property(property) = key - && let Some(id_ref) = property.as_id_ref() - { - if let Some(id) = response.created_ids.get(id_ref) { - if !property.try_set_id(id.clone()) { - return Err(trc::JmapEvent::InvalidResultReference - .into_err() - .details("Id reference points to invalid type.")); - } - } else { - return Err(trc::JmapEvent::InvalidResultReference - .into_err() - .details(format_compact!( - "Id reference {id_ref:?} not found." - ))); - } - } else if eval_strings - && let Some(id) = key - .as_string_key() - .and_then(|k| k.strip_prefix('#')) - .and_then(|id_ref| response.created_ids.get(id_ref)) - { - *key = Key::Owned(match id { - AnyId::Id(id) => id.to_string(), - AnyId::BlobId(id) => id.to_string(), - }); - } - - if visit_children && matches!(value, Value::Object(_)) { - value.eval_object_references( - response, - graph, - depth + 1, - max_depth, - eval_strings, - )?; - } - } - } - _ => {} } + _ => {} } Ok(()) diff --git a/crates/jmap-proto/src/references/mod.rs b/crates/jmap-proto/src/references/mod.rs index 62ecb78b..fa1c3307 100644 --- a/crates/jmap-proto/src/references/mod.rs +++ b/crates/jmap-proto/src/references/mod.rs @@ -98,6 +98,8 @@ mod tests { }, response::{ChangesResponseMethod, GetResponseMethod, Response, ResponseMethod}, }; + use crate::references::Graph; + use crate::references::eval::EvalObjectReferences; use jmap_tools::{Key, Map, Value}; use std::collections::HashMap; use types::id::Id; @@ -721,4 +723,230 @@ mod tests { panic!("Expected Mailbox Set Request"); } } + + #[test] + fn eval_nested_element_ref() { + let mut created_ids = HashMap::new(); + created_ids.insert("server-1".to_string(), Id::new(42).into()); + let response = Response::new(0, created_ids, 0); + + let mut value: Value<'_, MailboxProperty, MailboxValue> = Value::Object(Map::from(vec![ + ( + Key::Property(MailboxProperty::Name), + Value::Str("inbox".into()), + ), + ( + Key::Property(MailboxProperty::ParentId), + Value::Object(Map::from(vec![( + Key::Property(MailboxProperty::ParentId), + Value::Element(MailboxValue::IdReference("server-1".into())), + )])), + ), + ])); + + value + .eval_object_references(&response, &mut Graph::None, 0, 5, true) + .unwrap(); + + let nested = value + .as_object() + .unwrap() + .get(&Key::Property(MailboxProperty::ParentId)) + .unwrap() + .as_object() + .unwrap() + .get(&Key::Property(MailboxProperty::ParentId)) + .unwrap(); + assert_eq!(nested, &Value::Element(MailboxValue::Id(Id::new(42)))); + } + + #[test] + fn eval_array_element_ref() { + let mut created_ids = HashMap::new(); + created_ids.insert("a".to_string(), Id::new(1).into()); + created_ids.insert("b".to_string(), Id::new(2).into()); + let response = Response::new(0, created_ids, 0); + + let mut value: Value<'_, MailboxProperty, MailboxValue> = + Value::Object(Map::from(vec![( + Key::Property(MailboxProperty::ParentId), + Value::Array(vec![ + Value::Element(MailboxValue::IdReference("a".into())), + Value::Element(MailboxValue::IdReference("b".into())), + ]), + )])); + + value + .eval_object_references(&response, &mut Graph::None, 0, 5, true) + .unwrap(); + + let arr = value + .as_object() + .unwrap() + .get(&Key::Property(MailboxProperty::ParentId)) + .unwrap() + .as_array() + .unwrap(); + assert_eq!(arr.len(), 2); + assert_eq!(arr[0], Value::Element(MailboxValue::Id(Id::new(1)))); + assert_eq!(arr[1], Value::Element(MailboxValue::Id(Id::new(2)))); + } + + #[test] + fn eval_nested_array_of_objects_with_element_ref() { + let mut created_ids = HashMap::new(); + created_ids.insert("a".to_string(), Id::new(7).into()); + let response = Response::new(0, created_ids, 0); + + let mut value: Value<'_, MailboxProperty, MailboxValue> = + Value::Object(Map::from(vec![( + Key::Property(MailboxProperty::ParentId), + Value::Array(vec![Value::Object(Map::from(vec![( + Key::Property(MailboxProperty::ParentId), + Value::Element(MailboxValue::IdReference("a".into())), + )]))]), + )])); + + value + .eval_object_references(&response, &mut Graph::None, 0, 5, true) + .unwrap(); + + let resolved = value + .as_object() + .unwrap() + .get(&Key::Property(MailboxProperty::ParentId)) + .unwrap() + .as_array() + .unwrap()[0] + .as_object() + .unwrap() + .get(&Key::Property(MailboxProperty::ParentId)) + .unwrap(); + assert_eq!(resolved, &Value::Element(MailboxValue::Id(Id::new(7)))); + } + + #[test] + fn eval_graph_collects_nested_ref() { + let response = Response::new(0, HashMap::new(), 0); + let mut graph_map: HashMap> = HashMap::new(); + let child_id = "outer".to_string(); + + let mut value: Value<'_, MailboxProperty, MailboxValue> = + Value::Object(Map::from(vec![( + Key::Property(MailboxProperty::ParentId), + Value::Object(Map::from(vec![( + Key::Property(MailboxProperty::ParentId), + Value::Element(MailboxValue::IdReference("inner".into())), + )])), + )])); + + { + let mut graph = Graph::Some { + child_id: &child_id, + graph: &mut graph_map, + }; + value + .eval_object_references(&response, &mut graph, 0, 5, true) + .unwrap(); + } + + assert_eq!(graph_map.get("outer"), Some(&vec!["inner".to_string()])); + } + + #[test] + fn eval_unresolved_nested_ref_errors_without_graph() { + let response = Response::new(0, HashMap::new(), 0); + + let mut value: Value<'_, MailboxProperty, MailboxValue> = + Value::Object(Map::from(vec![( + Key::Property(MailboxProperty::ParentId), + Value::Object(Map::from(vec![( + Key::Property(MailboxProperty::ParentId), + Value::Element(MailboxValue::IdReference("missing".into())), + )])), + )])); + + let err = value + .eval_object_references(&response, &mut Graph::None, 0, 5, true) + .unwrap_err(); + assert!( + err.matches(trc::EventType::Jmap( + trc::JmapEvent::InvalidResultReference + )), + "{:?}", + err + ); + } + + #[test] + fn eval_depth_limit_blocks_walk_into_inner_object() { + let mut created_ids = HashMap::new(); + created_ids.insert("inner".to_string(), Id::new(99).into()); + let response = Response::new(0, created_ids, 0); + + let mut value: Value<'_, MailboxProperty, MailboxValue> = + Value::Object(Map::from(vec![( + Key::Property(MailboxProperty::ParentId), + Value::Object(Map::from(vec![( + Key::Property(MailboxProperty::ParentId), + Value::Object(Map::from(vec![( + Key::Property(MailboxProperty::ParentId), + Value::Element(MailboxValue::IdReference("inner".into())), + )])), + )])), + )])); + + value + .eval_object_references(&response, &mut Graph::None, 0, 2, true) + .unwrap(); + + let deepest = value + .as_object() + .unwrap() + .get(&Key::Property(MailboxProperty::ParentId)) + .unwrap() + .as_object() + .unwrap() + .get(&Key::Property(MailboxProperty::ParentId)) + .unwrap() + .as_object() + .unwrap() + .get(&Key::Property(MailboxProperty::ParentId)) + .unwrap(); + assert_eq!( + deepest, + &Value::Element(MailboxValue::IdReference("inner".into())) + ); + } + + #[test] + fn eval_depth_limit_substitutes_element_at_max_depth() { + let mut created_ids = HashMap::new(); + created_ids.insert("inner".to_string(), Id::new(99).into()); + let response = Response::new(0, created_ids, 0); + + let mut value: Value<'_, MailboxProperty, MailboxValue> = + Value::Object(Map::from(vec![( + Key::Property(MailboxProperty::ParentId), + Value::Object(Map::from(vec![( + Key::Property(MailboxProperty::ParentId), + Value::Element(MailboxValue::IdReference("inner".into())), + )])), + )])); + + value + .eval_object_references(&response, &mut Graph::None, 0, 2, true) + .unwrap(); + + let resolved = value + .as_object() + .unwrap() + .get(&Key::Property(MailboxProperty::ParentId)) + .unwrap() + .as_object() + .unwrap() + .get(&Key::Property(MailboxProperty::ParentId)) + .unwrap(); + assert_eq!(resolved, &Value::Element(MailboxValue::Id(Id::new(99)))); + } }