Skip to content

Commit c0fb3e1

Browse files
authored
Merge pull request #96940 from callstack-internal/eliran/2397-snapshot-picked-files
2 parents 084ac39 + f55d059 commit c0fb3e1

4 files changed

Lines changed: 176 additions & 5 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/**
2+
* Native keeps the picked File as-is: there is no IndexedDB blob path to poison, and the
3+
* File polyfill has no arrayBuffer. Only rename when the cleaned name differs.
4+
*/
5+
function snapshotPickedFile(file: File, name: string): Promise<File> {
6+
if (file.name !== name) {
7+
return Promise.resolve(new File([file], name, {type: file.type}));
8+
}
9+
return Promise.resolve(file);
10+
}
11+
12+
export default snapshotPickedFile;
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import {isMobile} from '@libs/Browser';
2+
3+
// iPadOS Safari in "Request Desktop Website" mode (the default) reports a Macintosh user agent that
4+
// isMobile() can't recognize; real Macs report zero touch points.
5+
function isIPadInDesktopMode(): boolean {
6+
return /Macintosh/i.test(navigator.userAgent) && navigator.maxTouchPoints > 1;
7+
}
8+
9+
/**
10+
* Copies a picked file's bytes into a memory-backed File. A picked File only references its OS
11+
* file, so if that file is modified or deleted before the queued request is persisted, the
12+
* IndexedDB write fails with "Failed to write blobs" and poisons the persisted request queue.
13+
* Rejects when the backing file is already unreadable.
14+
*/
15+
async function snapshotPickedFile(file: File, name: string): Promise<File> {
16+
// Mobile browsers hand over sandboxed temp copies the OS won't touch after picking, and copying
17+
// every file's bytes would multiply peak memory by batch size on memory-constrained mobile
18+
// Safari — keep the lazy File there and only clean the name.
19+
if (isMobile() || isIPadInDesktopMode()) {
20+
if (file.name !== name) {
21+
return new File([file], name, {type: file.type});
22+
}
23+
return file;
24+
}
25+
return new File([await file.arrayBuffer()], name, {type: file.type, lastModified: file.lastModified});
26+
}
27+
28+
export default snapshotPickedFile;

src/libs/validateAttachmentFile.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type {FileObject} from '@src/types/utils/Attachment';
44
import type {ValueOf} from 'type-fest';
55

66
import {cleanFileName, hasHeicOrHeifExtension, isValidReceiptExtension, normalizeFileObject, validateImageForCorruption} from './fileDownload/FileUtils';
7+
import snapshotPickedFile from './snapshotPickedFile';
78

