diff --git a/CHANGELOG.md b/CHANGELOG.md index 648005f6..605e5f59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ If you are upgrading from v0.16.x, replace the binary (or run `docker pull`). If - Methods are only available if their capability is in `using`. - Reject requests that do not specify `application/json` in the `Content-Type` header. - Require `accountId` argument on requests. + - Return unparsable ids in `notFound` / `notUpdated` / `notDestroyed` / `notCopied` instead of dropping them. - Default calendars and address books are not subscribed by default. - `*/set`: Unchanged immutable `id` property is rejected on update. - `*/query` and `*/queryChanges`: null` rejected as `notRequest`. diff --git a/crates/jmap-proto/src/method/copy.rs b/crates/jmap-proto/src/method/copy.rs index 47eeb8cc..25d61d10 100644 --- a/crates/jmap-proto/src/method/copy.rs +++ b/crates/jmap-proto/src/method/copy.rs @@ -75,7 +75,7 @@ pub struct CopyBlobResponse { #[serde(rename = "notCopied")] #[serde(skip_serializing_if = "VecMap::is_empty")] - pub not_copied: VecMap>, + pub not_copied: VecMap, SetError>, } impl<'de, T: JmapObject> DeserializeArguments<'de> for CopyRequest<'de, T> { diff --git a/crates/jmap-proto/src/method/get.rs b/crates/jmap-proto/src/method/get.rs index b6cd707f..942ac336 100644 --- a/crates/jmap-proto/src/method/get.rs +++ b/crates/jmap-proto/src/method/get.rs @@ -7,7 +7,7 @@ use crate::{ object::JmapObject, request::{ - IntoValid, MaybeInvalid, + MaybeInvalid, deserialize::{DeserializeArguments, deserialize_request}, reference::{MaybeIdReference, MaybeResultReference, ResultReference}, }, @@ -37,7 +37,13 @@ pub struct GetResponse { pub list: Vec>, #[serde(rename = "notFound")] - pub not_found: Vec, + pub not_found: Vec>, +} + +impl GetResponse { + pub fn push_not_found(&mut self, id: T::Id) { + self.not_found.push(MaybeInvalid::Value(id)); + } } impl<'de, T: JmapObject> DeserializeArguments<'de> for GetRequest { @@ -116,16 +122,30 @@ impl GetRequest { } } - pub fn unwrap_ids(&mut self, max_objects_in_get: usize) -> trc::Result>> { + #[allow(clippy::type_complexity)] + pub fn unwrap_ids( + &mut self, + max_objects_in_get: usize, + ) -> trc::Result<(Option>, Vec>)> { if let Some(ids) = self.ids.take() { let ids = ids.unwrap(); if ids.len() <= max_objects_in_get { - Ok(Some(ids.into_valid().collect::>())) + let mut valid = Vec::with_capacity(ids.len()); + let mut invalid = Vec::new(); + for id in ids { + match id { + MaybeIdReference::Id(id) => valid.push(id), + MaybeIdReference::Invalid(s) | MaybeIdReference::Reference(s) => { + invalid.push(MaybeInvalid::Invalid(s)) + } + } + } + Ok((Some(valid), invalid)) } else { Err(trc::JmapEvent::RequestTooLarge.into_err()) } } else { - Ok(None) + Ok((None, Vec::new())) } } } diff --git a/crates/jmap-proto/src/method/parse.rs b/crates/jmap-proto/src/method/parse.rs index 71ce0869..ca0f2611 100644 --- a/crates/jmap-proto/src/method/parse.rs +++ b/crates/jmap-proto/src/method/parse.rs @@ -40,7 +40,7 @@ pub struct ParseResponse { #[serde(rename = "notFound")] #[serde(skip_serializing_if = "Vec::is_empty")] - pub not_found: Vec, + pub not_found: Vec>, } impl<'de, T: JmapObject> DeserializeArguments<'de> for ParseRequest { diff --git a/crates/jmap-proto/src/method/set.rs b/crates/jmap-proto/src/method/set.rs index 724be701..445f91f0 100644 --- a/crates/jmap-proto/src/method/set.rs +++ b/crates/jmap-proto/src/method/set.rs @@ -66,11 +66,11 @@ pub struct SetResponse { #[serde(rename = "notUpdated")] #[serde(skip_serializing_if = "VecMap::is_empty")] - pub not_updated: VecMap>, + pub not_updated: VecMap, SetError>, #[serde(rename = "notDestroyed")] #[serde(skip_serializing_if = "VecMap::is_empty")] - pub not_destroyed: VecMap>, + pub not_destroyed: VecMap, SetError>, } impl<'de, T: JmapObject> DeserializeArguments<'de> for SetRequest<'de, T> { @@ -211,6 +211,17 @@ impl SetResponse { self } + pub fn collect_will_destroy(&mut self, ids: Vec>) -> Vec { + let mut will_destroy = Vec::with_capacity(ids.len()); + for id in ids { + match id { + MaybeInvalid::Value(id) => will_destroy.push(id), + invalid => self.not_destroyed.append(invalid, SetError::not_found()), + } + } + will_destroy + } + pub fn created(&mut self, id: String, document_id: impl Into) { self.created.insert( id, diff --git a/crates/jmap-proto/src/object/calendar_event_notification.rs b/crates/jmap-proto/src/object/calendar_event_notification.rs index 8e543245..64c2db3c 100644 --- a/crates/jmap-proto/src/object/calendar_event_notification.rs +++ b/crates/jmap-proto/src/object/calendar_event_notification.rs @@ -73,7 +73,13 @@ pub struct CalendarEventNotificationGetResponse { pub list: Vec, #[serde(rename = "notFound")] - pub not_found: Vec, + pub not_found: Vec>, +} + +impl CalendarEventNotificationGetResponse { + pub fn push_not_found(&mut self, id: Id) { + self.not_found.push(crate::request::MaybeInvalid::Value(id)); + } } #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] diff --git a/crates/jmap-proto/src/request/mod.rs b/crates/jmap-proto/src/request/mod.rs index 73ed18fd..42f6224e 100644 --- a/crates/jmap-proto/src/request/mod.rs +++ b/crates/jmap-proto/src/request/mod.rs @@ -214,6 +214,12 @@ impl serde::Serialize for MaybeInvalid { } } +impl From for MaybeInvalid { + fn from(value: V) -> Self { + MaybeInvalid::Value(value) + } +} + impl Default for MaybeInvalid { fn default() -> Self { MaybeInvalid::Invalid("".to_string()) diff --git a/crates/jmap/src/addressbook/get.rs b/crates/jmap/src/addressbook/get.rs index 2a775ff4..d8e6811f 100644 --- a/crates/jmap/src/addressbook/get.rs +++ b/crates/jmap/src/addressbook/get.rs @@ -38,7 +38,7 @@ impl AddressBookGet for Server { mut request: GetRequest, access_token: &AccessToken, ) -> trc::Result> { - let ids = request.unwrap_ids(self.core.jmap.get_max_objects)?; + let (ids, not_found_ids) = request.unwrap_ids(self.core.jmap.get_max_objects)?; let properties = request.unwrap_properties(&[ AddressBookProperty::Id, AddressBookProperty::Name, @@ -92,14 +92,14 @@ impl AddressBookGet for Server { account_id: request.account_id.into(), state: cache.get_state(true).into(), list: Vec::with_capacity(ids.len()), - not_found: vec![], + not_found: not_found_ids, }; for id in ids { // Obtain the address_book object let document_id = id.document_id(); if !address_book_ids.contains(document_id) { - response.not_found.push(id); + response.push_not_found(id); continue; } let _address_book = if let Some(address_book) = self @@ -113,7 +113,7 @@ impl AddressBookGet for Server { { address_book } else { - response.not_found.push(id); + response.push_not_found(id); continue; }; let address_book = _address_book diff --git a/crates/jmap/src/addressbook/set.rs b/crates/jmap/src/addressbook/set.rs index 77558fdd..3d1b6b30 100644 --- a/crates/jmap/src/addressbook/set.rs +++ b/crates/jmap/src/addressbook/set.rs @@ -16,7 +16,7 @@ use jmap_proto::{ error::set::SetError, method::set::{SetRequest, SetResponse}, object::addressbook::{self, AddressBookProperty, AddressBookValue}, - request::{IntoValid, reference::MaybeIdReference}, + request::{MaybeInvalid, reference::MaybeIdReference}, types::state::State, }; use jmap_tools::{JsonPointerItem, Key, Value}; @@ -59,7 +59,7 @@ impl AddressBookSet for Server { ) .await?; let mut response = SetResponse::from_request(&request, self.core.jmap.set_max_objects)?; - let will_destroy = request.unwrap_destroy().into_valid().collect::>(); + let will_destroy = response.collect_will_destroy(request.unwrap_destroy()); let is_shared = access_token.is_shared(account_id); let mut set_default = None; @@ -133,7 +133,14 @@ impl AddressBookSet for Server { } // Process updates - 'update: for (id, object) in request.unwrap_update().into_valid() { + 'update: for (id, object) in request.unwrap_update() { + let id = match id { + MaybeInvalid::Value(id) => id, + invalid => { + response.not_updated.append(invalid, SetError::not_found()); + continue 'update; + } + }; // Make sure id won't be destroyed if will_destroy.contains(&id) { response.not_updated.append(id, SetError::will_destroy()); diff --git a/crates/jmap/src/blob/copy.rs b/crates/jmap/src/blob/copy.rs index 78f6c06d..0d79d093 100644 --- a/crates/jmap/src/blob/copy.rs +++ b/crates/jmap/src/blob/copy.rs @@ -9,7 +9,7 @@ use common::{Server, auth::AccessToken}; use jmap_proto::{ error::set::{SetError, SetErrorType}, method::copy::{CopyBlobRequest, CopyBlobResponse}, - request::IntoValid, + request::MaybeInvalid, }; use registry::schema::enums::Permission; use std::future::Future; @@ -40,7 +40,19 @@ impl BlobCopy for Server { }; let account_id = request.account_id.document_id(); - for blob_id in request.blob_ids.into_valid() { + for blob_id in request.blob_ids { + let blob_id = match blob_id { + MaybeInvalid::Value(blob_id) => blob_id, + invalid => { + response.not_copied.append( + invalid, + SetError::new(SetErrorType::BlobNotFound).with_description( + "blobId does not exist or not enough permissions to access it.", + ), + ); + continue; + } + }; if self.has_access_blob(&blob_id, access_token).await? { // Enforce quota if !access_token.has_permission(Permission::UnlimitedUploads) diff --git a/crates/jmap/src/blob/get.rs b/crates/jmap/src/blob/get.rs index af902776..22b47f6e 100644 --- a/crates/jmap/src/blob/get.rs +++ b/crates/jmap/src/blob/get.rs @@ -47,9 +47,8 @@ impl BlobOperations for Server { mut request: GetRequest, access_token: &AccessToken, ) -> trc::Result> { - let ids = request - .unwrap_ids(self.core.jmap.get_max_objects)? - .unwrap_or_default(); + let (ids, not_found_ids) = request.unwrap_ids(self.core.jmap.get_max_objects)?; + let ids = ids.unwrap_or_default(); let properties = request.unwrap_properties(&[ BlobProperty::Id, BlobProperty::Data(DataProperty::Default), @@ -59,7 +58,7 @@ impl BlobOperations for Server { account_id: request.account_id.into(), state: None, list: Vec::with_capacity(ids.len()), - not_found: vec![], + not_found: not_found_ids, }; let range_from = request.arguments.offset.unwrap_or(0); @@ -153,7 +152,7 @@ impl BlobOperations for Server { // Add result to response response.list.push(blob.into()); } else { - response.not_found.push(blob_id); + response.push_not_found(blob_id); } } diff --git a/crates/jmap/src/calendar/get.rs b/crates/jmap/src/calendar/get.rs index d9edfad0..0d282263 100644 --- a/crates/jmap/src/calendar/get.rs +++ b/crates/jmap/src/calendar/get.rs @@ -45,7 +45,7 @@ impl CalendarGet for Server { mut request: GetRequest, access_token: &AccessToken, ) -> trc::Result> { - let ids = request.unwrap_ids(self.core.jmap.get_max_objects)?; + let (ids, not_found_ids) = request.unwrap_ids(self.core.jmap.get_max_objects)?; let properties = request.unwrap_properties(&[ CalendarProperty::Id, CalendarProperty::Name, @@ -102,14 +102,14 @@ impl CalendarGet for Server { account_id: request.account_id.into(), state: cache.get_state(true).into(), list: Vec::with_capacity(ids.len()), - not_found: vec![], + not_found: not_found_ids, }; for id in ids { // Obtain the calendar object let document_id = id.document_id(); if !calendar_ids.contains(document_id) { - response.not_found.push(id); + response.push_not_found(id); continue; } let _calendar = if let Some(calendar) = self @@ -123,7 +123,7 @@ impl CalendarGet for Server { { calendar } else { - response.not_found.push(id); + response.push_not_found(id); continue; }; let calendar = _calendar diff --git a/crates/jmap/src/calendar/set.rs b/crates/jmap/src/calendar/set.rs index 1969fef2..e536e667 100644 --- a/crates/jmap/src/calendar/set.rs +++ b/crates/jmap/src/calendar/set.rs @@ -21,7 +21,7 @@ use jmap_proto::{ error::set::SetError, method::set::{SetRequest, SetResponse}, object::calendar::{self, CalendarProperty, CalendarValue, IncludeInAvailability}, - request::{IntoValid, reference::MaybeIdReference}, + request::{MaybeInvalid, reference::MaybeIdReference}, types::state::State, }; use jmap_tools::{JsonPointerItem, Key, Map, Value}; @@ -64,7 +64,7 @@ impl CalendarSet for Server { ) .await?; let mut response = SetResponse::from_request(&request, self.core.jmap.set_max_objects)?; - let will_destroy = request.unwrap_destroy().into_valid().collect::>(); + let will_destroy = response.collect_will_destroy(request.unwrap_destroy()); let is_shared = access_token.is_shared(account_id); let mut set_default = None; @@ -138,7 +138,14 @@ impl CalendarSet for Server { } // Process updates - 'update: for (id, object) in request.unwrap_update().into_valid() { + 'update: for (id, object) in request.unwrap_update() { + let id = match id { + MaybeInvalid::Value(id) => id, + invalid => { + response.not_updated.append(invalid, SetError::not_found()); + continue 'update; + } + }; // Make sure id won't be destroyed if will_destroy.contains(&id) { response.not_updated.append(id, SetError::will_destroy()); diff --git a/crates/jmap/src/calendar_event/get.rs b/crates/jmap/src/calendar_event/get.rs index 4c1775b2..58719777 100644 --- a/crates/jmap/src/calendar_event/get.rs +++ b/crates/jmap/src/calendar_event/get.rs @@ -213,7 +213,7 @@ impl CalendarEventGet for Server { // Obtain the calendar_event object let document_id = id.document_id(); if !calendar_event_ids.contains(document_id) { - response.not_found.push(id); + response.push_not_found(id); continue; } @@ -226,7 +226,7 @@ impl CalendarEventGet for Server { )) .await? else { - response.not_found.push(id); + response.push_not_found(id); continue; }; let mut calendar_event = _calendar_event @@ -290,7 +290,7 @@ impl CalendarEventGet for Server { { for expansion in expansions { if !expansion.is_valid() { - response.not_found.push(::new( + response.push_not_found(::new( expansion.expansion_id, document_id, )); @@ -411,11 +411,12 @@ impl CalendarEventGet for Server { )); } } else { - response - .not_found - .extend(expansion_ids.into_iter().map(|expansion_id| { - ::new(expansion_id, document_id) - })); + for expansion_id in expansion_ids { + response.push_not_found(::new( + expansion_id, + document_id, + )); + } continue; } } diff --git a/crates/jmap/src/calendar_event/parse.rs b/crates/jmap/src/calendar_event/parse.rs index 825bd8f6..b6507aa0 100644 --- a/crates/jmap/src/calendar_event/parse.rs +++ b/crates/jmap/src/calendar_event/parse.rs @@ -13,7 +13,7 @@ use common::{Server, auth::AccessToken}; use jmap_proto::{ method::parse::{ParseRequest, ParseResponse}, object::calendar_event::CalendarEvent, - request::IntoValid, + request::{IntoValid, MaybeInvalid}, }; use jmap_tools::{Key, Value}; use types::{blob::BlobId, id::Id}; @@ -54,7 +54,7 @@ impl CalendarEventParse for Server { let raw_vcard = match self.blob_download(&blob_id, access_token).await? { Some(raw_vcard) => raw_vcard, None => { - response.not_found.push(blob_id); + response.not_found.push(MaybeInvalid::Value(blob_id)); continue; } }; diff --git a/crates/jmap/src/calendar_event/set.rs b/crates/jmap/src/calendar_event/set.rs index cd93225a..b8690217 100644 --- a/crates/jmap/src/calendar_event/set.rs +++ b/crates/jmap/src/calendar_event/set.rs @@ -34,7 +34,7 @@ use jmap_proto::{ error::set::SetError, method::set::{SetRequest, SetResponse}, object::calendar_event, - request::IntoValid, + request::MaybeInvalid, types::state::State, }; use jmap_tools::{JsonPointerHandler, JsonPointerItem, Key, Map, Value}; @@ -97,7 +97,7 @@ impl CalendarEventSet for Server { .await .caused_by(trc::location!())?; let mut response = SetResponse::from_request(&request, self.core.jmap.set_max_objects)?; - let will_destroy = request.unwrap_destroy().into_valid().collect::>(); + let will_destroy = response.collect_will_destroy(request.unwrap_destroy()); // Obtain calendarIds let (can_add_calendars, can_delete_calendars, can_modify_calendars) = @@ -146,7 +146,14 @@ impl CalendarEventSet for Server { } // Process updates - 'update: for (id, object) in request.unwrap_update().into_valid() { + 'update: for (id, object) in request.unwrap_update() { + let id = match id { + MaybeInvalid::Value(id) => id, + invalid => { + response.not_updated.append(invalid, SetError::not_found()); + continue 'update; + } + }; // Make sure id won't be destroyed if will_destroy.contains(&id) { response.not_updated.append(id, SetError::will_destroy()); diff --git a/crates/jmap/src/calendar_event_notification/get.rs b/crates/jmap/src/calendar_event_notification/get.rs index 4edd15cb..e8a12e24 100644 --- a/crates/jmap/src/calendar_event_notification/get.rs +++ b/crates/jmap/src/calendar_event_notification/get.rs @@ -50,7 +50,7 @@ impl CalendarEventNotificationGet for Server { mut request: GetRequest, access_token: &AccessToken, ) -> trc::Result { - let ids = request.unwrap_ids(self.core.jmap.get_max_objects)?; + let (ids, not_found_ids) = request.unwrap_ids(self.core.jmap.get_max_objects)?; let properties = request.unwrap_properties(&[ CalendarEventNotificationProperty::Id, CalendarEventNotificationProperty::Created, @@ -80,7 +80,7 @@ impl CalendarEventNotificationGet for Server { account_id: request.account_id.into(), state: cache.get_state(false).into(), list: Vec::with_capacity(ids.len()), - not_found: vec![], + not_found: not_found_ids, }; for id in ids { @@ -97,7 +97,7 @@ impl CalendarEventNotificationGet for Server { { event } else { - response.not_found.push(id); + response.push_not_found(id); continue; }; let event = _event diff --git a/crates/jmap/src/contact/get.rs b/crates/jmap/src/contact/get.rs index 0abcd8d6..533bf50f 100644 --- a/crates/jmap/src/contact/get.rs +++ b/crates/jmap/src/contact/get.rs @@ -41,7 +41,7 @@ impl ContactCardGet for Server { mut request: GetRequest, access_token: &AccessToken, ) -> trc::Result> { - let ids = request.unwrap_ids(self.core.jmap.get_max_objects)?; + let (ids, not_found_ids) = request.unwrap_ids(self.core.jmap.get_max_objects)?; let return_all_properties = request .properties .as_ref() @@ -74,7 +74,7 @@ impl ContactCardGet for Server { account_id: request.account_id.into(), state: cache.get_state(false).into(), list: Vec::with_capacity(ids.len()), - not_found: vec![], + not_found: not_found_ids, }; let mut return_id = return_all_properties; let mut return_address_book_ids = return_all_properties; @@ -101,7 +101,7 @@ impl ContactCardGet for Server { // Obtain the contact object let document_id = id.document_id(); if !contact_ids.contains(document_id) { - response.not_found.push(id); + response.push_not_found(id); continue; } @@ -116,7 +116,7 @@ impl ContactCardGet for Server { { contact } else { - response.not_found.push(id); + response.push_not_found(id); continue; }; diff --git a/crates/jmap/src/contact/parse.rs b/crates/jmap/src/contact/parse.rs index 1679087c..bfd64a65 100644 --- a/crates/jmap/src/contact/parse.rs +++ b/crates/jmap/src/contact/parse.rs @@ -10,7 +10,7 @@ use common::{Server, auth::AccessToken}; use jmap_proto::{ method::parse::{ParseRequest, ParseResponse}, object::contact::ContactCard, - request::IntoValid, + request::{IntoValid, MaybeInvalid}, }; use types::{blob::BlobId, id::Id}; use utils::map::vec_map::VecMap; @@ -50,7 +50,7 @@ impl ContactCardParse for Server { let raw_vcard = match self.blob_download(&blob_id, access_token).await? { Some(raw_vcard) => raw_vcard, None => { - response.not_found.push(blob_id); + response.not_found.push(MaybeInvalid::Value(blob_id)); continue; } }; diff --git a/crates/jmap/src/contact/set.rs b/crates/jmap/src/contact/set.rs index 97a3778c..0c6602e7 100644 --- a/crates/jmap/src/contact/set.rs +++ b/crates/jmap/src/contact/set.rs @@ -16,7 +16,7 @@ use jmap_proto::{ error::set::SetError, method::set::{SetRequest, SetResponse}, object::contact, - request::IntoValid, + request::MaybeInvalid, types::state::State, }; use jmap_tools::{JsonPointerHandler, JsonPointerItem, Key, Value}; @@ -73,7 +73,7 @@ impl ContactCardSet for Server { ) .await?; let mut response = SetResponse::from_request(&request, self.core.jmap.set_max_objects)?; - let will_destroy = request.unwrap_destroy().into_valid().collect::>(); + let will_destroy = response.collect_will_destroy(request.unwrap_destroy()); // Obtain addressBookIds let (can_add_address_books, can_delete_address_books, can_modify_address_books) = @@ -120,7 +120,14 @@ impl ContactCardSet for Server { } // Process updates - 'update: for (id, object) in request.unwrap_update().into_valid() { + 'update: for (id, object) in request.unwrap_update() { + let id = match id { + MaybeInvalid::Value(id) => id, + invalid => { + response.not_updated.append(invalid, SetError::not_found()); + continue 'update; + } + }; // Make sure id won't be destroyed if will_destroy.contains(&id) { response.not_updated.append(id, SetError::will_destroy()); diff --git a/crates/jmap/src/email/get.rs b/crates/jmap/src/email/get.rs index b08aabd6..4577dfd1 100644 --- a/crates/jmap/src/email/get.rs +++ b/crates/jmap/src/email/get.rs @@ -55,7 +55,7 @@ impl EmailGet for Server { mut request: GetRequest, access_token: &AccessToken, ) -> trc::Result> { - let ids = request.unwrap_ids(self.core.jmap.get_max_objects)?; + let (ids, not_found_ids) = request.unwrap_ids(self.core.jmap.get_max_objects)?; let properties = request.unwrap_properties(&[ EmailProperty::Id, EmailProperty::BlobId, @@ -131,7 +131,7 @@ impl EmailGet for Server { account_id: request.account_id.into(), state: cache.get_state(false).into(), list: Vec::with_capacity(ids.len()), - not_found: vec![], + not_found: not_found_ids, }; // Check if we need to fetch the raw headers or body @@ -153,7 +153,7 @@ impl EmailGet for Server { for id in ids { // Obtain the email object if !message_ids.contains(id.document_id()) { - response.not_found.push(id); + response.push_not_found(id); continue; } let metadata_ = match self @@ -168,7 +168,7 @@ impl EmailGet for Server { { Some(metadata) => metadata, None => { - response.not_found.push(id); + response.push_not_found(id); continue; } }; @@ -180,7 +180,7 @@ impl EmailGet for Server { let data = match cache.email_by_id(&id.document_id()) { Some(data) => data, None => { - response.not_found.push(id); + response.push_not_found(id); continue; } }; @@ -212,7 +212,7 @@ impl EmailGet for Server { CausedBy = trc::location!(), ); - response.not_found.push(id); + response.push_not_found(id); continue; } } diff --git a/crates/jmap/src/email/parse.rs b/crates/jmap/src/email/parse.rs index 6dd64c2e..d7f2fba5 100644 --- a/crates/jmap/src/email/parse.rs +++ b/crates/jmap/src/email/parse.rs @@ -14,11 +14,12 @@ use email::message::index::PREVIEW_LENGTH; use jmap_proto::{ method::parse::{ParseRequest, ParseResponse}, object::email::{Email, EmailProperty}, - request::IntoValid, + request::{IntoValid, MaybeInvalid, reference::MaybeIdReference}, }; use jmap_tools::{Key, Map, Value}; use mail_parser::{ - MessageParser, PartType, decoders::html::html_to_text, parsers::preview::preview_text, + HeaderName, MessageParser, PartType, decoders::html::html_to_text, + parsers::preview::preview_text, }; use std::future::Future; use utils::{chained_bytes::ChainedBytes, map::vec_map::VecMap}; @@ -97,20 +98,34 @@ impl EmailParse for Server { not_found: vec![], }; - for blob_id in request.blob_ids.into_valid() { + for blob_id in request.blob_ids { + let blob_id = match blob_id { + MaybeIdReference::Id(blob_id) => blob_id, + MaybeIdReference::Invalid(s) | MaybeIdReference::Reference(s) => { + response.not_found.push(MaybeInvalid::Invalid(s)); + continue; + } + }; // Fetch raw message to parse let raw_message = match self.blob_download(&blob_id, access_token).await? { Some(raw_message) => raw_message, None => { - response.not_found.push(blob_id); + response.not_found.push(MaybeInvalid::Value(blob_id)); continue; } }; - let message = if let Some(message) = MessageParser::new().parse(&raw_message) { + let message = match MessageParser::new().parse(&raw_message).filter(|message| { message - } else { - response.not_parsable.push(blob_id); - continue; + .root_part() + .headers() + .iter() + .any(|header| !matches!(header.name, HeaderName::Other(_))) + }) { + Some(message) => message, + None => { + response.not_parsable.push(blob_id); + continue; + } }; let raw_message = ChainedBytes::new(&raw_message); diff --git a/crates/jmap/src/email/set.rs b/crates/jmap/src/email/set.rs index 34dbe5ed..414fb21c 100644 --- a/crates/jmap/src/email/set.rs +++ b/crates/jmap/src/email/set.rs @@ -28,7 +28,7 @@ use jmap_proto::{ method::set::{SetRequest, SetResponse}, object::email::{Email, EmailProperty, EmailValue}, references::resolve::ResolveCreatedReference, - request::IntoValid, + request::MaybeInvalid, types::state::State, }; use jmap_tools::{Key, Value}; @@ -117,7 +117,7 @@ impl EmailSet for Server { }; let mut last_change_id = None; - let will_destroy = request.unwrap_destroy().into_valid().collect::>(); + let will_destroy = response.collect_will_destroy(request.unwrap_destroy()); // Process creates 'create: for (id, object) in request.unwrap_create() { @@ -792,7 +792,14 @@ impl EmailSet for Server { let mut batch = BatchBuilder::new(); let mut changed_mailboxes: AHashMap> = AHashMap::new(); let mut will_update = Vec::with_capacity(request.update.as_ref().map_or(0, |u| u.len())); - 'update: for (id, object) in request.unwrap_update().into_valid() { + 'update: for (id, object) in request.unwrap_update() { + let id = match id { + MaybeInvalid::Value(id) => id, + invalid => { + response.not_updated.append(invalid, SetError::not_found()); + continue 'update; + } + }; // Make sure id won't be destroyed if will_destroy.contains(&id) { response.not_updated.append(id, SetError::will_destroy()); diff --git a/crates/jmap/src/file/get.rs b/crates/jmap/src/file/get.rs index 52a47720..6846b803 100644 --- a/crates/jmap/src/file/get.rs +++ b/crates/jmap/src/file/get.rs @@ -40,7 +40,7 @@ impl FileNodeGet for Server { mut request: GetRequest, access_token: &AccessToken, ) -> trc::Result> { - let ids = request.unwrap_ids(self.core.jmap.get_max_objects)?; + let (ids, not_found_ids) = request.unwrap_ids(self.core.jmap.get_max_objects)?; let properties = request.unwrap_properties(&[ FileNodeProperty::Id, FileNodeProperty::ParentId, @@ -114,14 +114,14 @@ impl FileNodeGet for Server { account_id: request.account_id.into(), state: cache.get_state(false).into(), list: Vec::with_capacity(ids.len()), - not_found: vec![], + not_found: not_found_ids, }; for id in ids { // Obtain the file_node object let document_id = id.document_id(); if !file_node_ids.contains(document_id) { - response.not_found.push(id); + response.push_not_found(id); continue; } let _file_node = if let Some(file_node) = self @@ -135,7 +135,7 @@ impl FileNodeGet for Server { { file_node } else { - response.not_found.push(id); + response.push_not_found(id); continue; }; let file_node = _file_node diff --git a/crates/jmap/src/file/set.rs b/crates/jmap/src/file/set.rs index bc2a9ee4..8f7190b0 100644 --- a/crates/jmap/src/file/set.rs +++ b/crates/jmap/src/file/set.rs @@ -19,7 +19,7 @@ use jmap_proto::{ file_node::{self, FileNodeProperty, FileNodeValue, OnExists}, }, references::resolve::ResolveCreatedReference, - request::IntoValid, + request::MaybeInvalid, types::state::State, }; use jmap_tools::{JsonPointerItem, Key, Value}; @@ -68,7 +68,7 @@ impl FileNodeSet for Server { ) .await?; let mut response = SetResponse::from_request(&request, self.core.jmap.set_max_objects)?; - let mut will_destroy = request.unwrap_destroy().into_valid().collect::>(); + let mut will_destroy = response.collect_will_destroy(request.unwrap_destroy()); let is_shared = access_token.is_shared(account_id); let on_destroy_remove_children = request .arguments @@ -312,7 +312,14 @@ impl FileNodeSet for Server { } // Process updates - 'update: for (id, object) in request.unwrap_update().into_valid() { + 'update: for (id, object) in request.unwrap_update() { + let id = match id { + MaybeInvalid::Value(id) => id, + invalid => { + response.not_updated.append(invalid, SetError::not_found()); + continue 'update; + } + }; // Make sure id won't be destroyed if will_destroy.contains(&id) || implicit_destroys.contains(&id.document_id()) { response.not_updated.append(id, SetError::will_destroy()); diff --git a/crates/jmap/src/identity/get.rs b/crates/jmap/src/identity/get.rs index d7350516..5a14e1a8 100644 --- a/crates/jmap/src/identity/get.rs +++ b/crates/jmap/src/identity/get.rs @@ -43,7 +43,7 @@ impl IdentityGet for Server { &self, mut request: GetRequest, ) -> trc::Result> { - let ids = request.unwrap_ids(self.core.jmap.get_max_objects)?; + let (ids, not_found_ids) = request.unwrap_ids(self.core.jmap.get_max_objects)?; let properties = request.unwrap_properties(&[ IdentityProperty::Id, IdentityProperty::Name, @@ -72,14 +72,14 @@ impl IdentityGet for Server { .await? .into(), list: Vec::with_capacity(ids.len()), - not_found: vec![], + not_found: not_found_ids, }; for id in ids { // Obtain the identity object let document_id = id.document_id(); if !identity_ids.contains(document_id) { - response.not_found.push(id); + response.push_not_found(id); continue; } let _identity = if let Some(identity) = self @@ -93,7 +93,7 @@ impl IdentityGet for Server { { identity } else { - response.not_found.push(id); + response.push_not_found(id); continue; }; let identity = _identity diff --git a/crates/jmap/src/identity/set.rs b/crates/jmap/src/identity/set.rs index 96e408d7..f52cd714 100644 --- a/crates/jmap/src/identity/set.rs +++ b/crates/jmap/src/identity/set.rs @@ -11,7 +11,7 @@ use jmap_proto::{ method::set::{SetRequest, SetResponse}, object::identity::{self, IdentityProperty, IdentityValue}, references::resolve::ResolveCreatedReference, - request::IntoValid, + request::MaybeInvalid, types::state::State, }; use jmap_tools::{Key, Value}; @@ -46,7 +46,7 @@ impl IdentitySet for Server { .document_ids(account_id, Collection::Identity, IdentityField::DocumentId) .await?; let mut response = SetResponse::from_request(&request, self.core.jmap.set_max_objects)?; - let will_destroy = request.unwrap_destroy().into_valid().collect::>(); + let will_destroy = response.collect_will_destroy(request.unwrap_destroy()); let account_info = self .account_info(account_id) .await @@ -131,7 +131,14 @@ impl IdentitySet for Server { } // Process updates - 'update: for (id, object) in request.unwrap_update().into_valid() { + 'update: for (id, object) in request.unwrap_update() { + let id = match id { + MaybeInvalid::Value(id) => id, + invalid => { + response.not_updated.append(invalid, SetError::not_found()); + continue 'update; + } + }; // Make sure id won't be destroyed if will_destroy.contains(&id) { response.not_updated.append(id, SetError::will_destroy()); diff --git a/crates/jmap/src/mailbox/get.rs b/crates/jmap/src/mailbox/get.rs index e4f19b58..5ec2ec30 100644 --- a/crates/jmap/src/mailbox/get.rs +++ b/crates/jmap/src/mailbox/get.rs @@ -31,7 +31,7 @@ impl MailboxGet for Server { mut request: GetRequest, access_token: &AccessToken, ) -> trc::Result> { - let ids = request.unwrap_ids(self.core.jmap.get_max_objects)?; + let (ids, not_found_ids) = request.unwrap_ids(self.core.jmap.get_max_objects)?; let properties = request.unwrap_properties(&[ MailboxProperty::Id, MailboxProperty::Name, @@ -69,7 +69,7 @@ impl MailboxGet for Server { account_id: request.account_id.into(), state: Some(cache.mailboxes.change_id.into()), list: Vec::with_capacity(ids.len()), - not_found: vec![], + not_found: not_found_ids, }; for id in ids { @@ -83,7 +83,7 @@ impl MailboxGet for Server { }) { mailbox } else { - response.not_found.push(id); + response.push_not_found(id); continue; }; diff --git a/crates/jmap/src/mailbox/set.rs b/crates/jmap/src/mailbox/set.rs index 8db63ae8..d4fc5667 100644 --- a/crates/jmap/src/mailbox/set.rs +++ b/crates/jmap/src/mailbox/set.rs @@ -25,7 +25,7 @@ use jmap_proto::{ method::set::{SetRequest, SetResponse}, object::mailbox::{self, MailboxProperty, MailboxValue}, references::resolve::ResolveCreatedReference, - request::IntoValid, + request::MaybeInvalid, types::state::State, }; use jmap_tools::{JsonPointerItem, Key, Map, Value}; @@ -80,14 +80,16 @@ impl MailboxSet for Server { let account_id = request.account_id.document_id(); let on_destroy_remove_emails = request.arguments.on_destroy_remove_emails.unwrap_or(false); let cache = self.get_cached_messages(account_id).await?; + let mut response = SetResponse::from_request(&request, self.core.jmap.set_max_objects)? + .with_state(cache.assert_state(true, &request.if_in_state)?); + let will_destroy = response.collect_will_destroy(request.unwrap_destroy()); let mut ctx = SetContext { account_id, is_shared: access_token.is_shared(account_id), access_token, - response: SetResponse::from_request(&request, self.core.jmap.set_max_objects)? - .with_state(cache.assert_state(true, &request.if_in_state)?), + response, mailbox_ids: RoaringBitmap::from_iter(cache.mailboxes.index.keys()), - will_destroy: request.unwrap_destroy().into_valid().collect(), + will_destroy, }; let mut change_id = None; let account_info = self.account(account_id).await?; @@ -161,7 +163,16 @@ impl MailboxSet for Server { // Process updates let mut will_update = Vec::with_capacity(request.update.as_ref().map_or(0, |u| u.len())); let mut batch = BatchBuilder::new(); - 'update: for (id, object) in request.unwrap_update().into_valid() { + 'update: for (id, object) in request.unwrap_update() { + let id = match id { + MaybeInvalid::Value(id) => id, + invalid => { + ctx.response + .not_updated + .append(invalid, SetError::not_found()); + continue 'update; + } + }; // Make sure id won't be destroyed if ctx.will_destroy.contains(&id) { ctx.response diff --git a/crates/jmap/src/participant_identity/get.rs b/crates/jmap/src/participant_identity/get.rs index 197557b8..00eb39a4 100644 --- a/crates/jmap/src/participant_identity/get.rs +++ b/crates/jmap/src/participant_identity/get.rs @@ -35,7 +35,7 @@ impl ParticipantIdentityGet for Server { &self, mut request: GetRequest, ) -> trc::Result> { - let ids = request.unwrap_ids(self.core.jmap.get_max_objects)?; + let (ids, not_found_ids) = request.unwrap_ids(self.core.jmap.get_max_objects)?; let properties = request.unwrap_properties(&[ ParticipantIdentityProperty::Id, ParticipantIdentityProperty::Name, @@ -49,11 +49,13 @@ impl ParticipantIdentityGet for Server { account_id: request.account_id.into(), state: None, list: Vec::new(), - not_found: vec![], + not_found: not_found_ids, }; let Some(identities) = identities else { - response.not_found = ids.unwrap_or_default(); + for id in ids.unwrap_or_default() { + response.push_not_found(id); + } return Ok(response); }; @@ -76,7 +78,7 @@ impl ParticipantIdentityGet for Server { // Obtain the identity object let document_id = id.document_id(); let Some(identity) = identities.identities.iter().find(|i| i.id == document_id) else { - response.not_found.push(id); + response.push_not_found(id); continue; }; diff --git a/crates/jmap/src/participant_identity/set.rs b/crates/jmap/src/participant_identity/set.rs index 9dc3596c..5034be6d 100644 --- a/crates/jmap/src/participant_identity/set.rs +++ b/crates/jmap/src/participant_identity/set.rs @@ -11,7 +11,7 @@ use jmap_proto::{ error::set::{SetError, SetErrorType}, method::set::{SetRequest, SetResponse}, object::participant_identity::{self, ParticipantIdentityProperty, ParticipantIdentityValue}, - request::{IntoValid, reference::MaybeIdReference}, + request::{MaybeInvalid, reference::MaybeIdReference}, }; use jmap_tools::{Key, Value}; use registry::schema::prelude::StorageQuota; @@ -38,7 +38,7 @@ impl ParticipantIdentitySet for Server { ) -> trc::Result> { let account_id = request.account_id.document_id(); let mut response = SetResponse::from_request(&request, self.core.jmap.set_max_objects)?; - let will_destroy = request.unwrap_destroy().into_valid().collect::>(); + let will_destroy = response.collect_will_destroy(request.unwrap_destroy()); let (identity_archive, mut identities) = match self.participant_identity_get_or_create(account_id).await? { Some(archive) => { @@ -127,7 +127,14 @@ impl ParticipantIdentitySet for Server { } // Process updates - 'update: for (id, object) in request.unwrap_update().into_valid() { + 'update: for (id, object) in request.unwrap_update() { + let id = match id { + MaybeInvalid::Value(id) => id, + invalid => { + response.not_updated.append(invalid, SetError::not_found()); + continue 'update; + } + }; // Make sure id won't be destroyed if will_destroy.contains(&id) { response.not_updated.append(id, SetError::will_destroy()); diff --git a/crates/jmap/src/principal/get.rs b/crates/jmap/src/principal/get.rs index 685e8346..0071e3da 100644 --- a/crates/jmap/src/principal/get.rs +++ b/crates/jmap/src/principal/get.rs @@ -39,7 +39,7 @@ impl PrincipalGet for Server { .details("The administrator has disabled directory queries.".to_string())); } - let ids = request.unwrap_ids(self.core.jmap.get_max_objects)?; + let (ids, not_found_ids) = request.unwrap_ids(self.core.jmap.get_max_objects)?; let properties = request.unwrap_properties(&[ PrincipalProperty::Id, PrincipalProperty::Type, @@ -70,14 +70,14 @@ impl PrincipalGet for Server { account_id: request.account_id.into(), state: State::Initial.into(), list: Vec::with_capacity(ids.len()), - not_found: vec![], + not_found: not_found_ids, }; for id in ids { // Obtain the principal let document_id = id.document_id(); if !principal_ids.contains(document_id) { - response.not_found.push(id); + response.push_not_found(id); continue; }; let principal = self diff --git a/crates/jmap/src/push/get.rs b/crates/jmap/src/push/get.rs index fb8a3e8f..688642ef 100644 --- a/crates/jmap/src/push/get.rs +++ b/crates/jmap/src/push/get.rs @@ -35,7 +35,7 @@ impl PushSubscriptionFetch for Server { mut request: GetRequest, access_token: &AccessToken, ) -> trc::Result> { - let ids = request.unwrap_ids(self.core.jmap.get_max_objects)?; + let (ids, not_found_ids) = request.unwrap_ids(self.core.jmap.get_max_objects)?; let properties = request.unwrap_properties(&[ PushSubscriptionProperty::Id, PushSubscriptionProperty::DeviceClientId, @@ -50,7 +50,7 @@ impl PushSubscriptionFetch for Server { account_id: request.account_id.into(), state: None, list: Vec::new(), - not_found: vec![], + not_found: not_found_ids, }; let Some(subscriptions_) = self @@ -64,7 +64,7 @@ impl PushSubscriptionFetch for Server { .await? else { for id in ids.unwrap_or_default() { - response.not_found.push(id); + response.push_not_found(id); } return Ok(response); }; @@ -93,7 +93,7 @@ impl PushSubscriptionFetch for Server { .iter() .find(|p| p.id.to_native() == document_id) else { - response.not_found.push(id); + response.push_not_found(id); continue; }; diff --git a/crates/jmap/src/push/set.rs b/crates/jmap/src/push/set.rs index 8027e08b..5153dd19 100644 --- a/crates/jmap/src/push/set.rs +++ b/crates/jmap/src/push/set.rs @@ -12,7 +12,7 @@ use jmap_proto::{ method::set::{SetRequest, SetResponse}, object::push_subscription::{self, PushSubscriptionProperty, PushSubscriptionValue}, references::resolve::ResolveCreatedReference, - request::IntoValid, + request::MaybeInvalid, types::date::UTCDate, }; use jmap_tools::{Key, Map, Value}; @@ -76,7 +76,7 @@ impl PushSubscriptionSet for Server { // Prepare response let mut response = SetResponse::from_request(&request, self.core.jmap.set_max_objects)?; - let will_destroy = request.unwrap_destroy().into_valid().collect::>(); + let will_destroy = response.collect_will_destroy(request.unwrap_destroy()); let account = self.account(account_id).await.caused_by(trc::location!())?; // Process creates @@ -154,7 +154,14 @@ impl PushSubscriptionSet for Server { } // Process updates - 'update: for (id, object) in request.unwrap_update().into_valid() { + 'update: for (id, object) in request.unwrap_update() { + let id = match id { + MaybeInvalid::Value(id) => id, + invalid => { + response.not_updated.append(invalid, SetError::not_found()); + continue 'update; + } + }; // Make sure id won't be destroyed if will_destroy.contains(&id) { response.not_updated.append(id, SetError::will_destroy()); diff --git a/crates/jmap/src/quota/get.rs b/crates/jmap/src/quota/get.rs index d4a1f89f..b06875cd 100644 --- a/crates/jmap/src/quota/get.rs +++ b/crates/jmap/src/quota/get.rs @@ -29,7 +29,7 @@ impl QuotaGet for Server { mut request: GetRequest, access_token: &AccessToken, ) -> trc::Result> { - let ids = request.unwrap_ids(self.core.jmap.get_max_objects)?; + let (ids, not_found_ids) = request.unwrap_ids(self.core.jmap.get_max_objects)?; let properties = request.unwrap_properties(&[ QuotaProperty::Id, QuotaProperty::ResourceType, @@ -58,7 +58,7 @@ impl QuotaGet for Server { account_id: request.account_id.into(), state: State::Initial.into(), list: Vec::with_capacity(ids.len()), - not_found: vec![], + not_found: not_found_ids, }; let account = if account_id == access_token.account_id() { @@ -71,7 +71,7 @@ impl QuotaGet for Server { // Obtain the sieve script object let document_id = id.document_id(); if !quota_ids.contains(&document_id) { - response.not_found.push(id); + response.push_not_found(id); continue; } diff --git a/crates/jmap/src/registry/get.rs b/crates/jmap/src/registry/get.rs index 2860f0a9..ecd25a57 100644 --- a/crates/jmap/src/registry/get.rs +++ b/crates/jmap/src/registry/get.rs @@ -65,12 +65,13 @@ impl RegistryGet for Server { (object_flags & OBJ_FILTER_TENANT) != 0 && access_token.tenant_id().is_some(); let is_account_filtered = (object_flags & OBJ_FILTER_ACCOUNT) != 0 && !access_token.has_permission(Permission::Impersonate); + let (ids, not_found_ids) = request.unwrap_ids(self.core.jmap.get_max_objects)?; let mut get = RegistryGetResponse { access_token, server: self, account_id: request.account_id.document_id(), object_type, - ids: request.unwrap_ids(self.core.jmap.get_max_objects)?, + ids, properties: request .properties .take() @@ -83,7 +84,7 @@ impl RegistryGet for Server { account_id: request.account_id.into(), state: None, list: vec![], - not_found: vec![], + not_found: not_found_ids, }, object_flags, is_tenant_filtered, @@ -233,7 +234,7 @@ impl RegistryGet for Server { continue; }; - let mut extra_properties = VecMap::new(); + let mut extra_properties: VecMap = VecMap::new(); match &object.inner { ObjectInner::DkimSignature(obj) if get.properties.is_empty() @@ -384,11 +385,13 @@ impl RegistryGetResponse<'_> { } pub fn not_found(&mut self, id: Id) { - self.response.not_found.push(id); + self.response.push_not_found(id); } pub fn not_found_any(mut self) -> Self { - self.response.not_found = self.ids.take().unwrap_or_default(); + for id in self.ids.take().unwrap_or_default() { + self.response.push_not_found(id); + } self } diff --git a/crates/jmap/src/registry/mapping/account.rs b/crates/jmap/src/registry/mapping/account.rs index ca8da8c6..ecd4139d 100644 --- a/crates/jmap/src/registry/mapping/account.rs +++ b/crates/jmap/src/registry/mapping/account.rs @@ -25,7 +25,7 @@ use common::{ ipc::CacheInvalidation, }; use directory::core::secret::{SecretVerificationResult, hash_secret, verify_mfa_secret_hash}; -use jmap_proto::{error::set::SetError, types::state::State}; +use jmap_proto::{error::set::SetError, request::MaybeInvalid, types::state::State}; use jmap_tools::{JsonPointer, JsonPointerItem, Key, Map, Value}; use registry::{ jmap::{IntoValue, JsonPointerPatch, MaybeUnpatched, RegistryJsonPatch, RegistryValue}, @@ -646,13 +646,13 @@ pub(crate) async fn account_set( .response .updated .into_keys() - .map(|id| (id, err.clone())) + .map(|id| (MaybeInvalid::Value(id), err.clone())) .collect::>(); let failed_delete = set .response .destroyed .into_iter() - .map(|id| (id, err.clone())) + .map(|id| (MaybeInvalid::Value(id), err.clone())) .collect::>(); set.response.not_created.extend(failed_create); @@ -706,7 +706,7 @@ pub(crate) async fn account_get( } } - get.response.not_found.extend(ids); + get.response.not_found.extend(ids.map(MaybeInvalid::Value)); } ObjectType::AccountPassword => { let mut ids = get @@ -747,7 +747,7 @@ pub(crate) async fn account_get( } } - get.response.not_found.extend(ids); + get.response.not_found.extend(ids.map(MaybeInvalid::Value)); } ObjectType::ApiKey | ObjectType::AppPassword => { let mut ids = if let Some(ids) = get.ids.take() { diff --git a/crates/jmap/src/registry/mapping/bootstrap.rs b/crates/jmap/src/registry/mapping/bootstrap.rs index 06f7ff50..351f81c9 100644 --- a/crates/jmap/src/registry/mapping/bootstrap.rs +++ b/crates/jmap/src/registry/mapping/bootstrap.rs @@ -13,7 +13,10 @@ use common::{ network::acme::account::acme_create_account, psl, }; use directory::core::secret::hash_secret; -use jmap_proto::error::set::{SetError, SetErrorType}; +use jmap_proto::{ + error::set::{SetError, SetErrorType}, + request::MaybeInvalid, +}; use jmap_tools::{JsonPointer, JsonPointerItem, Key}; use rand::{Rng, distr::Alphanumeric, rng}; use registry::{ @@ -67,7 +70,7 @@ pub(crate) async fn bootstrap_get( } } - get.response.not_found.extend(ids); + get.response.not_found.extend(ids.map(MaybeInvalid::Value)); Ok(get) } diff --git a/crates/jmap/src/registry/set.rs b/crates/jmap/src/registry/set.rs index 7bdf46ae..46399da0 100644 --- a/crates/jmap/src/registry/set.rs +++ b/crates/jmap/src/registry/set.rs @@ -38,7 +38,7 @@ use jmap_proto::{ method::set::{SetRequest, SetResponse}, object::registry::Registry, references::resolve::ResolveCreatedReference, - request::IntoValid, + request::{IntoValid, MaybeInvalid}, }; use jmap_tools::{JsonPointer, JsonPointerItem, Key}; use registry::{ @@ -125,9 +125,11 @@ impl RegistrySet for Server { // Initial destroy validation for singletons let mut destroy = request.unwrap_destroy().into_valid().collect::>(); if is_singleton && !destroy.is_empty() { - response - .not_destroyed - .extend(destroy.drain(..).map(|id| (id, SetError::singleton()))); + response.not_destroyed.extend( + destroy + .drain(..) + .map(|id| (MaybeInvalid::Value(id), SetError::singleton())), + ); } // Update validation for willDestroy diff --git a/crates/jmap/src/share_notification/get.rs b/crates/jmap/src/share_notification/get.rs index 076ec039..a9da1f24 100644 --- a/crates/jmap/src/share_notification/get.rs +++ b/crates/jmap/src/share_notification/get.rs @@ -172,9 +172,9 @@ impl ShareNotificationGet for Server { response.state = Some(State::Initial); } - response - .not_found - .extend(ids.into_iter().map(Id::from).collect::>()); + for id in ids { + response.push_not_found(Id::from(id)); + } Ok(response) } diff --git a/crates/jmap/src/sieve/get.rs b/crates/jmap/src/sieve/get.rs index bb5d524a..7abda923 100644 --- a/crates/jmap/src/sieve/get.rs +++ b/crates/jmap/src/sieve/get.rs @@ -36,7 +36,7 @@ impl SieveScriptGet for Server { &self, mut request: GetRequest, ) -> trc::Result> { - let ids = request.unwrap_ids(self.core.jmap.get_max_objects)?; + let (ids, not_found_ids) = request.unwrap_ids(self.core.jmap.get_max_objects)?; let properties = request.unwrap_properties(&[ SieveProperty::Id, SieveProperty::Name, @@ -63,7 +63,7 @@ impl SieveScriptGet for Server { .await? .into(), list: Vec::with_capacity(ids.len()), - not_found: vec![], + not_found: not_found_ids, }; let active_script_id = self.sieve_script_get_active_id(account_id).await?; @@ -71,7 +71,7 @@ impl SieveScriptGet for Server { // Obtain the sieve script object let document_id = id.document_id(); if !script_ids.contains(document_id) { - response.not_found.push(id); + response.push_not_found(id); continue; } let sieve_ = if let Some(sieve) = self @@ -85,7 +85,7 @@ impl SieveScriptGet for Server { { sieve } else { - response.not_found.push(id); + response.push_not_found(id); continue; }; let sieve = sieve_ diff --git a/crates/jmap/src/sieve/set.rs b/crates/jmap/src/sieve/set.rs index e8cc2d5a..3132eb2e 100644 --- a/crates/jmap/src/sieve/set.rs +++ b/crates/jmap/src/sieve/set.rs @@ -19,7 +19,7 @@ use jmap_proto::{ method::set::{SetRequest, SetResponse}, object::sieve::{Sieve, SieveProperty, SieveValue}, references::resolve::ResolveCreatedReference, - request::{IntoValid, reference::MaybeIdReference}, + request::{MaybeInvalid, reference::MaybeIdReference}, types::state::State, }; use jmap_tools::{Key, Map, Value}; @@ -91,7 +91,7 @@ impl SieveScriptSet for Server { .await?, ), }; - let will_destroy = request.unwrap_destroy().into_valid().collect::>(); + let will_destroy = ctx.response.collect_will_destroy(request.unwrap_destroy()); // Validate active script id if let Some(MaybeIdReference::Id(id)) = &request.arguments.on_success_activate_script @@ -197,7 +197,16 @@ impl SieveScriptSet for Server { } // Process updates - 'update: for (id, object) in request.unwrap_update().into_valid() { + 'update: for (id, object) in request.unwrap_update() { + let id = match id { + MaybeInvalid::Value(id) => id, + invalid => { + ctx.response + .not_updated + .append(invalid, SetError::not_found()); + continue 'update; + } + }; // Make sure id won't be destroyed if will_destroy.contains(&id) { ctx.response diff --git a/crates/jmap/src/submission/get.rs b/crates/jmap/src/submission/get.rs index 5153783c..90a538bd 100644 --- a/crates/jmap/src/submission/get.rs +++ b/crates/jmap/src/submission/get.rs @@ -46,7 +46,7 @@ impl EmailSubmissionGet for Server { &self, mut request: GetRequest, ) -> trc::Result> { - let ids = request.unwrap_ids(self.core.jmap.get_max_objects)?; + let (ids, not_found_ids) = request.unwrap_ids(self.core.jmap.get_max_objects)?; let properties = request.unwrap_properties(&[ EmailSubmissionProperty::Id, EmailSubmissionProperty::EmailId, @@ -107,7 +107,7 @@ impl EmailSubmissionGet for Server { .await? .into(), list: Vec::with_capacity(ids.len()), - not_found: vec![], + not_found: not_found_ids, }; for id in ids { @@ -124,7 +124,7 @@ impl EmailSubmissionGet for Server { { submission } else { - response.not_found.push(id); + response.push_not_found(id); continue; }; let submission = submission_ diff --git a/crates/jmap/src/submission/set.rs b/crates/jmap/src/submission/set.rs index eab2fa18..99f099f7 100644 --- a/crates/jmap/src/submission/set.rs +++ b/crates/jmap/src/submission/set.rs @@ -21,7 +21,7 @@ use jmap_proto::{ object::email_submission::{self, EmailSubmissionProperty, EmailSubmissionValue}, references::resolve::ResolveCreatedReference, request::{ - Call, IntoValid, MaybeInvalid, RequestMethod, SetRequestMethod, + Call, MaybeInvalid, RequestMethod, SetRequestMethod, method::{MethodFunction, MethodName, MethodObject}, reference::{MaybeIdReference, MaybeResultReference}, }, @@ -71,7 +71,7 @@ impl EmailSubmissionSet for Server { ) -> trc::Result> { let account_id = request.account_id.document_id(); let mut response = SetResponse::from_request(&request, self.core.jmap.set_max_objects)?; - let will_destroy = request.unwrap_destroy().into_valid().collect::>(); + let will_destroy = response.collect_will_destroy(request.unwrap_destroy()); // Process creates let mut success_email_ids = HashMap::new(); @@ -137,7 +137,16 @@ impl EmailSubmissionSet for Server { } // Process updates - 'update: for (id, object) in request.unwrap_update().into_valid() { + 'update: for (id, object) in request.unwrap_update() { + let id = match id { + MaybeInvalid::Value(id) => id, + invalid => { + response + .not_updated + .append(invalid, SetError::not_found()); + continue 'update; + } + }; // Make sure id won't be destroyed if will_destroy.contains(&id) { response.not_updated.append(id, SetError::will_destroy()); diff --git a/crates/jmap/src/thread/get.rs b/crates/jmap/src/thread/get.rs index 75599c33..654b1b1a 100644 --- a/crates/jmap/src/thread/get.rs +++ b/crates/jmap/src/thread/get.rs @@ -63,7 +63,8 @@ impl ThreadGet for Server { all_ids.insert(item.document_id); } - let ids = if let Some(ids) = request.unwrap_ids(self.core.jmap.get_max_objects)? { + let (ids, not_found_ids) = request.unwrap_ids(self.core.jmap.get_max_objects)?; + let ids = if let Some(ids) = ids { ids } else { thread_map @@ -84,7 +85,7 @@ impl ThreadGet for Server { .await? .into(), list: Vec::with_capacity(ids.len()), - not_found: vec![], + not_found: not_found_ids, }; let ordered_ids = if add_email_ids && !all_ids.is_empty() { @@ -125,7 +126,7 @@ impl ThreadGet for Server { } response.list.push(thread.into()); } else { - response.not_found.push(id); + response.push_not_found(id); } } diff --git a/crates/jmap/src/vacation/get.rs b/crates/jmap/src/vacation/get.rs index 45140103..ab2b8ef0 100644 --- a/crates/jmap/src/vacation/get.rs +++ b/crates/jmap/src/vacation/get.rs @@ -12,7 +12,10 @@ use jmap_proto::{ object::vacation_response::{ VacationResponse, VacationResponseProperty, VacationResponseValue, }, - request::reference::MaybeResultReference, + request::{ + MaybeInvalid, + reference::{MaybeIdReference, MaybeResultReference}, + }, types::date::UTCDate, }; use jmap_tools::{Map, Value}; @@ -68,14 +71,16 @@ impl VacationResponseGet for Server { let do_get = if let Some(MaybeResultReference::Value(ids)) = request.ids { let mut do_get = false; for id in ids { - match id.try_unwrap() { - Some(id) if id.is_singleton() => { + match id { + MaybeIdReference::Id(id) if id.is_singleton() => { do_get = true; } - Some(id) => { - response.not_found.push(id); + MaybeIdReference::Id(id) => { + response.push_not_found(id); + } + MaybeIdReference::Invalid(s) | MaybeIdReference::Reference(s) => { + response.not_found.push(MaybeInvalid::Invalid(s)); } - _ => {} } } do_get diff --git a/crates/utils/src/map/vec_map.rs b/crates/utils/src/map/vec_map.rs index 3b5c7845..67b42b43 100644 --- a/crates/utils/src/map/vec_map.rs +++ b/crates/utils/src/map/vec_map.rs @@ -41,7 +41,8 @@ impl VecMap { } #[inline(always)] - pub fn set(&mut self, key: K, value: V) -> bool { + pub fn set(&mut self, key: impl Into, value: V) -> bool { + let key = key.into(); if let Some(kv) = self.inner.iter_mut().find(|kv| kv.key == key) { kv.value = value; false @@ -52,19 +53,28 @@ impl VecMap { } #[inline(always)] - pub fn append(&mut self, key: K, value: V) { - self.inner.push(KeyValue { key, value }); + pub fn append(&mut self, key: impl Into, value: V) { + self.inner.push(KeyValue { + key: key.into(), + value, + }); } #[inline(always)] - pub fn with_append(mut self, key: K, value: V) -> Self { + pub fn with_append(mut self, key: impl Into, value: V) -> Self { self.append(key, value); self } #[inline(always)] - pub fn insert(&mut self, idx: usize, key: K, value: V) { - self.inner.insert(idx, KeyValue { key, value }); + pub fn insert(&mut self, idx: usize, key: impl Into, value: V) { + self.inner.insert( + idx, + KeyValue { + key: key.into(), + value, + }, + ); } #[inline(always)]