Fix hasAttachment query for binary attachments

Author: onevcat

crates/email/src/message/index/search.rs

@@ -8,7 +8,8 @@ use crate::message::{
     index::{MAX_MESSAGE_PARTS, extractors::VisitTextArchived},
     metadata::{
         ArchivedMessageMetadata, ArchivedMetadataHeaderName, ArchivedMetadataHeaderValue,
-        ArchivedMetadataPartType, DecodedPartContent, MESSAGE_RECEIVED_MASK, MetadataHeaderName,
+        ArchivedMetadataPartType, DecodedPartContent, MESSAGE_HAS_ATTACHMENT,
+        MESSAGE_RECEIVED_MASK, MetadataHeaderName,
     },
 };
 use mail_parser::{DateTime, decoders::html::html_to_text, parsers::fields::thread::thread_name};
@@ -339,10 +340,17 @@ impl ArchivedMessageMetadata {
         #[cfg(feature = "test_mode")]
         document.set_unknown_language(default_language);
 
-        let has_attachment =
-            document.has_field(&(SearchField::Email(EmailSearchField::Attachment)));
-
+        // `Email.hasAttachment` (RFC 8621) is defined as "message has one or more attachments".
+        //
+        // The indexing pipeline also builds a separate full-text `Attachment` field when it can
+        // extract text from attachments (e.g. text/plain attachments, message/rfc822, etc.).
+        // That *text index* is not equivalent to the existence of attachments.
+        //
+        // Use the metadata bit that is also used by Email/get, so Email/query hasAttachment filter
+        // matches Email/get `hasAttachment`.
+        let has_attachment = (self.rcvd_attach.to_native() & MESSAGE_HAS_ATTACHMENT) != 0;
         document.index_bool(EmailSearchField::HasAttachment, has_attachment);
+
         document
     }
 }

crates/jmap-proto/src/request/method.rs

