fix: keep attachment file extension when saving - #11268
Open
mvanhorn wants to merge 1 commit into
Open
Conversation
Saving an attachment launched ACTION_CREATE_DOCUMENT with the MIME type declared in the mail Content-Type header. When that type did not round-trip to the display name's extension, the Storage Access Framework appended the declared type's preferred extension, so appointment.ics declared as text/plain was written as appointment.ics.txt. Derive a MIME type that is consistent with the display name's extension (the same approach the VIEW flow already uses in ViewIntentFinder) via a new MimeTypeUtil.getMimeTypeForFilename helper and pass it to the create-document intent, so the provider keeps the filename unchanged. When the name has no extension, the declared type (or the default) is used and the provider may add a suitable extension as before. Fixes thunderbird#7968
2 tasks
Contributor
|
✅ Validation Passed: All report and feature-flag labels are correctly set. |
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
2 participants
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.
Contribution Summary
Linked Issue/Ticket: Closes #7968
RFC / Technical Design (if applicable): N/A (small bug fix)
Description
When saving a mail attachment to the filesystem, an extra file extension was appended to the file on disk:
appointment.icswas written asappointment.ics.txt(and a corroborating report showedsomething.pkpasswritten with a spurious trailing extension). The save dialog showed the correct name, but the saved file gained an extra extension.Root cause
MessageViewFragment.onSaveAttachmentlaunchesIntent.ACTION_CREATE_DOCUMENT(viaCreateDocumentResultContract) passing the attachment's declared MIME type, which comes from the mailContent-Typeheader (e.g.text/plain), together with the display name (appointment.ics). Android's Storage Access Framework (FileSystemProvider.splitFileName) checks whether the display name's extension round-trips to the supplied MIME type. Because.icsdoes not map back totext/plain, the provider appends the MIME type's preferred extension (text/plain->.txt), producingappointment.ics.txt.Fix
Pass a MIME type that is consistent with the display name's extension, so the SAF provider leaves the filename unchanged. This mirrors the pattern the VIEW flow already uses (
ViewIntentFinder.getBestViewIntentderivesMimeTypeUtil.getMimeTypeByExtension(displayName)).MimeTypeUtil.getMimeTypeForFilename(displayName, declaredMimeType)that returns the extension-derived MIME type when the display name has an extension, and otherwise falls back to the declared MIME type (orapplication/octet-streamwhen that isnull).onSaveAttachmentnow builds theCreateDocumentResultContract.InputwithmimeType = MimeTypeUtil.getMimeTypeForFilename(attachment.displayName, attachment.mimeType).Because the derived MIME type round-trips with the extension Android computes, the provider no longer appends an extra extension. The actual byte copy in
AttachmentController.writeAttachmentis unaffected, and the unrelated EML-export path (which passes its own explicitmessage/rfc822) is untouched.Notes on edge cases (all covered by the new unit test):
appointment.ics,boardingpass.pkpass): resolves to the extension-consistent type (text/calendar,application/vnd.apple.pkpass) so the filename is preserved.noname): falls back to the declared type, and toapplication/octet-streamwhen the declared type isnull— the provider may then add a suitable extension, which is the desired behaviour.mystery.unknownext): resolves toapplication/octet-stream. This is deliberate: SAF treatsoctet-streamas matching any unrecognised extension, so it keeps the filename as-is rather than appending a spurious extension. (Returning the declared type here would reintroduce the double-extension bug.)This also removes a latent
requireNotNull(attachment.mimeType)that could throw for an attachment with a null declared MIME type; the new helper handles that case gracefully.Screen Shots
No UI changes (the fix affects the filename passed to the create-document intent, not any visible screen).
AI Disclosure
Select one of the following (mandatory)
Contribution Checklist
MimeTypeUtilJava utility class alongside the closely relatedgetMimeTypeByExtensionit builds on, to keep the change minimal and consistent)./gradlew :legacy:core:spotlessCheck :legacy:ui:legacy:spotlessCheckpasses;./gradlew :legacy:core:detekt :legacy:ui:legacy:detektpasses)./gradlew :legacy:core:testDebugUnitTest :legacy:ui:legacy:testDebugUnitTestpasses: 751 and 245 tests respectively, 0 failures)MimeTypeUtilTestwith 6 cases covering known/unknown/missing extensions)Testing
Run locally with JDK 21 and Android SDK 36:
./gradlew :legacy:core:testDebugUnitTest --tests "com.fsck.k9.helper.MimeTypeUtilTest"— 6 passed, 0 failed./gradlew :legacy:core:testDebugUnitTest :legacy:ui:legacy:testDebugUnitTest— 996 passed (6 skipped, pre-existing), 0 failed./gradlew :legacy:core:spotlessCheck :legacy:ui:legacy:spotlessCheck— passed./gradlew :legacy:core:detekt :legacy:ui:legacy:detekt— passedconnectedAndroidTest(instrumented) was not run; there is no device/emulator in this environment. The SAF filename behaviour itself is provided by the Android platform and is exercised via the new unit test on the MIME-resolution helper that drives it.AI was used for assistance.