Fix Email/query hasAttachment for binary attachments#2778
Open
onevcat wants to merge 1 commit intostalwartlabs:mainfrom
Open
Fix Email/query hasAttachment for binary attachments#2778onevcat wants to merge 1 commit intostalwartlabs:mainfrom
onevcat wants to merge 1 commit intostalwartlabs:mainfrom
Conversation
71ac33c to
313e6e4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Email/getreportshasAttachment: truefor messages that include attachments, butEmail/querywith filter{hasAttachment: true}can return an empty result set for the same messages.In practice this shows up with messages that have binary attachments (e.g.
application/octet-stream) where Stalwart does not extract/index attachment text.Investigation / root cause
During investigation (triggered from xin's JMAP integration tests), we observed:
Email/getfor a message with an attachment returnshasAttachment: trueandattachmentsCount > 0.Email/querywithfilter: {hasAttachment: true}returnsids: [].The root cause is that
HasAttachmentwas indexed based on whether the full-textAttachmentsearch field exists:EmailSearchField::Attachmentis only present when attachment text is indexed.Meanwhile
Email/getderiveshasAttachmentfrom the message metadata bit (MESSAGE_HAS_ATTACHMENT, stored inrcvd_attach).Fix
Index
EmailSearchField::HasAttachmentfrom the message metadata bit (MESSAGE_HAS_ATTACHMENT) soEmail/querymatchesEmail/getsemantics.Tests
Add a regression test that imports an email with a binary attachment (
application/octet-stream) and asserts that it is returned byEmail/querywithhasAttachment: true.Local testing
(Full
-p testsrequires thefdb_clibrary on my local machine; CI should run the full suite.)