Fix PDF viewer crash for filenames with '%' or other URL-significant characters#269
Fix PDF viewer crash for filenames with '%' or other URL-significant characters#269sentry[bot] wants to merge 2 commits into
Conversation
…characters PDFs whose filename contains a '%' (e.g. "...BE BETTER THAN 99%.pdf") crashed the viewer on iOS with NSInvalidArgumentException. Root cause: File.downloadFileAsync saved the cached file under a name derived from the original filename, preserving the literal '%'. The react-native-pdf JS layer strips "file://" and runs decodeURIComponent on the path, then the iOS native layer runs CFURLCreateStringByReplacingPercentEscapes on it again. A literal '%' is an invalid percent-escape, so that call returns nil, and [NSURL fileURLWithPath:nil] throws. Percent-encoding the URI cannot fix this because the JS layer decodes it before the native layer sees it. The reliable fix is to ensure the cached file is never written with a name containing URL-significant characters. - Extend sanitizeFileName to also neutralize '%' and '#' - Download the PDF (and the share copy) to a sanitized cacheFileDestination instead of the raw cache directory, matching the existing pattern used by the post and purchase download screens - Pass the file name through to the PDF viewer so the cached/shared file keeps a meaningful name (spaces are preserved; they are valid in file URIs) Fixes GUMROAD-MOBILE-WX
|
@claude review once |
Greptile SummaryFixes an iOS crash (
Confidence Score: 5/5Safe to merge — changes are confined to cache path construction and filename sanitization, with no impact on auth, networking, or data persistence. The fix is surgical: a two-character addition to a regex and a destination swap in the download calls. Both the initial download and the share path go through the same sanitized destination. The fallback key ensures the new path is always used. Regression tests directly assert the absence of % in the download destination and cover the no-productFileId fallback. No pre-existing behavior is altered for filenames that were already clean. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Router as Expo Router (post/purchase)
participant Viewer as pdf-viewer.tsx
participant Utils as file-utils.ts
participant FS as expo-file-system
participant Native as react-native-pdf (iOS native)
Router->>Viewer: "navigate({ uri, fileName, productFileId })"
Viewer->>Utils: cacheFileDestination(productFileId ?? "pdf-viewer", fileName ?? "document.pdf")
Utils->>Utils: "sanitizeFileName() replaces %#/\\:*?"<>|"
Utils->>FS: new Directory(Paths.cache, uniqueKey).create()
Utils-->>Viewer: "File { uri: "/cache/pf1/safe_name.pdf" }"
Viewer->>FS: "File.downloadFileAsync(remoteUri, sanitizedFile, { idempotent: true })"
FS-->>Viewer: "{ uri: "file:///cache/pf1/safe_name.pdf" }"
Viewer->>Native: "source={{ uri: "file:///cache/pf1/safe_name.pdf" }}"
Note over Native: CFURLCreateStringByReplacingPercentEscapes succeeds — no bare % in path
Native-->>Viewer: onLoadComplete()
Reviews (2): Last reviewed commit: "Ensure PDF cache fallback is sanitized" | Re-trigger Greptile |
What
Opening a PDF whose filename contains a
%(e.g.HOW TO BECOME AN ELITE PLAYER AND BE BETTER THAN 99%.pdf) crashed the PDF viewer on iOS withNSInvalidArgumentException. This PR makes the viewer download such PDFs to a sanitized cache path so they load correctly.Concrete changes:
sanitizeFileNameinlib/file-utils.tsto also neutralize%and#(characters that are significant in URLs and cannot appear unescaped in afile://path).app/pdf-viewer.tsxto a sanitizedcacheFileDestinationinstead of the raw cache directory — the same pattern already used by the post and purchase download screens.app/post/[id].tsxandapp/purchase/[token].tsxso the cached/shared file keeps a meaningful name (spaces are preserved; they are valid in file URIs).Why
Root cause (a literal
%in the cached filename):File.downloadFileAsyncsaved the cached file under a name derived from the original PDF filename, preserving the literal%.react-native-pdfJS layer stripsfile://and runsdecodeURIComponenton the path, then passes it to native.RNPDFPdfView.mm) runsCFURLCreateStringByReplacingPercentEscapeson the path again. A literal%is an invalid percent-escape, so it returnsnil, and the subsequent[NSURL fileURLWithPath:nil]throwsNSInvalidArgumentException.Percent-encoding the URI before handing it to
react-native-pdfdoes not work, because the JS layerdecodeURIComponents it back before the native layer sees it — reintroducing the literal%. The only reliable fix is to make sure the cached file is never written with a name containing URL-significant characters.Spaces are intentionally left untouched: they survive the whole decode pipeline and are valid in
file://URIs, so sanitizing them would needlessly degrade shared file names for the (very common) case of PDFs with spaces.Before/After
%before merging, per the contributing guide....99%.pdfcrashes the app on load....99_.pdfand opens normally; sharing still works and keeps a readable name.Test Results
tests/lib/file-utils.test.tsverifiescacheFileDestinationneutralizes%/#(and filesystem-illegal chars) while preserving spaces.tests/app/pdf-viewer.test.tsxasserts the download destination is the sanitized cache path (no%). Both tests fail when the fix is reverted.222 passed.tsc --noEmitandexpo lintboth clean.🤖 AI disclosure: implemented by Claude (Claude Opus 4.6).
Prompt given to the agent: "Fix the PDF crash for filenames with spaces/special chars (invalid
file://URI → nil path crash in react-native-pdf). Ensure the fix is fully working, add tests, commit, push, and open a PR."Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Low Risk
Localized PDF cache/share path change with existing utility pattern and regression tests; no auth or API changes.
Overview
Fixes an iOS crash when opening PDFs whose original filenames contain
%or#, by caching downloads under a sanitized path instead of writing into the raw cache with the unsanitized name.The PDF viewer now uses
cacheFileDestination(same helper as post/purchase downloads) for both the initial download and the share fallback, keyed byproductFileIdand an optionalfileNameroute param (defaultdocument.pdf).sanitizeFileNameinfile-utilsadditionally replaces%and#so nativefile://handling inreact-native-pdfdoes not hit invalid percent-escapes.Post and purchase entry points pass through each file’s
nameasfileNameso cached/shared files stay readable while spaces remain allowed.New/updated tests cover sanitized destinations in the viewer and
cacheFileDestinationbehavior.Reviewed by Cursor Bugbot for commit 404bc05. Bugbot is set up for automated code reviews on this repo. Configure here.