@@ -122,7 +122,9 @@ impl MethodName {
             (MethodFunction::Query, MethodObject::Principal) => "Principal/query",
             (MethodFunction::Changes, MethodObject::Principal) => "Principal/changes",
             (MethodFunction::QueryChanges, MethodObject::Principal) => "Principal/queryChanges",
-            (MethodFunction::GetAvailability, MethodObject::Principal) => "Principal/getAvailability",
+            (MethodFunction::GetAvailability, MethodObject::Principal) => {
+                "Principal/getAvailability"
+            }
 
             (MethodFunction::Get, MethodObject::Quota) => "Quota/get",
             (MethodFunction::Changes, MethodObject::Quota) => "Quota/changes",
@@ -153,9 +155,13 @@ impl MethodName {
             (MethodFunction::Set, MethodObject::FileNode) => "FileNode/set",
 
             (MethodFunction::Get, MethodObject::ShareNotification) => "ShareNotification/get",
-            (MethodFunction::Changes, MethodObject::ShareNotification) => "ShareNotification/changes",
+            (MethodFunction::Changes, MethodObject::ShareNotification) => {
+                "ShareNotification/changes"
+            }
             (MethodFunction::Query, MethodObject::ShareNotification) => "ShareNotification/query",
-            (MethodFunction::QueryChanges, MethodObject::ShareNotification) => "ShareNotification/queryChanges",
+            (MethodFunction::QueryChanges, MethodObject::ShareNotification) => {
+                "ShareNotification/queryChanges"
+            }
             (MethodFunction::Set, MethodObject::ShareNotification) => "ShareNotification/set",
 
             (MethodFunction::Get, MethodObject::Calendar) => "Calendar/get",
@@ -165,19 +171,33 @@ impl MethodName {
             (MethodFunction::Get, MethodObject::CalendarEvent) => "CalendarEvent/get",
             (MethodFunction::Changes, MethodObject::CalendarEvent) => "CalendarEvent/changes",
             (MethodFunction::Query, MethodObject::CalendarEvent) => "CalendarEvent/query",
-            (MethodFunction::QueryChanges, MethodObject::CalendarEvent) => "CalendarEvent/queryChanges",
+            (MethodFunction::QueryChanges, MethodObject::CalendarEvent) => {
+                "CalendarEvent/queryChanges"
+            }
             (MethodFunction::Set, MethodObject::CalendarEvent) => "CalendarEvent/set",
             (MethodFunction::Copy, MethodObject::CalendarEvent) => "CalendarEvent/copy",
             (MethodFunction::Parse, MethodObject::CalendarEvent) => "CalendarEvent/parse",
 
-            (MethodFunction::Get, MethodObject::CalendarEventNotification) => "CalendarEventNotification/get",
-            (MethodFunction::Changes, MethodObject::CalendarEventNotification) => "CalendarEventNotification/changes",
-            (MethodFunction::Query, MethodObject::CalendarEventNotification) => "CalendarEventNotification/query",
-            (MethodFunction::QueryChanges, MethodObject::CalendarEventNotification) => "CalendarEventNotification/queryChanges",
-            (MethodFunction::Set, MethodObject::CalendarEventNotification) => "CalendarEventNotification/set",
+            (MethodFunction::Get, MethodObject::CalendarEventNotification) => {
+                "CalendarEventNotification/get"
+            }
+            (MethodFunction::Changes, MethodObject::CalendarEventNotification) => {
+                "CalendarEventNotification/changes"
+            }
+            (MethodFunction::Query, MethodObject::CalendarEventNotification) => {
+                "CalendarEventNotification/query"
+            }
+            (MethodFunction::QueryChanges, MethodObject::CalendarEventNotification) => {
+                "CalendarEventNotification/queryChanges"
+            }
+            (MethodFunction::Set, MethodObject::CalendarEventNotification) => {
+                "CalendarEventNotification/set"
+            }
 
             (MethodFunction::Get, MethodObject::ParticipantIdentity) => "ParticipantIdentity/get",
-            (MethodFunction::Changes, MethodObject::ParticipantIdentity) => "ParticipantIdentity/changes",
+            (MethodFunction::Changes, MethodObject::ParticipantIdentity) => {
+                "ParticipantIdentity/changes"
+            }
             (MethodFunction::Set, MethodObject::ParticipantIdentity) => "ParticipantIdentity/set",
 
             (MethodFunction::Echo, MethodObject::Core) => "Core/echo",
@@ -186,7 +206,7 @@ impl MethodName {
     }
 
     pub fn parse(s: &str) -> Option<Self> {
-       hashify::tiny_map!(s.as_bytes(), 
+        hashify::tiny_map!(s.as_bytes(),
             "PushSubscription/get" => (MethodObject::PushSubscription, MethodFunction::Get),
             "PushSubscription/set" => (MethodObject::PushSubscription, MethodFunction::Set),
 
@@ -295,7 +315,6 @@ impl MethodName {
 
         ).map(|(obj, fnc)| MethodName { obj, fnc })
     }
-
 }
 
 impl Display for MethodObject {
@@ -326,17 +345,15 @@ impl Display for MethodObject {
     }
 }
 
-
 impl<'de> serde::Deserialize<'de> for MethodName {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
         D: serde::Deserializer<'de>,
     {
         let value = <&str>::deserialize(deserializer)?;
 
-        MethodName::parse(value).ok_or_else(|| {
-            serde::de::Error::custom(format!("Invalid method name: {:?}", value))
-        })
+        MethodName::parse(value)
+            .ok_or_else(|| serde::de::Error::custom(format!("Invalid method name: {:?}", value)))
     }
 }
 

tests/src/jmap/mail/query.rs

@@ -88,8 +88,51 @@ pub async fn test(params: &mut JMAPTest, insert: bool) {
             MAX_THREADS
         );
 
-        // Wait for indexing to complete
+        // Regression test: Email/query filter {hasAttachment:true} should match emails with binary
+        // attachments (not just those with attachment text indexed).
+        let sent_at = now();
+        let binary_anchor = "stalwart-hasattachment-binary";
+        let binary_message = MessageBuilder::new()
+            .from(("Sender", "sender@domain.com"))
+            .to(("Recipient", "rcpt@domain.com"))
+            .subject("Binary attachment regression")
+            .date(Date::new(sent_at as i64 + 10_000))
+            .message_id(binary_anchor)
+            .text_body("binary attachment")
+            .attachment("application/octet-stream", "file.bin", &[0u8; 8][..])
+            .write_to_vec()
+            .unwrap();
+
+        client
+            .email_import(
+                binary_message,
+                [Id::new(2000u64).to_string()],
+                Vec::<String>::new().into(),
+                Some(sent_at as i64 + 10_000),
+            )
+            .await
+            .unwrap();
+
+        // Wait for indexing to complete (covers both the bulk import and the regression message).
         wait_for_index(&server).await;
+
+        let binary_email_id = get_anchor(client, binary_anchor)
+            .await
+            .expect("binary regression email should be findable by Message-Id");
+
+        let ids = client
+            .email_query(
+                email::query::Filter::has_attachment(true).into(),
+                None::<Vec<_>>,
+            )
+            .await
+            .unwrap()
+            .take_ids();
+
+        assert!(
+            ids.iter().any(|id| id.to_string() == binary_email_id),
+            "expected hasAttachment query to include imported binary attachment message"
+        );
     }
 
     let can_stem = !params.server.search_store().is_mysql();