89
type ValidateAttachmentValidResult = {
910
isValid: true;
@@ -67,12 +68,16 @@ async function validateAttachmentFile(file: FileObject, item?: DataTransferItem,
6768
*/
6869
let updatedFile = normalizedFile;
6970
const cleanName = cleanFileName(updatedFile.name);
70-
if (updatedFile.name !== cleanName) {
71-
updatedFile = new File([updatedFile], cleanName, {type: updatedFile.type});
71+
// On web this snapshots the bytes into a memory-backed File so a later change to the OS file
72+
// can't invalidate the queued request (see snapshotPickedFile); on native it only cleans the name.
73+
try {
74+
updatedFile = await snapshotPickedFile(updatedFile, cleanName);
75+
} catch {
76+
// The backing file was already modified or deleted since it was picked.
77+
return {isValid: false, error: CONST.FILE_VALIDATION_ERRORS.FILE_INVALID};
7278
}
73-
// Read the superseded URI from normalizedFile: when the name needed cleaning, updatedFile was
74-
// reassigned to a fresh File that doesn't carry the custom .uri property, so reading it there
75-
// would skip the revoke exactly for cleaned filenames (e.g. default macOS screenshot names).
79+
// Read the superseded URI from normalizedFile: snapshotPickedFile may return a fresh File that
80+
// doesn't carry the custom .uri property, so updatedFile.uri is not reliable for the previous URL.
7681
const previousUri = normalizedFile.uri;
7782
const inputSource = URL.createObjectURL(updatedFile);
7883
if (previousUri && previousUri !== inputSource && previousUri.startsWith('blob:')) {
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
import {isMobile} from '@libs/Browser';
2+
import validateAttachmentFile from '@libs/validateAttachmentFile';
3+
4+
import type {FileObject} from '@src/types/utils/Attachment';
5+
6+
import CONST from '../../src/CONST';
7+
import * as FileUtils from '../../src/libs/fileDownload/FileUtils';
8+
9+
// Jest resolves the .native variant of platform-split modules; force the web implementation
10+
// since the OS-file snapshot behavior under test is web-only.
11+
jest.mock('@src/libs/snapshotPickedFile', () => jest.requireActual<{default: (file: File, name: string) => Promise<File>}>('@src/libs/snapshotPickedFile/index.ts'));
12+
13+
// The web snapshot only copies bytes on desktop browsers; make the browser type controllable per test.
14+
jest.mock('@src/libs/Browser', () => ({
15+
...jest.requireActual<Record<string, unknown>>('@src/libs/Browser'),
16+
isMobile: jest.fn(() => false),
17+
}));
18+
19+
// Mock only normalizeFileObject and validateImageForCorruption; keep the rest real
20+
jest.mock('@src/libs/fileDownload/FileUtils', () => {
21+
const actual = jest.requireActual<typeof FileUtils>('@src/libs/fileDownload/FileUtils');
22+
return {
23+
...actual,
24+
normalizeFileObject: jest.fn(),
25+
validateImageForCorruption: jest.fn(),
26+
};
27+
});
28+
29+
const mockFileUtils = jest.mocked(FileUtils);
30+
31+
describe('validateAttachmentFile OS-backed file snapshot (web)', () => {
32+
beforeEach(() => {
33+
jest.clearAllMocks();
34+
mockFileUtils.normalizeFileObject.mockImplementation(async (file) => file);
35+
mockFileUtils.validateImageForCorruption.mockResolvedValue(undefined);
36+
jest.mocked(isMobile).mockReturnValue(false);
37+
});
38+
39+
it('snapshots the picked file bytes into a new memory-backed File', async () => {
40+
const createObjectURLSpy = jest.spyOn(URL, 'createObjectURL').mockReturnValue('blob:new-url');
41+
try {
42+
const file: FileObject = new File([new Blob(['content'], {type: 'text/plain'})], 'image.png', {type: 'image/png'});
43+
// The RN File polyfill in Jest has no arrayBuffer; emulate the web File API.
44+
const arrayBufferSpy = jest.fn().mockResolvedValue(new ArrayBuffer(7));
45+
Object.defineProperty(file, 'arrayBuffer', {value: arrayBufferSpy, configurable: true});
46+
47+
const result = await validateAttachmentFile(file);
48+
49+
expect(result.isValid).toBe(true);
50+
if (!result.isValid) {
51+
throw new Error('validateAttachmentFile should return a valid result');
52+
}
53+
expect(arrayBufferSpy).toHaveBeenCalled();
54+
// The returned File must be a fresh memory-backed copy, not the OS-backed original.
55+
expect(result.file).not.toBe(file);
56+
} finally {
57+
createObjectURLSpy.mockRestore();
58+
}
59+
});
60+
61+
it('returns FILE_INVALID when the picked file can no longer be read (deleted or modified on disk)', async () => {
62+
const file: FileObject = new File([new Blob(['content'], {type: 'text/plain'})], 'image.png', {type: 'image/png'});
63+
// Chromium rejects the read when the backing OS file changed since it was picked.
64+
const arrayBufferSpy = jest.fn().mockRejectedValue(new DOMException('The requested file could not be read', 'NotReadableError'));
65+
Object.defineProperty(file, 'arrayBuffer', {value: arrayBufferSpy, configurable: true});
66+
67+
const result = await validateAttachmentFile(file);
68+
69+
expect(result.isValid).toBe(false);
70+
if (result.isValid) {
71+
throw new Error('validateAttachmentFile should return an invalid result');
72+
}
73+
expect(result.error).toBe(CONST.FILE_VALIDATION_ERRORS.FILE_INVALID);
74+
});
75+
76+
it('keeps the lazy OS-backed File on mobile browsers so a multi-file selection is not held in memory', async () => {
77+
jest.mocked(isMobile).mockReturnValue(true);
78+
const createObjectURLSpy = jest.spyOn(URL, 'createObjectURL').mockReturnValue('blob:new-url');
79+
try {
80+
const file: FileObject = new File([new Blob(['content'], {type: 'text/plain'})], 'image.png', {type: 'image/png'});
81+
const arrayBufferSpy = jest.fn();
82+
Object.defineProperty(file, 'arrayBuffer', {value: arrayBufferSpy, configurable: true});
83+
84+
const result = await validateAttachmentFile(file);
85+
86+
expect(result.isValid).toBe(true);
87+
if (!result.isValid) {
88+
throw new Error('validateAttachmentFile should return a valid result');
89+
}
90+
// Mobile-picked files are sandboxed temp copies, so the bytes are not copied into memory.
91+
expect(arrayBufferSpy).not.toHaveBeenCalled();
92+
expect(result.file).toBe(file);
93+
} finally {
94+
createObjectURLSpy.mockRestore();
95+
}
96+
});
97+
98+
it('keeps the lazy File on iPadOS Safari in desktop mode (Macintosh user agent with touch points)', async () => {
99+
const createObjectURLSpy = jest.spyOn(URL, 'createObjectURL').mockReturnValue('blob:new-url');
100+
const originalUserAgent = navigator.userAgent;
101+
const originalMaxTouchPoints = navigator.maxTouchPoints;
102+
Object.defineProperty(navigator, 'userAgent', {
103+
value: 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.4 Safari/605.1.15',
104+
configurable: true,
105+
});
106+
Object.defineProperty(navigator, 'maxTouchPoints', {value: 5, configurable: true});
107+
try {
108+
const file: FileObject = new File([new Blob(['content'], {type: 'text/plain'})], 'image.png', {type: 'image/png'});
109+
const arrayBufferSpy = jest.fn();
110+
Object.defineProperty(file, 'arrayBuffer', {value: arrayBufferSpy, configurable: true});
111+
112+
const result = await validateAttachmentFile(file);
113+
114+
expect(result.isValid).toBe(true);
115+
if (!result.isValid) {
116+
throw new Error('validateAttachmentFile should return a valid result');
117+
}
118+
expect(arrayBufferSpy).not.toHaveBeenCalled();
119+
expect(result.file).toBe(file);
120+
} finally {
121+
createObjectURLSpy.mockRestore();
122+
Object.defineProperty(navigator, 'userAgent', {value: originalUserAgent, configurable: true});
123+
Object.defineProperty(navigator, 'maxTouchPoints', {value: originalMaxTouchPoints, configurable: true});
124+
}
125+
});
126+
});

0 commit comments

Comments
 (0)