thunderbird / thunderbird/thunderbird-android

non uri addressable attachments

Open
#1,560 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

type: architecture
Dominant language
Kotlin
Stars
14k
Forks
2.8k
Avg merge
3d 3h
Merged PRs (30d)
57

Description

We still have a bit of a situation with our handling of attachments. Here is a rough overview of our parsing control flow, to get on the same page:

  1. data is loaded from the database into one LocalMessage and several LocalBodyParts
  2. encrypted message parts are decrypted, recursively parsing the mime structure of the decrypted data. for all further processing, the decrypted MimeBodyParts are substituted for their encrypted LocalParts
  3. MessageExtractor.findViewablesAndAttachments subdivides all parts of a message into those parts which are displayed inline as part of the message text, and those which are displayed as attachments
    4a) Inline parts are parsed into text, and concatenated into MessageViewInfo.text
    4b) Attachment parts are parsed into AttachmentViewInfo objects, which hold info on the attachments including a content-URI (which may be handed to external apps to access the part data)

The problematic bit is that for step 4b), all parts to be displayed as attachments need to be addressable by a URI. This is generally fine for parts loaded from the database. For the decrypted parts from step 2), all leaf parts have a body of class DeferredFileBody, for which a URI can be obtained through DecryptedFileProvider.

A problem arises if MessageExtractor in step 3) decides that any decrypted, non-leaf part should be displayed as an attachment. At the moment, the only situation where this can happen is a message/rfc822 part with content-disposition: attachment.

Possible solutions:

  • Don't recursively parse message/rfc822 parts with content-disposition attachment in MimePartStreamParser, but treat them as a leaf body. This is a relatively simple solution to the problem, and has good performance because we don't parse a mime structure that is never used later on. However, it breaks separation of concerns, since the decision how a part should be displayed (inline/attachment) is supposed to be made in MessageExtractor.
  • In AttachmentInfoExtractor, write the MimeBodyPart back into a file that can be accessed through DecryptedFileProvider. Worse for performance since it means we needlessly parse the message structure just to write it down at a later point. This also breaks separation of concerns but in a different way, since it assigns responsibility to AttachmentInfoExtractor to not only find a URI to access a part, but to create it in the first place.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with MimePartStreamParser, MessageExtractor.findViewablesAndAttachments, AttachmentInfoExtractor, and DecryptedFileProvider to trace how decrypted message/rfc822 attachments are parsed and addressed. Compare the listed alternatives and define a resolution that gives every attachment a usable URI without assigning display decisions to the wrong component; no specific implementation or test is named.

Written by the indexing model from the issue text.

Assessment

Tech stack
android, kotlin
Domain
mobile-dev
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.