-
Notifications
You must be signed in to change notification settings - Fork 4k
Snapshot picked files into memory-backed Files to prevent request queue poisoning #96940
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
mountiny
merged 7 commits into
Expensify:main
from
callstack-internal:eliran/2397-snapshot-picked-files
Jul 28, 2026
+176
−5
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
ed4a441
Snapshot picked files into memory-backed Files to prevent queue poiso…
elirangoshen 2ac142d
Avoid unsafe type assertions in arrayBuffer test mocks
elirangoshen 04ed14c
Move file snapshot into a platform-split module
elirangoshen af128f9
Snapshot picked file bytes on desktop browsers only
elirangoshen c19c7d6
Skip byte snapshot on iPadOS Safari in desktop mode
elirangoshen a47ebb1
Merge branch 'main' into eliran/2397-snapshot-picked-files
elirangoshen f55d059
Potential fix for pull request finding
elirangoshen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| /** | ||
| * Native keeps the picked File as-is: there is no IndexedDB blob path to poison, and the | ||
| * File polyfill has no arrayBuffer. Only rename when the cleaned name differs. | ||
| */ | ||
| function snapshotPickedFile(file: File, name: string): Promise<File> { | ||
| if (file.name !== name) { | ||
| return Promise.resolve(new File([file], name, {type: file.type})); | ||
| } | ||
| return Promise.resolve(file); | ||
| } | ||
|
|
||
| export default snapshotPickedFile; |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| import {isMobile} from '@libs/Browser'; | ||
|
|
||
| // iPadOS Safari in "Request Desktop Website" mode (the default) reports a Macintosh user agent that | ||
| // isMobile() can't recognize; real Macs report zero touch points. | ||
| function isIPadInDesktopMode(): boolean { | ||
| return /Macintosh/i.test(navigator.userAgent) && navigator.maxTouchPoints > 1; | ||
| } | ||
|
|
||
| /** | ||
| * Copies a picked file's bytes into a memory-backed File. A picked File only references its OS | ||
| * file, so if that file is modified or deleted before the queued request is persisted, the | ||
| * IndexedDB write fails with "Failed to write blobs" and poisons the persisted request queue. | ||
| * Rejects when the backing file is already unreadable. | ||
| */ | ||
| async function snapshotPickedFile(file: File, name: string): Promise<File> { | ||
| // Mobile browsers hand over sandboxed temp copies the OS won't touch after picking, and copying | ||
| // every file's bytes would multiply peak memory by batch size on memory-constrained mobile | ||
| // Safari — keep the lazy File there and only clean the name. | ||
| if (isMobile() || isIPadInDesktopMode()) { | ||
| if (file.name !== name) { | ||
| return new File([file], name, {type: file.type}); | ||
| } | ||
| return file; | ||
| } | ||
| return new File([await file.arrayBuffer()], name, {type: file.type, lastModified: file.lastModified}); | ||
| } | ||
|
|
||
| export default snapshotPickedFile; | ||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| import {isMobile} from '@libs/Browser'; | ||
| import validateAttachmentFile from '@libs/validateAttachmentFile'; | ||
|
|
||
| import type {FileObject} from '@src/types/utils/Attachment'; | ||
|
|
||
| import CONST from '../../src/CONST'; | ||
| import * as FileUtils from '../../src/libs/fileDownload/FileUtils'; | ||
|
|
||
| // Jest resolves the .native variant of platform-split modules; force the web implementation | ||
| // since the OS-file snapshot behavior under test is web-only. | ||
| jest.mock('@src/libs/snapshotPickedFile', () => jest.requireActual<{default: (file: File, name: string) => Promise<File>}>('@src/libs/snapshotPickedFile/index.ts')); | ||
|
|
||
| // The web snapshot only copies bytes on desktop browsers; make the browser type controllable per test. | ||
| jest.mock('@src/libs/Browser', () => ({ | ||
| ...jest.requireActual<Record<string, unknown>>('@src/libs/Browser'), | ||
| isMobile: jest.fn(() => false), | ||
| })); | ||
|
|
||
| // Mock only normalizeFileObject and validateImageForCorruption; keep the rest real | ||
| jest.mock('@src/libs/fileDownload/FileUtils', () => { | ||
| const actual = jest.requireActual<typeof FileUtils>('@src/libs/fileDownload/FileUtils'); | ||
| return { | ||
| ...actual, | ||
| normalizeFileObject: jest.fn(), | ||
| validateImageForCorruption: jest.fn(), | ||
| }; | ||
| }); | ||
|
|
||
| const mockFileUtils = jest.mocked(FileUtils); | ||
|
|
||
| describe('validateAttachmentFile OS-backed file snapshot (web)', () => { | ||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| mockFileUtils.normalizeFileObject.mockImplementation(async (file) => file); | ||
| mockFileUtils.validateImageForCorruption.mockResolvedValue(undefined); | ||
| jest.mocked(isMobile).mockReturnValue(false); | ||
| }); | ||
|
|
||
| it('snapshots the picked file bytes into a new memory-backed File', async () => { | ||
| const createObjectURLSpy = jest.spyOn(URL, 'createObjectURL').mockReturnValue('blob:new-url'); | ||
| try { | ||
| const file: FileObject = new File([new Blob(['content'], {type: 'text/plain'})], 'image.png', {type: 'image/png'}); | ||
| // The RN File polyfill in Jest has no arrayBuffer; emulate the web File API. | ||
| const arrayBufferSpy = jest.fn().mockResolvedValue(new ArrayBuffer(7)); | ||
| Object.defineProperty(file, 'arrayBuffer', {value: arrayBufferSpy, configurable: true}); | ||
|
|
||
| const result = await validateAttachmentFile(file); | ||
|
|
||
| expect(result.isValid).toBe(true); | ||
| if (!result.isValid) { | ||
| throw new Error('validateAttachmentFile should return a valid result'); | ||
| } | ||
| expect(arrayBufferSpy).toHaveBeenCalled(); | ||
| // The returned File must be a fresh memory-backed copy, not the OS-backed original. | ||
| expect(result.file).not.toBe(file); | ||
| } finally { | ||
| createObjectURLSpy.mockRestore(); | ||
| } | ||
| }); | ||
|
|
||
| it('returns FILE_INVALID when the picked file can no longer be read (deleted or modified on disk)', async () => { | ||
| const file: FileObject = new File([new Blob(['content'], {type: 'text/plain'})], 'image.png', {type: 'image/png'}); | ||
| // Chromium rejects the read when the backing OS file changed since it was picked. | ||
| const arrayBufferSpy = jest.fn().mockRejectedValue(new DOMException('The requested file could not be read', 'NotReadableError')); | ||
| Object.defineProperty(file, 'arrayBuffer', {value: arrayBufferSpy, configurable: true}); | ||
|
|
||
| const result = await validateAttachmentFile(file); | ||
|
|
||
| expect(result.isValid).toBe(false); | ||
| if (result.isValid) { | ||
| throw new Error('validateAttachmentFile should return an invalid result'); | ||
| } | ||
| expect(result.error).toBe(CONST.FILE_VALIDATION_ERRORS.FILE_INVALID); | ||
| }); | ||
|
|
||
| it('keeps the lazy OS-backed File on mobile browsers so a multi-file selection is not held in memory', async () => { | ||
| jest.mocked(isMobile).mockReturnValue(true); | ||
| const createObjectURLSpy = jest.spyOn(URL, 'createObjectURL').mockReturnValue('blob:new-url'); | ||
| try { | ||
| const file: FileObject = new File([new Blob(['content'], {type: 'text/plain'})], 'image.png', {type: 'image/png'}); | ||
| const arrayBufferSpy = jest.fn(); | ||
| Object.defineProperty(file, 'arrayBuffer', {value: arrayBufferSpy, configurable: true}); | ||
|
|
||
| const result = await validateAttachmentFile(file); | ||
|
|
||
| expect(result.isValid).toBe(true); | ||
| if (!result.isValid) { | ||
| throw new Error('validateAttachmentFile should return a valid result'); | ||
| } | ||
| // Mobile-picked files are sandboxed temp copies, so the bytes are not copied into memory. | ||
| expect(arrayBufferSpy).not.toHaveBeenCalled(); | ||
| expect(result.file).toBe(file); | ||
| } finally { | ||
| createObjectURLSpy.mockRestore(); | ||
| } | ||
| }); | ||
|
|
||
| it('keeps the lazy File on iPadOS Safari in desktop mode (Macintosh user agent with touch points)', async () => { | ||
| const createObjectURLSpy = jest.spyOn(URL, 'createObjectURL').mockReturnValue('blob:new-url'); | ||
| const originalUserAgent = navigator.userAgent; | ||
| const originalMaxTouchPoints = navigator.maxTouchPoints; | ||
| Object.defineProperty(navigator, 'userAgent', { | ||
| 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', | ||
| configurable: true, | ||
| }); | ||
| Object.defineProperty(navigator, 'maxTouchPoints', {value: 5, configurable: true}); | ||
| try { | ||
| const file: FileObject = new File([new Blob(['content'], {type: 'text/plain'})], 'image.png', {type: 'image/png'}); | ||
| const arrayBufferSpy = jest.fn(); | ||
| Object.defineProperty(file, 'arrayBuffer', {value: arrayBufferSpy, configurable: true}); | ||
|
|
||
| const result = await validateAttachmentFile(file); | ||
|
|
||
| expect(result.isValid).toBe(true); | ||
| if (!result.isValid) { | ||
| throw new Error('validateAttachmentFile should return a valid result'); | ||
| } | ||
| expect(arrayBufferSpy).not.toHaveBeenCalled(); | ||
| expect(result.file).toBe(file); | ||
| } finally { | ||
| createObjectURLSpy.mockRestore(); | ||
| Object.defineProperty(navigator, 'userAgent', {value: originalUserAgent, configurable: true}); | ||
| Object.defineProperty(navigator, 'maxTouchPoints', {value: originalMaxTouchPoints, configurable: true}); | ||
| } | ||
| }); | ||
| }); |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.