Fix JMAP: Email/query: Improper anchor handling
This commit is contained in:
@@ -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.
|
- `*/set`: Unchanged immutable `id` property is rejected on update.
|
||||||
- `*/query` and `*/queryChanges`: null` rejected as `notRequest`.
|
- `*/query` and `*/queryChanges`: null` rejected as `notRequest`.
|
||||||
- `Email/query`:
|
- `Email/query`:
|
||||||
|
* Improper `anchor` handling.
|
||||||
* Total miscount when `collapseThreads` is enabled.
|
* Total miscount when `collapseThreads` is enabled.
|
||||||
* Wrong sort order on `hasKeyword`, `allInThreadHaveKeyword`, and `someInThreadHaveKeyword` conditions.
|
* Wrong sort order on `hasKeyword`, `allInThreadHaveKeyword`, and `someInThreadHaveKeyword` conditions.
|
||||||
* Non-standard header values are not searchable.
|
* Non-standard header values are not searchable.
|
||||||
|
|||||||
@@ -102,7 +102,9 @@ impl<'de, T: JmapObject> DeserializeArguments<'de> for QueryRequest<T> {
|
|||||||
self.position = map.next_value()?;
|
self.position = map.next_value()?;
|
||||||
},
|
},
|
||||||
b"anchor" => {
|
b"anchor" => {
|
||||||
self.anchor = map.next_value()?;
|
self.anchor = map
|
||||||
|
.next_value::<Option<crate::request::MaybeInvalid<Id>>>()?
|
||||||
|
.map(|anchor| anchor.try_unwrap().unwrap_or(Id::from(u64::MAX)));
|
||||||
},
|
},
|
||||||
b"anchorOffset" => {
|
b"anchorOffset" => {
|
||||||
self.anchor_offset = map.next_value()?;
|
self.anchor_offset = map.next_value()?;
|
||||||
|
|||||||
@@ -10,8 +10,7 @@ use super::{
|
|||||||
};
|
};
|
||||||
use crate::request::{
|
use crate::request::{
|
||||||
CopyRequestMethod, GetRequestMethod, ParseRequestMethod, QueryChangesRequestMethod,
|
CopyRequestMethod, GetRequestMethod, ParseRequestMethod, QueryChangesRequestMethod,
|
||||||
QueryRequestMethod, SetRequestMethod,
|
QueryRequestMethod, SetRequestMethod, deserialize::DeserializeArguments,
|
||||||
deserialize::DeserializeArguments,
|
|
||||||
};
|
};
|
||||||
use serde::{
|
use serde::{
|
||||||
Deserialize, Deserializer,
|
Deserialize, Deserializer,
|
||||||
|
|||||||
@@ -19,6 +19,7 @@ pub struct QueryResponseBuilder {
|
|||||||
anchor_offset: i32,
|
anchor_offset: i32,
|
||||||
pub has_anchor: bool,
|
pub has_anchor: bool,
|
||||||
pub anchor_found: bool,
|
pub anchor_found: bool,
|
||||||
|
index: i32,
|
||||||
|
|
||||||
pub response: QueryResponse,
|
pub response: QueryResponse,
|
||||||
}
|
}
|
||||||
@@ -49,6 +50,7 @@ impl QueryResponseBuilder {
|
|||||||
anchor: request.anchor.map(|anchor| anchor.id()).unwrap_or(0),
|
anchor: request.anchor.map(|anchor| anchor.id()).unwrap_or(0),
|
||||||
anchor_offset: request.anchor_offset.unwrap_or(0),
|
anchor_offset: request.anchor_offset.unwrap_or(0),
|
||||||
anchor_found: false,
|
anchor_found: false,
|
||||||
|
index: 0,
|
||||||
response: QueryResponse {
|
response: QueryResponse {
|
||||||
account_id: request.account_id,
|
account_id: request.account_id,
|
||||||
query_state,
|
query_state,
|
||||||
@@ -91,30 +93,31 @@ impl QueryResponseBuilder {
|
|||||||
} else {
|
} else {
|
||||||
self.response.ids.push(id);
|
self.response.ids.push(id);
|
||||||
}
|
}
|
||||||
} else if self.anchor_offset >= 0 {
|
} else {
|
||||||
if !self.anchor_found {
|
let current_index = self.index;
|
||||||
if id_u64 != self.anchor {
|
self.index += 1;
|
||||||
return true;
|
|
||||||
}
|
if id_u64 == self.anchor {
|
||||||
self.anchor_found = true;
|
self.anchor_found = true;
|
||||||
|
self.position = (current_index + self.anchor_offset).max(0);
|
||||||
}
|
}
|
||||||
|
|
||||||
if self.anchor_offset > 0 {
|
if self.anchor_offset >= 0 {
|
||||||
self.anchor_offset -= 1;
|
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 {
|
} else {
|
||||||
self.response.ids.push(id);
|
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;
|
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
|
true
|
||||||
@@ -125,35 +128,49 @@ impl QueryResponseBuilder {
|
|||||||
}
|
}
|
||||||
|
|
||||||
pub fn build(mut self) -> trc::Result<QueryResponse> {
|
pub fn build(mut self) -> trc::Result<QueryResponse> {
|
||||||
if !self.has_anchor || self.anchor_found {
|
if self.has_anchor {
|
||||||
if !self.has_anchor && self.requested_position >= 0 {
|
if !self.anchor_found {
|
||||||
self.response.position = if self.position == 0 {
|
return Err(trc::JmapEvent::AnchorNotFound.into_err());
|
||||||
self.requested_position
|
}
|
||||||
} else {
|
|
||||||
0
|
let start = self.position.max(0) as usize;
|
||||||
};
|
if self.anchor_offset < 0 {
|
||||||
} else if self.position >= 0 {
|
let start = start.min(self.response.ids.len());
|
||||||
self.response.position = self.position;
|
let end = if self.limit > 0 {
|
||||||
} else {
|
std::cmp::min(start + self.limit, self.response.ids.len())
|
||||||
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 {
|
} else {
|
||||||
self.response.ids.len()
|
self.response.ids.len()
|
||||||
};
|
};
|
||||||
|
self.response.ids = self.response.ids[start..end].to_vec();
|
||||||
self.response.ids = self.response.ids[start_offset..end_offset].to_vec()
|
|
||||||
}
|
}
|
||||||
|
self.response.position = start as i32;
|
||||||
|
|
||||||
Ok(self.response)
|
return Ok(self.response);
|
||||||
} else {
|
|
||||||
Err(trc::JmapEvent::AnchorNotFound.into_err())
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
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)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -151,7 +151,11 @@ impl QueryChanges for Server {
|
|||||||
}
|
}
|
||||||
QueryChangesRequestMethod::FileNode(mut request) => {
|
QueryChangesRequestMethod::FileNode(mut request) => {
|
||||||
// Query changes
|
// 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 = self
|
||||||
.changes(
|
.changes(
|
||||||
build_changes_request(&request),
|
build_changes_request(&request),
|
||||||
|
|||||||
@@ -199,11 +199,9 @@ impl QueryResults {
|
|||||||
results.sort_by(|a, b| {
|
results.sort_by(|a, b| {
|
||||||
for comparator in &comparators {
|
for comparator in &comparators {
|
||||||
let (a, b, is_ascending) = match comparator {
|
let (a, b, is_ascending) = match comparator {
|
||||||
SearchComparator::DocumentSet { set, ascending } => (
|
SearchComparator::DocumentSet { set, ascending } => {
|
||||||
set.contains(*a) as u32,
|
(set.contains(*a) as u32, set.contains(*b) as u32, *ascending)
|
||||||
set.contains(*b) as u32,
|
}
|
||||||
*ascending,
|
|
||||||
),
|
|
||||||
SearchComparator::SortedSet { set, ascending } => {
|
SearchComparator::SortedSet { set, ascending } => {
|
||||||
let missing = if *ascending { u32::MAX } else { 0 };
|
let missing = if *ascending { u32::MAX } else { 0 };
|
||||||
(
|
(
|
||||||
|
|||||||
Reference in New Issue
Block a user