Skip to content

[Due for payment 2026-09-03] Patch react-native-fs to build file URIs with Uri.fromFile #99350

Closed

Description

@mountiny

Problem

getFileUri in react-native-fs built a file URI by string concatenation, Uri.parse("file://" + filepath). Uri.parse then interprets URI syntax in the result, so a # in the path opens a fragment and everything after it is dropped, and a ? opens a query. Every RNFS call that opens a stream goes through getFileUri — readFile, writeFile, appendFile, copyFile, and the cross-filesystem fallback inside moveFile. On Android that fallback always runs for receipts, because the receipts folder sits on external storage while picked files land in the app cache, so the rename fails. The result: a receipt picked through the document picker with a # in its name, such as Gmail - Receipt for Order #12345.pdf, resolved to a truncated path, never reached Documents/Receipts-Upload, and broke ReceiptStorage.adopt.

Solution

Patch getFileUri to call Uri.fromFile(file) instead, which percent-encodes the path before building the URI, so #, ?, and bare % in filenames survive. The patch is a one-line change to RNFSManager.java under patches/react-native-fs/, with a details.md recording the reason and removal condition (delete when react-native-fs is replaced by react-native-blob-util).

Patching rather than upgrading is deliberate: itinance/react-native-fs is unmaintained — last pushed 2024-03-18, 630 open issues, and npm latest is still the 2.20.0 we depend on. The maintained fork already carries the identical fix (birdofpreyru/react-native-fs#6, commit bbcab23, released in @dr.pogodin/react-native-fs v2.22.0), but it ships as a separate npm package, so there is no version of react-native-fs to upgrade to.

Notes

This PR is currently a draft, +31/−0 across 2 new files. Android native only; other platforms are unaffected.

Related: #97979 introduced ReceiptStorage and chose RNFS.moveFile precisely because it falls back to copy-then-delete across filesystems where react-native-blob-util's mv does not. This patch fixes a latent bug in that same fallback path.

PR

#99333

Issue OwnerCurrent Issue Owner: @mallenexpensify

Activity

  1. added
    BugSomething is broken. Auto assigns a BugZero manager.
    ReviewingHas a PR in review
    on Aug 24, 2026
  2. MelvinBot commented on Aug 24, 2026

    @MelvinBot
    Contributor

    Issue Analysis

    Confidence: High — the mechanism is confirmed directly in the vendored react-native-fs source and the ReceiptStorage caller. This validates the root cause and the fix already drafted in #99333.

    Root cause: react-native-fs's Android getFileUri builds a file URI by raw string concatenation — Uri.parse("file://" + filepath) — in RNFSManager.java. Uri.parse then interprets URI syntax in that string, so a # in the path starts a fragment (everything after it is dropped) and a ? starts a query. A receipt picked through the document picker with a # in its name (e.g. Gmail - Receipt for Order #12345.pdf) lands in the app cache under that raw name, and reading it back resolves to a truncated path → openInputStream returns ENOENT → the copy fails.

    Why it surfaces for receipts specifically: ReceiptStorage.adopt calls RNFS.moveFile(sourcePath, …). On Android the receipts folder is on external storage while the picked file is in the app cache, so inFile.renameTo(...) fails across filesystems and RNFS falls back to CopyFileTask (copy-then-delete). That fallback reads the source through getInputStream → getFileUri, and the source still carries the raw #. The destination name is already sanitized via cleanFileName, so the break is on the read side, not the write — which is why adopt fails even though the destination looks clean.

    Fix (as drafted in the PR): patch getFileUri to use Uri.fromFile(file), which percent-encodes the path before constructing the URI, so #, ?, and bare % survive. This is the correct and minimal fix — it matches the identical change already merged in the maintained fork (birdofpreyru/react-native-fs#6), and patching rather than upgrading is justified since itinance/react-native-fs is unmaintained and stuck at the 2.20.0 we depend on. The patches/react-native-fs/ approach keeps the change contained to the Android native path, and the details.md removal condition (drop when RNFS is replaced by react-native-blob-util) is the right hygiene.

    Verification details & one review note
    • Vendored source on main still concatenates: RNFSManager.java getFileUri does uri = Uri.parse("file://" + filepath). Every stream-opening RNFS call funnels through it — getInputStream, getOutputStream, and therefore readFile/writeFile/appendFile/copyFile and the moveFile copy fallback.
    • Uri.fromFile(new File(path)) produces a file:// URI with the path segment percent-encoded, so Uri.parse no longer sees literal #/? as delimiters. Fix scope matches bug scope (all filenames with reserved chars, not just one edge case).
    • Review note: the patch must only change the Uri.parse("file://" + filepath) branch (the no-scheme, absolute-path case). The earlier Uri.parse(filepath) at the top of getFileUri must stay untouched, since callers pass real content:// and file:// URIs there that should not be re-encoded.
    • Testing: Android native only; iOS and all web/mWeb platforms are unaffected (this is Java in the Android module). It can't be meaningfully verified on web — QA should follow the PR's Android steps (pick a file with #, then ?, then bare % in the name and confirm the receipt attaches with no [Receipt] adopt failed in logs).

    Possibly related (not duplicates):


    view run

  3. changed the title [-]Patch react-native-fs to build file URIs with Uri.fromFile[/-] [+][Due for payment 2026-09-03] Patch react-native-fs to build file URIs with Uri.fromFile[/+] on Aug 27, 2026
  4. melvin-bot commented on Aug 27, 2026

    @melvin-bot

    @ZhenjaHorbach

    The solution for this issue has been 🚀 deployed to production 🚀 in version 9.4.62-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

    If no regressions arise, payment will be issued on 2026-09-03. 🎊

    The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the Contributor+ Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]. If no checklist is needed for this issue, you can click: [no checklist button]

  5. moved this from CRITICAL to AWAITING DEPLOYMENT in [#whatsnext] #qualityon Aug 31, 2026
  6. melvin-bot commented on Aug 31, 2026

    @melvin-bot

    @ZhenjaHorbach Whoops! This issue is 2 days overdue. Let's get this updated quick!

  7. melvin-bot commented on Sep 2, 2026

    @melvin-bot

    Triggered auto assignment to @mallenexpensify (Awaiting Payment)

  8. melvin-bot commented on Sep 2, 2026

    @melvin-bot

    Payment Summary

    Resolving PRs:

    BugZero Checklist (@mallenexpensify)

    • I have confirmed assignees, roles, and Upwork contracts look correct
    • I have paid out Upwork contracts / manual NewDot requests
    • [BugZero Assignee] I have created a GH issue for creating/updating the regression test once above steps have been agreed upon
  9. ZhenjaHorbach commented on Sep 3, 2026

    @ZhenjaHorbach
    Contributor

    Contributor+ Checklist:

    • [Contributor] The offending PR and associated issue have been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

    It's a library issue

    • [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

      Link to discussion:

    • [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

    Regression Test Proposal

    Test:

    1. On an Android device, get a file whose name contains a #. Printing a Gmail message to PDF produces one, for example Gmail - Receipt for Order #12345.pdf.
    2. Start an expense and pick that file through the paperclip option in the attachment picker.
    3. Submit the expense.
    4. Verify the expense is created with the receipt attached.
    5. Verify no [Receipt] adopt failed line appears in the logs.
    6. Repeat with a filename containing a ?, and with one containing a bare %, for example Report 50%.pdf.

    Do we agree 👍 or 👎

  10. mallenexpensify commented on Sep 4, 2026

    @mallenexpensify
    Contributor

    Payment Summary

    Contributor+: @ZhenjaHorbach due $250 via NewDot

  11. moved this from AWAITING DEPLOYMENT to Done in [#whatsnext] #qualityon Sep 4, 2026
  12. garrettmknight commented on Oct 7, 2026

    @garrettmknight
    Contributor

    $250 approved for @ZhenjaHorbach

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionBugSomething is broken. Auto assigns a BugZero manager.DailyKSv2

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions