From 7c4d1df9bad7b584a3914403809f9db689c3d6e0 Mon Sep 17 00:00:00 2001 From: Maurus Decimus <11444311+mdecimus@users.noreply.github.com> Date: Thu, 18 Jun 2026 17:48:22 +0200 Subject: [PATCH] Fix JMAP: Email/set, Email/copy, Email/import, Mailbox/set duplicate name --- CHANGELOG.md | 4 +++ crates/jmap-proto/src/references/resolve.rs | 2 +- crates/jmap/src/email/copy.rs | 36 ++++++++++++++------- crates/jmap/src/email/import.rs | 19 +++++++++-- crates/jmap/src/mailbox/set.rs | 6 ++-- 5 files changed, 49 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d01d99b..fec365ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,10 @@ If you are upgrading from v0.16.x, replace the binary (or run `docker pull`). If - `VacationResponse/set`: incorrect singleton handling. - `EmailSubmission/set`: return `sendAt` and `undoStatus` in the created response. - `Thread/changes`: emit a container delete when a thread becomes empty. + - `Mailbox/set`: Return `alreadyExists` instead of `invalidProperties` when creating a mailbox with an existing name. + - `Email/copy`: Take the source message id from the value's `id` property. + - `Email/set`: Bump reference-resolution max_depth from 1 to 2. + - `Email/import`: Reject blobs that do not contain valid messages. - OIDC: Add default domain name to groups that are not email addresses. - RocksDB: Enable blob garbage collection to reclaim disk space from deleted blobs. diff --git a/crates/jmap-proto/src/references/resolve.rs b/crates/jmap-proto/src/references/resolve.rs index e5884be6..90803283 100644 --- a/crates/jmap-proto/src/references/resolve.rs +++ b/crates/jmap-proto/src/references/resolve.rs @@ -60,7 +60,7 @@ impl Response<'_> { GetRequestMethod::Registry(request) => request.resolve_references(self)?, }, RequestMethod::Set(request) => match request { - SetRequestMethod::Email(request) => request.resolve_references(self, 1, false)?, + SetRequestMethod::Email(request) => request.resolve_references(self, 2, false)?, SetRequestMethod::Mailbox(request) => request.resolve_references(self, 1, false)?, SetRequestMethod::Identity(request) => { request.resolve_references(self, 1, false)? diff --git a/crates/jmap/src/email/copy.rs b/crates/jmap/src/email/copy.rs index 824aa420..43e8918f 100644 --- a/crates/jmap/src/email/copy.rs +++ b/crates/jmap/src/email/copy.rs @@ -89,24 +89,16 @@ impl JmapEmailCopy for Server { let mut destroy_ids = Vec::new(); 'create: for (id, create) in request.create.into_valid() { - let from_message_id = id.document_id(); - if !from_message_ids.contains(from_message_id) { - response.not_created.append( - id, - SetError::not_found().with_description(format!( - "Item {} not found in account {}.", - id, response.from_account_id - )), - ); - continue; - } - + let mut from_message_id = None; let mut mailboxes = Vec::new(); let mut keywords = Vec::new(); let mut received_at = None; for (property, value) in create.into_expanded_object() { match (property, value) { + (Key::Property(EmailProperty::Id), Value::Element(EmailValue::Id(src))) => { + from_message_id = Some(src.document_id()); + } (Key::Property(EmailProperty::MailboxIds), Value::Object(ids)) => { mailboxes = ids .into_expanded_boolean_set() @@ -163,6 +155,26 @@ impl JmapEmailCopy for Server { } } + let Some(from_message_id) = from_message_id else { + response.not_created.append( + id, + SetError::invalid_properties() + .with_property(EmailProperty::Id) + .with_description("Missing or invalid \"id\" property."), + ); + continue 'create; + }; + if !from_message_ids.contains(from_message_id) { + response.not_created.append( + id, + SetError::not_found().with_description(format!( + "Item {} not found in account {}.", + id, response.from_account_id + )), + ); + continue 'create; + } + // Make sure message belongs to at least one mailbox if mailboxes.is_empty() { response.not_created.append( diff --git a/crates/jmap/src/email/import.rs b/crates/jmap/src/email/import.rs index 76fbfb95..eabac5c3 100644 --- a/crates/jmap/src/email/import.rs +++ b/crates/jmap/src/email/import.rs @@ -21,7 +21,7 @@ use jmap_proto::{ request::MaybeInvalid, types::state::State, }; -use mail_parser::MessageParser; +use mail_parser::{HeaderName, MessageParser}; use std::future::Future; use types::{acl::Acl, id::Id, keyword::Keyword}; use utils::map::vec_map::VecMap; @@ -146,10 +146,25 @@ impl EmailImport for Server { }; // Import message + let parsed = MessageParser::new().parse(&raw_message); + let is_valid_message = parsed.as_ref().is_some_and(|message| { + message + .headers() + .iter() + .any(|header| !matches!(header.name, HeaderName::Other(_))) + }); + if !is_valid_message { + response.not_created.append( + id, + SetError::new(SetErrorType::InvalidEmail) + .with_description("Blob does not contain a valid RFC 5322 message."), + ); + continue; + } match self .email_ingest(IngestEmail { raw_message: &raw_message, - message: MessageParser::new().parse(&raw_message), + message: parsed, blob_hash: Some(&blob_id.hash), access_token: import_access_token.as_ref().unwrap_or(access_token), source: IngestSource::Jmap { diff --git a/crates/jmap/src/mailbox/set.rs b/crates/jmap/src/mailbox/set.rs index 3c9601ad..8db63ae8 100644 --- a/crates/jmap/src/mailbox/set.rs +++ b/crates/jmap/src/mailbox/set.rs @@ -561,13 +561,13 @@ impl MailboxSet for Server { if update .as_ref() .is_none_or(|(_, m)| m.inner.name != changes.name) - && cached_mailboxes.mailboxes.items.iter().any(|m| { + && let Some(existing) = cached_mailboxes.mailboxes.items.iter().find(|m| { m.name.to_lowercase() == lower_name && m.parent_id().map_or(0, |id| id + 1) == changes.parent_id }) { - return Ok(Err(SetError::invalid_properties() - .with_property(MailboxProperty::Name) + return Ok(Err(SetError::already_exists() + .with_existing_id(Id::from(existing.document_id)) .with_description(format!( "A mailbox with name '{}' already exists.", changes.name