diff --git a/CHANGELOG.md b/CHANGELOG.md index 3121580c..648005f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ If you are upgrading from v0.16.x, replace the binary (or run `docker pull`). If - `*/set`: Unchanged immutable `id` property is rejected on update. - `*/query` and `*/queryChanges`: null` rejected as `notRequest`. - `Email/query`: + * Improper `anchor` handling. * Total miscount when `collapseThreads` is enabled. * Wrong sort order on `hasKeyword`, `allInThreadHaveKeyword`, and `someInThreadHaveKeyword` conditions. * Non-standard header values are not searchable. diff --git a/crates/jmap-proto/src/method/query.rs b/crates/jmap-proto/src/method/query.rs index 14516f80..50309543 100644 --- a/crates/jmap-proto/src/method/query.rs +++ b/crates/jmap-proto/src/method/query.rs @@ -102,7 +102,9 @@ impl<'de, T: JmapObject> DeserializeArguments<'de> for QueryRequest { self.position = map.next_value()?; }, b"anchor" => { - self.anchor = map.next_value()?; + self.anchor = map + .next_value::>>()? + .map(|anchor| anchor.try_unwrap().unwrap_or(Id::from(u64::MAX))); }, b"anchorOffset" => { self.anchor_offset = map.next_value()?; diff --git a/crates/jmap-proto/src/request/parser.rs b/crates/jmap-proto/src/request/parser.rs index 7f09e727..295ce8fd 100644 --- a/crates/jmap-proto/src/request/parser.rs +++ b/crates/jmap-proto/src/request/parser.rs @@ -10,8 +10,7 @@ use super::{ }; use crate::request::{ CopyRequestMethod, GetRequestMethod, ParseRequestMethod, QueryChangesRequestMethod, - QueryRequestMethod, SetRequestMethod, - deserialize::DeserializeArguments, + QueryRequestMethod, SetRequestMethod, deserialize::DeserializeArguments, }; use serde::{ Deserialize, Deserializer, diff --git a/crates/jmap/src/api/query.rs b/crates/jmap/src/api/query.rs index 961ab895..212e93a5 100644 --- a/crates/jmap/src/api/query.rs +++ b/crates/jmap/src/api/query.rs @@ -19,6 +19,7 @@ pub struct QueryResponseBuilder { anchor_offset: i32, pub has_anchor: bool, pub anchor_found: bool, + index: i32, pub response: QueryResponse, } @@ -49,6 +50,7 @@ impl QueryResponseBuilder { anchor: request.anchor.map(|anchor| anchor.id()).unwrap_or(0), anchor_offset: request.anchor_offset.unwrap_or(0), anchor_found: false, + index: 0, response: QueryResponse { account_id: request.account_id, query_state, @@ -91,30 +93,31 @@ impl QueryResponseBuilder { } else { self.response.ids.push(id); } - } else if self.anchor_offset >= 0 { - if !self.anchor_found { - if id_u64 != self.anchor { - return true; - } + } else { + let current_index = self.index; + self.index += 1; + + if id_u64 == self.anchor { self.anchor_found = true; + self.position = (current_index + self.anchor_offset).max(0); } - if self.anchor_offset > 0 { - self.anchor_offset -= 1; + if self.anchor_offset >= 0 { + if self.anchor_found && current_index >= self.position { + self.response.ids.push(id); + if self.limit > 0 && self.response.ids.len() == self.limit { + return false; + } + } } else { self.response.ids.push(id); - if self.response.ids.len() == self.limit { + if self.anchor_found + && self.limit > 0 + && self.response.ids.len() >= self.position as usize + self.limit + { return false; } } - } else { - self.anchor_found = id_u64 == self.anchor; - self.response.ids.push(id); - - if self.anchor_found { - self.position = self.anchor_offset; - return false; - } } true @@ -125,35 +128,49 @@ impl QueryResponseBuilder { } pub fn build(mut self) -> trc::Result { - if !self.has_anchor || self.anchor_found { - if !self.has_anchor && self.requested_position >= 0 { - self.response.position = if self.position == 0 { - self.requested_position - } else { - 0 - }; - } else if self.position >= 0 { - self.response.position = self.position; - } else { - let position = self.position.unsigned_abs() as usize; - let start_offset = if position < self.response.ids.len() { - self.response.ids.len() - position - } else { - 0 - }; - self.response.position = start_offset as i32; - let end_offset = if self.limit > 0 { - std::cmp::min(start_offset + self.limit, self.response.ids.len()) + if self.has_anchor { + if !self.anchor_found { + return Err(trc::JmapEvent::AnchorNotFound.into_err()); + } + + let start = self.position.max(0) as usize; + if self.anchor_offset < 0 { + let start = start.min(self.response.ids.len()); + let end = if self.limit > 0 { + std::cmp::min(start + self.limit, self.response.ids.len()) } else { self.response.ids.len() }; - - self.response.ids = self.response.ids[start_offset..end_offset].to_vec() + self.response.ids = self.response.ids[start..end].to_vec(); } + self.response.position = start as i32; - Ok(self.response) - } else { - Err(trc::JmapEvent::AnchorNotFound.into_err()) + return Ok(self.response); } + + if self.requested_position >= 0 { + self.response.position = if self.position == 0 { + self.requested_position + } else { + 0 + }; + } else { + let position = self.position.unsigned_abs() as usize; + let start_offset = if position < self.response.ids.len() { + self.response.ids.len() - position + } else { + 0 + }; + self.response.position = start_offset as i32; + let end_offset = if self.limit > 0 { + std::cmp::min(start_offset + self.limit, self.response.ids.len()) + } else { + self.response.ids.len() + }; + + self.response.ids = self.response.ids[start_offset..end_offset].to_vec(); + } + + Ok(self.response) } } diff --git a/crates/jmap/src/changes/query.rs b/crates/jmap/src/changes/query.rs index 4cb95fd0..77e391f1 100644 --- a/crates/jmap/src/changes/query.rs +++ b/crates/jmap/src/changes/query.rs @@ -151,7 +151,11 @@ impl QueryChanges for Server { } QueryChangesRequestMethod::FileNode(mut request) => { // Query changes - resolve_account_id(&mut request.account_id, MethodObject::FileNode, access_token)?; + resolve_account_id( + &mut request.account_id, + MethodObject::FileNode, + access_token, + )?; changes = self .changes( build_changes_request(&request), diff --git a/crates/store/src/search/local.rs b/crates/store/src/search/local.rs index 944052a2..ab312a07 100644 --- a/crates/store/src/search/local.rs +++ b/crates/store/src/search/local.rs @@ -199,11 +199,9 @@ impl QueryResults { results.sort_by(|a, b| { for comparator in &comparators { let (a, b, is_ascending) = match comparator { - SearchComparator::DocumentSet { set, ascending } => ( - set.contains(*a) as u32, - set.contains(*b) as u32, - *ascending, - ), + SearchComparator::DocumentSet { set, ascending } => { + (set.contains(*a) as u32, set.contains(*b) as u32, *ascending) + } SearchComparator::SortedSet { set, ascending } => { let missing = if *ascending { u32::MAX } else { 0 }; (