From cd4ce9d08b66157ca822fc95ca202ea09a72f61f Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 24 Sep 2026 18:34:42 +0200 Subject: [PATCH] fix: upload files shared into the app with their real name and type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sharing a photo or document from another app into Bulwark failed every time: the upload task was handed the share's `content://` URI, but it can only stream from a file path, so it failed with "Directory for '/external/images/media/…' doesn't exist". The attachment was also named after the URI's last segment (a MediaStore row id such as "1000000027"), and SEND_MULTIPLE shares only carried a wildcard type like `image/*`. - `uploadBlob` copies `content://` URIs into the cache, uploads the copy and deletes it afterwards, also when the upload fails - `ShareIntentStore` asks the providing app for the display name, size and MIME type when the intent arrives, falling back to the URI segment plus an extension for the MIME type - the composer gets the real name, type and size, so the size limit check now applies to shared files too --- .../anonymous/bulwarkmobile/MainActivity.kt | 4 +- .../bulwarkmobile/ShareIntentStore.kt | 54 ++++++++++++++++++- src/api/__tests__/blob.test.ts | 39 ++++++++++++++ src/api/blob.ts | 44 ++++++++++++++- src/lib/__tests__/share-intent.test.ts | 53 ++++++++++++++++++ src/lib/share-intent.ts | 29 ++++++---- src/navigation/linking.ts | 5 +- 7 files changed, 211 insertions(+), 17 deletions(-) create mode 100644 src/lib/__tests__/share-intent.test.ts diff --git a/android/app/src/main/java/com/anonymous/bulwarkmobile/MainActivity.kt b/android/app/src/main/java/com/anonymous/bulwarkmobile/MainActivity.kt index 45922fd7..8f6f69fb 100644 --- a/android/app/src/main/java/com/anonymous/bulwarkmobile/MainActivity.kt +++ b/android/app/src/main/java/com/anonymous/bulwarkmobile/MainActivity.kt @@ -19,13 +19,13 @@ class MainActivity : ReactActivity() { setTheme(R.style.AppTheme); super.onCreate(null) NotificationTapStore.captureFromIntent(intent) - ShareIntentStore.captureFromIntent(intent) + ShareIntentStore.captureFromIntent(intent, contentResolver) } override fun onNewIntent(intent: Intent) { super.onNewIntent(intent) setIntent(intent) - ShareIntentStore.captureFromIntent(intent)?.let { share -> + ShareIntentStore.captureFromIntent(intent, contentResolver)?.let { share -> BulwarkFcmModule.emit("app:share", share.toMap()) } val payload = NotificationTapStore.captureFromIntent(intent) ?: return diff --git a/android/app/src/main/java/com/anonymous/bulwarkmobile/ShareIntentStore.kt b/android/app/src/main/java/com/anonymous/bulwarkmobile/ShareIntentStore.kt index ecdfa886..c5e5ed08 100644 --- a/android/app/src/main/java/com/anonymous/bulwarkmobile/ShareIntentStore.kt +++ b/android/app/src/main/java/com/anonymous/bulwarkmobile/ShareIntentStore.kt @@ -1,8 +1,11 @@ package com.anonymous.bulwarkmobile +import android.content.ContentResolver import android.content.Intent import android.net.Uri import android.os.Build +import android.provider.OpenableColumns +import android.webkit.MimeTypeMap import com.facebook.react.bridge.Arguments import com.facebook.react.bridge.WritableMap @@ -20,7 +23,7 @@ object ShareIntentStore { return value } - fun captureFromIntent(intent: Intent?): SharePayload? { + fun captureFromIntent(intent: Intent?, resolver: ContentResolver): SharePayload? { val action = intent?.action ?: return null if (action != Intent.ACTION_SEND && action != Intent.ACTION_SEND_MULTIPLE) return null @@ -35,11 +38,14 @@ object ShareIntentStore { val type = intent.type if (text.isNullOrBlank() && uris.isEmpty()) return null + val files = uris.mapIndexed { i, uri -> describe(resolver, uri, type, i) } val payload = SharePayload( text = text, subject = subject, uris = uris.map { it.toString() }, - mimeTypes = uris.map { type ?: "" }, + mimeTypes = files.map { it.mimeType }, + names = files.map { it.name }, + sizes = files.map { it.size }, ) pending = payload // Clear so a later lifecycle event doesn't replay this share. @@ -50,6 +56,46 @@ object ShareIntentStore { return payload } + private class SharedFile(val name: String, val mimeType: String, val size: Double) + + // Ask the provider for the file's real name, type and size: a MediaStore + // URI ends in a bare row id ("1000000027") and SEND_MULTIPLE only carries + // a wildcard type such as "image/*". Size is -1 when unknown. + private fun describe(resolver: ContentResolver, uri: Uri, intentType: String?, index: Int): SharedFile { + var displayName: String? = null + var size = -1.0 + try { + resolver.query(uri, arrayOf(OpenableColumns.DISPLAY_NAME, OpenableColumns.SIZE), null, null, null)?.use { cursor -> + if (cursor.moveToFirst()) { + val nameColumn = cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME) + if (nameColumn >= 0 && !cursor.isNull(nameColumn)) displayName = cursor.getString(nameColumn) + val sizeColumn = cursor.getColumnIndex(OpenableColumns.SIZE) + if (sizeColumn >= 0 && !cursor.isNull(sizeColumn)) size = cursor.getLong(sizeColumn).toDouble() + } + } + } catch (_: Exception) { + // Provider without OpenableColumns: fall back to the URI below. + } + val resolvedType = try { + resolver.getType(uri) + } catch (_: Exception) { + null + } + val mimeType = resolvedType ?: intentType?.takeUnless { it.contains('*') } ?: "" + val name = displayName?.takeIf { it.isNotBlank() } ?: fallbackName(uri, mimeType, index) + return SharedFile(name, mimeType, size) + } + + // Last path segment plus an extension for the MIME type, so the + // recipient's mail client can still open the attachment. + private fun fallbackName(uri: Uri, mimeType: String, index: Int): String { + val base = uri.lastPathSegment?.substringAfterLast('/')?.takeIf { it.isNotBlank() } + ?: "shared-${index + 1}" + if (base.contains('.')) return base + val extension = MimeTypeMap.getSingleton().getExtensionFromMimeType(mimeType) + return if (extension.isNullOrEmpty()) base else "$base.$extension" + } + @Suppress("DEPRECATION") private fun streamExtra(intent: Intent): Uri? = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { @@ -71,12 +117,16 @@ object ShareIntentStore { val subject: String?, val uris: List, val mimeTypes: List, + val names: List, + val sizes: List, ) { fun toMap(): WritableMap = Arguments.createMap().apply { if (text != null) putString("text", text) if (subject != null) putString("subject", subject) putArray("uris", Arguments.fromList(uris)) putArray("mimeTypes", Arguments.fromList(mimeTypes)) + putArray("names", Arguments.fromList(names)) + putArray("sizes", Arguments.fromList(sizes)) } } diff --git a/src/api/__tests__/blob.test.ts b/src/api/__tests__/blob.test.ts index 26fec2bd..4fc03585 100644 --- a/src/api/__tests__/blob.test.ts +++ b/src/api/__tests__/blob.test.ts @@ -14,8 +14,13 @@ const uploadTask = { cancelAsync: vi.fn(async () => undefined), }; const createUploadTask = vi.fn((..._args: unknown[]) => uploadTask); +const copyAsync = vi.fn(async (_options: { from: string; to: string }) => undefined); +const deleteAsync = vi.fn(async (_uri: string, _options?: unknown) => undefined); vi.mock('expo-file-system/legacy', () => ({ + cacheDirectory: 'file:///cache/', createUploadTask: (...args: unknown[]) => createUploadTask(...args), + copyAsync: (options: { from: string; to: string }) => copyAsync(options), + deleteAsync: (uri: string, options?: unknown) => deleteAsync(uri, options), FileSystemUploadType: { BINARY_CONTENT: 0, MULTIPART: 1 }, })); @@ -125,4 +130,38 @@ describe('uploadBlob', () => { ).rejects.toMatchObject({ name: 'AbortError' }); expect(createUploadTask).not.toHaveBeenCalled(); }); + + it('uploads files on disk directly without copying them', async () => { + uploadTask.uploadAsync.mockResolvedValueOnce({ status: 200, body: JSON.stringify({ blobId: 'b' }) }); + await uploadBlob('file:///tmp/a.pdf', 'application/pdf'); + expect(copyAsync).not.toHaveBeenCalled(); + expect(deleteAsync).not.toHaveBeenCalled(); + }); + + it('copies a shared content:// URI into the cache, uploads the copy and deletes it', async () => { + uploadTask.uploadAsync.mockResolvedValueOnce({ status: 200, body: JSON.stringify({ blobId: 'blob-3' }) }); + const shared = 'content://media/external/images/media/1000000027'; + + const result = await uploadBlob(shared, 'image/png'); + + expect(result.blobId).toBe('blob-3'); + const { from, to } = copyAsync.mock.calls[0][0]; + expect(from).toBe(shared); + expect(to).toMatch(/^file:\/\/\/cache\/upload-/); + expect(createUploadTask.mock.calls[0][1]).toBe(to); + expect(deleteAsync).toHaveBeenCalledWith(to, { idempotent: true }); + }); + + it('deletes the cache copy when the upload fails', async () => { + uploadTask.uploadAsync.mockResolvedValueOnce({ status: 500, body: 'boom' }); + await expect(uploadBlob('content://com.example.provider/doc/7', 'application/pdf')).rejects.toThrow(/500/); + const { to } = copyAsync.mock.calls[0][0]; + expect(deleteAsync).toHaveBeenCalledWith(to, { idempotent: true }); + }); + + it('does not upload when the shared file cannot be copied', async () => { + copyAsync.mockRejectedValueOnce(new Error('Permission Denial')); + await expect(uploadBlob('content://com.example.provider/doc/8', 'image/jpeg')).rejects.toThrow(/Permission Denial/); + expect(createUploadTask).not.toHaveBeenCalled(); + }); }); diff --git a/src/api/blob.ts b/src/api/blob.ts index 2739c6f7..ba88c492 100644 --- a/src/api/blob.ts +++ b/src/api/blob.ts @@ -87,7 +87,7 @@ export async function uploadBlob( const accountId = jmapClient.accountId; const uploadUrl = uploadUrlFor(accountId); const contentType = type || 'application/octet-stream'; - const { onProgress, signal } = options; + const { signal } = options; if (signal?.aborted) throw abortError(); const alias = await getClientCertAlias(); @@ -96,9 +96,49 @@ export async function uploadBlob( } const LegacyFileSystem = await loadLegacyFileSystem(); + const cacheCopy = await copyContentUriToCache(LegacyFileSystem, uri); + try { + return await uploadFileStreamed(LegacyFileSystem, cacheCopy ?? uri, contentType, accountId, uploadUrl, options); + } finally { + if (cacheCopy) { + await LegacyFileSystem.deleteAsync(cacheCopy, { idempotent: true }).catch(() => undefined); + } + } +} + +let cacheCopySeq = 0; + +// The native upload task reads from a file path, but files shared into the +// app from another app arrive as Android `content://` URIs with nothing on +// disk behind them. Stream those into the cache first; returns null when +// `uri` is already a file. +async function copyContentUriToCache( + LegacyFileSystem: LegacyFileSystemModule, + uri: string, +): Promise { + if (!uri.startsWith('content://')) return null; + const to = `${LegacyFileSystem.cacheDirectory}upload-${Date.now()}-${++cacheCopySeq}`; + try { + await LegacyFileSystem.copyAsync({ from: uri, to }); + } catch (e) { + await LegacyFileSystem.deleteAsync(to, { idempotent: true }).catch(() => undefined); + throw e; + } + return to; +} + +async function uploadFileStreamed( + LegacyFileSystem: LegacyFileSystemModule, + fileUri: string, + contentType: string, + accountId: string, + uploadUrl: string, + { onProgress, signal }: UploadBlobOptions, +): Promise { + if (signal?.aborted) throw abortError(); const task = LegacyFileSystem.createUploadTask( uploadUrl, - uri, + fileUri, { httpMethod: 'POST', uploadType: LegacyFileSystem.FileSystemUploadType.BINARY_CONTENT, diff --git a/src/lib/__tests__/share-intent.test.ts b/src/lib/__tests__/share-intent.test.ts new file mode 100644 index 00000000..7fcd24b8 --- /dev/null +++ b/src/lib/__tests__/share-intent.test.ts @@ -0,0 +1,53 @@ +import { describe, it, expect } from 'vitest'; +import { shareAttachments } from '../share-intent'; + +describe('shareAttachments', () => { + it('uses the display name, type and size the providing app reported', () => { + expect(shareAttachments({ + uris: ['content://media/external/images/media/1000000027'], + mimeTypes: ['image/png'], + names: ['Screenshot_20260924.png'], + sizes: [48213], + })).toEqual([{ + uri: 'content://media/external/images/media/1000000027', + name: 'Screenshot_20260924.png', + type: 'image/png', + size: 48213, + }]); + }); + + it('falls back to the decoded last URI segment and leaves an unknown size out', () => { + const [a] = shareAttachments({ + uris: ['content://com.example.files/share/Quarterly%20report.pdf'], + mimeTypes: ['application/pdf'], + names: [''], + sizes: [-1], + }); + expect(a).toEqual({ + uri: 'content://com.example.files/share/Quarterly%20report.pdf', + name: 'Quarterly report.pdf', + type: 'application/pdf', + }); + }); + + it('treats a missing or wildcard MIME type as a generic binary', () => { + const list = shareAttachments({ + uris: ['content://a/1', 'content://a/2'], + mimeTypes: ['image/*', ''], + }); + expect(list.map((a) => a.type)).toEqual(['application/octet-stream', 'application/octet-stream']); + }); + + it('keeps names, types and sizes aligned for multiple files', () => { + const list = shareAttachments({ + uris: ['content://a/1', 'content://a/2'], + mimeTypes: ['image/jpeg', 'video/mp4'], + names: ['one.jpg', 'two.mp4'], + sizes: [10, 20], + }); + expect(list.map((a) => [a.name, a.type, a.size])).toEqual([ + ['one.jpg', 'image/jpeg', 10], + ['two.mp4', 'video/mp4', 20], + ]); + }); +}); diff --git a/src/lib/share-intent.ts b/src/lib/share-intent.ts index 82c105a8..33c8743f 100644 --- a/src/lib/share-intent.ts +++ b/src/lib/share-intent.ts @@ -34,17 +34,26 @@ export function addShareListener(listener: (payload: SharePayload) => void): () /** Attachment descriptors for the composer's `prefillAttachments`. */ export function shareAttachments( share: SharePayload, -): Array<{ uri: string; name: string; type: string }> { +): Array<{ uri: string; name: string; type: string; size?: number }> { const uris = share.uris ?? []; return uris.map((uri, i) => { - const last = uri.split('/').filter(Boolean).pop() ?? `shared-${i + 1}`; - let name: string; - try { - name = decodeURIComponent(last); - } catch { - name = last; - } - const type = share.mimeTypes?.[i] || 'application/octet-stream'; - return { uri, name, type }; + const mimeType = share.mimeTypes?.[i]; + const type = mimeType && !mimeType.includes('*') ? mimeType : 'application/octet-stream'; + const size = share.sizes?.[i]; + return { + uri, + name: share.names?.[i]?.trim() || nameFromUri(uri, i), + type, + ...(typeof size === 'number' && size >= 0 ? { size } : {}), + }; }); } + +function nameFromUri(uri: string, i: number): string { + const last = uri.split('/').filter(Boolean).pop() ?? `shared-${i + 1}`; + try { + return decodeURIComponent(last); + } catch { + return last; + } +} diff --git a/src/navigation/linking.ts b/src/navigation/linking.ts index 9d97161b..14488c01 100644 --- a/src/navigation/linking.ts +++ b/src/navigation/linking.ts @@ -181,9 +181,12 @@ export async function handleDeepLink(link: DeepLink, nav: DeepLinkNavigator): Pr export interface SharePayload { text?: string; subject?: string; - // content:// URIs of shared files, with their MIME types when known. + // content:// URIs of shared files, with their MIME types, display names + // and sizes (-1 when unknown) as reported by the providing app. uris?: string[]; mimeTypes?: string[]; + names?: string[]; + sizes?: number[]; } /**