mirror of
https://github.com/bulwarkmail/native.git
synced 2026-11-06 08:07:23 +00:00
fix: upload files shared into the app with their real name and type
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
This commit is contained in:
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
+42
-2
@@ -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<string | null> {
|
||||
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<UploadResult> {
|
||||
if (signal?.aborted) throw abortError();
|
||||
const task = LegacyFileSystem.createUploadTask(
|
||||
uploadUrl,
|
||||
uri,
|
||||
fileUri,
|
||||
{
|
||||
httpMethod: 'POST',
|
||||
uploadType: LegacyFileSystem.FileSystemUploadType.BINARY_CONTENT,
|
||||
|
||||
@@ -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],
|
||||
]);
|
||||
});
|
||||
});
|
||||
+19
-10
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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[];
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user