Skip to content

fix: Block raw HEIC upload when conversion fails - #96024

Merged
mountiny merged 11 commits into
Expensify:mainfrom
annaweber830:fix/95825-heic-image
Aug 7, 2026
Merged

mountiny merged 11 commits into
Expensify:mainfrom
annaweber830:fix/95825-heic-image

Conversation

@annaweber830

@annaweber830 annaweber830 commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Explanation of Change

Fixed Issues

$ #95825
PROPOSAL: #95825 (comment)

Tests

Pre-requisite: download test.zip — a HEIC that triggers conversion failure.

Note: HEIC conversion failure is intermittent — it may take multiple attempts. When it fails, verify the error modal appears and the upload is blocked (no raw .heic is uploaded).

  1. Go to FAB -> Create expense -> Scan.
  2. Select the attached large HEIC image .
  3. Verify either the conversion fails and an Attachment error modal appears with We couldn't process this image. Please try again, or upload the photo in a different format. or you will be able to create the expense if conversion succeeds (which will mean you will not see error submiting expense error from the server)
  4. Open any chat and FAB -> add attachment
  5. Select the HEIC img above
  6. Verify either the conversion fails and an Attachment error modal appears with We couldn't process this image. Please try again, or upload the photo in a different format. or you will be able to add the attachment if conversion succeeds (which will mean you will not see an error from the server on the attachment)
  • Verify that no errors appear in the JS console

Offline tests

Same as test

QA Steps

Pre-requisite: download test.zip — a HEIC that triggers conversion failure.

Note: HEIC conversion failure is intermittent — it may take multiple attempts. When it fails, verify the error modal appears and the upload is blocked (no raw .heic is uploaded).

  1. Go to FAB -> Create expense -> Scan.
  2. Select the attached large HEIC image .
  3. Verify either the conversion fails and an Attachment error modal appears with We couldn't process this image. Please try again, or upload the photo in a different format. or you will be able to create the expense if conversion succeeds (which will mean you will not see error submiting expense error from the server)
  4. Open any chat and FAB -> add attachment
  5. Select the HEIC img above
  6. Verify either the conversion fails and an Attachment error modal appears with We couldn't process this image. Please try again, or upload the photo in a different format. or you will be able to add the attachment if conversion succeeds (which will mean you will not see an error from the server on the attachment)
  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native
issue-95825-fixed-compressed.mp4
Android: mWeb Chrome
expensify-recording-2-compressed.mp4
iOS: Native
Screen.Recording.2026-07-15.at.8.23.32.AM.mov
iOS: mWeb Safari
Screen_Recording_2026-07-16_at_9.06.45.PM.mp4
MacOS: Chrome / Safari
Screen_Recording_2026-07-16_at_8.27.27.PM.mp4

@melvin-bot

melvin-bot Bot commented Jul 14, 2026

Copy link
Copy Markdown

Hey, I noticed you changed src/languages/en.ts in a PR from a fork. For security reasons, translations are not generated automatically for PRs from forks.

If you want to automatically generate translations for other locales, an Expensify employee will have to:

  1. Look at the code and make sure there are no malicious changes.
  2. Run the Generate static translations GitHub workflow. If you have write access and the K2 extension, you can simply click: [this button]

Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running:

npx ts-node ./scripts/generateTranslations.ts --help

Typically, you'd want to translate only what you changed by running npx ts-node ./scripts/generateTranslations.ts --compare-ref main

@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.

Files with missing lines Coverage Δ
src/CONST/index.ts 94.81% <ø> (ø)
...rc/libs/fileDownload/heicConverter/index.native.ts 80.76% <100.00%> (+76.68%) ⬆️
src/components/AttachmentPicker/index.native.tsx 19.40% <0.00%> (+0.08%) ⬆️
src/libs/fileDownload/FileUtils.ts 37.25% <0.00%> (-0.18%) ⬇️
src/hooks/useFilesValidation.tsx 34.59% <0.00%> (+0.21%) ⬆️
... and 83 files with indirect coverage changes

@mountiny

mountiny commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

@annaweber830 how is it looking?

@annaweber830

Copy link
Copy Markdown
Contributor Author

@annaweber830 how is it looking?

I am preparing PR and I will prepare today or tomorrow.

@annaweber830
annaweber830 marked this pull request as ready for review July 15, 2026 04:09
@annaweber830
annaweber830 requested review from a team as code owners July 15, 2026 04:09
@melvin-bot
melvin-bot Bot requested review from FitseTLT and joekaufmanexpensify and removed request for a team July 15, 2026 04:10
@melvin-bot

melvin-bot Bot commented Jul 15, 2026

Copy link
Copy Markdown

@FitseTLT Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

@melvin-bot
melvin-bot Bot removed the request for review from a team July 15, 2026 04:10
Comment thread src/hooks/useFilesValidation.tsx
@FitseTLT

Copy link
Copy Markdown
Contributor

@annaweber830 where is the case of heic upload failure in the test steps. For me when I try to upload a 15MB heic it fails which you can use in the test step.

Also record screenshots pls.

@annaweber830

Copy link
Copy Markdown
Contributor Author

Hi @FitseTLT, thanks! Could you please share a 15 MB HEIC image?

@FitseTLT

Copy link
Copy Markdown
Contributor

Hi @FitseTLT, thanks! Could you please share a 15 MB HEIC image?

You will have to download large img and online convert it to heic on your emulators web browser that's how you can make the file available on the emulators.

@annaweber830

Copy link
Copy Markdown
Contributor Author

Hi @FitseTLT I updated PR.

@FitseTLT

Copy link
Copy Markdown
Contributor

Ok also add android native snapshots and also update the test steps accordingly

@FitseTLT

Copy link
Copy Markdown
Contributor

@joekaufmanexpensify @mountiny can you guys confirm the text that is displayed in the modal is? U can see the ios native snapshot in the OP for reference.

@annaweber830

Copy link
Copy Markdown
Contributor Author

Hi @FitseTLT I updated PR.

@FitseTLT

Copy link
Copy Markdown
Contributor

Hi @FitseTLT I updated PR.

Why don't you update the test steps accordingly for the case that shows the modal? I am asking it for the third time

@annaweber830

Copy link
Copy Markdown
Contributor Author

Hi @FitseTLT Sorry I confused. I updated PR.

@FitseTLT

Copy link
Copy Markdown
Contributor

Hi @FitseTLT Sorry I confused. I updated PR.

It would be nice if you add the exact heic img that caused the modal to appear in the steps otherwise large file can be ambigious.

@joekaufmanexpensify joekaufmanexpensify left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good for Product

@FitseTLT

Copy link
Copy Markdown
Contributor

@joekaufmanexpensify can you confirm the text in the modal is correct when the heic file conversion fails ?

image

@annaweber830

Copy link
Copy Markdown
Contributor Author

Hi @FitseTLT
I don’t think this is specifically a large-image bug. Large images may increase the chance of conversion failure, but smaller images can also fail, while large images can succeed.
The actual bug is the fallback: when conversion fails, the original HEIC is uploaded. The fix should retry once, then block the HEIC and show an error. No file-size limit is needed.

@FitseTLT

Copy link
Copy Markdown
Contributor

Hi @FitseTLT I don’t think this is specifically a large-image bug. Large images may increase the chance of conversion failure, but smaller images can also fail, while large images can succeed. The actual bug is the fallback: when conversion fails, the original HEIC is uploaded. The fix should retry once, then block the HEIC and show an error. No file-size limit is needed.

Yeah but for me a 15 MB heic img always fails and my recommendation worked for you (I suppose you are also using a large file) What I am saying is it would be easy for the QA to give them a clear step and otherwise they can't test this case of heic conversion failure. Can u attach the img you are using for testing?

@annaweber830

Copy link
Copy Markdown
Contributor Author

Hi @FitseTLT thanks for response.
The large HEIC I tested converted successfully on my device, so it is not a reliable QA fixture. Could you please attach the 15 MB HEIC that consistently fails for you? I can use that exact file in the test steps.

@annaweber830

Copy link
Copy Markdown
Contributor Author

Hi @FitseTLT I updated test steps.

@annaweber830

Copy link
Copy Markdown
Contributor Author

Hi @FitseTLT @mountiny what should I do?

@FitseTLT

FitseTLT commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

@annaweber830
Pre-requisite: download test.zip — a HEIC that triggers conversion failure.

Note: HEIC conversion failure is intermittent — it may take multiple attempts. When it fails, verify the error modal appears and the upload is blocked (no raw .heic is uploaded).

Go to FAB -> Create expense -> Scan.
Select the attached large HEIC image .
Verify either the conversion fails and an Attachment error modal appears with We couldn't process this image. Please try again, or upload the photo in a different format. or you will be able to create the expense if conversion succeeds (which will mean you will not see error submiting expense error from the server)
Open any chat and FAB -> add attachment
select the HEIC img above
Verify either the conversion fails and an Attachment error modal appears with We couldn't process this image. Please try again, or upload the photo in a different format. or you will be able to add the attachment if conversion succeeds (which will mean you will not see an error from the server on the attachment)

@annaweber830

Copy link
Copy Markdown
Contributor Author

Hi @FitseTLT I udpated my PR.

@FitseTLT FitseTLT left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Except the copy we are using looks a bit weird when a file fails for multiselecting files I pointed here Let me know your opinion on that. cc @mountiny

@melvin-bot
melvin-bot Bot requested a review from carlosmiceli August 6, 2026 15:12
@mountiny

mountiny commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

That is fair, do we have a way to know it was just one/some from those uploaded that failed when showing the error?

@annaweber830

Copy link
Copy Markdown
Contributor Author

That is fair, do we have a way to know it was just one/some from those uploaded that failed when showing the error?

Hi @mountiny I'll check and see if we can identify which of the uploaded files failed when the error is shown.

@FitseTLT

FitseTLT commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

That is fair, do we have a way to know it was just one/some from those uploaded that failed when showing the error?

It is a gap in the current main that it displays error modals then it will navigate you to the next page if there are successful files. So it is not a gap our pr introduced but I only have a doubt that our error text is a bit misleading. I want to know your direction on that @mountiny
Or we can optionally extend the scope here and fix the existing problem too

@annaweber830

Copy link
Copy Markdown
Contributor Author

That is fair, do we have a way to know it was just one/some from those uploaded that failed when showing the error?

Yes, we can. The conversion callback knows which file failed; we just need to pass that filename into the error state and display it in the modal.

@FitseTLT

FitseTLT commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

That is fair, do we have a way to know it was just one/some from those uploaded that failed when showing the error?

Yes, we can. The conversion callback knows which file failed; we just need to pass that filename into the error state and display it in the modal.

btw there can be multiple files failing and also the other error copies also can be displayed which have the same problems. But at least we can make our error text to not seem like there is only one img failing by making it more general.

@mountiny mountiny left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can handle that separately

@mountiny mountiny closed this Aug 7, 2026
@mountiny mountiny reopened this Aug 7, 2026
@mountiny
mountiny merged commit ec69ea6 into Expensify:main Aug 7, 2026
47 checks passed
@OSBotify

OSBotify commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here.

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.52-0 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 cancelled 🔪

@MelvinBot

Copy link
Copy Markdown
Contributor

🤖 I reviewed this PR against Expensify's help site files under App/docs/articles and no docs updates are required, so I did not open a draft PR.

Why: This is an internal error-handling fix. When a HEIC image fails to convert, the app now blocks the raw .heic upload and shows an error modal (We couldn't process this image. Please try again, or upload the photo in a different format.) instead of silently falling back to uploading the unconverted file. It doesn't add, remove, or change any user-facing feature, workflow, or setting that the help site describes.

What I checked
  • Searched docs/articles for heic/heif → no matches. HEIC/HEIF conversion behavior is not documented anywhere on the help site.
  • Reviewed the two articles that document upload formats and image guidance:
    • Create-an-Expense.md lists supported file formats (JPG, PNG, GIF, PDF, TIFF, HTML, RTF, DOCX, XML) — HEIC is not listed, and this PR doesn't change that list.
    • Troubleshoot-SmartScan-Issues.md mentions common image formats (JPG, PNG) and PDF for best results — unchanged by this PR.
  • The only user-facing addition is a new error string (errorWhileConvertingHeic) shown on conversion failure, which is transient UI copy, not documented help-site behavior.

Since HEIC support is not a documented feature and the change is limited to failure-path error handling, there is no help-site content that becomes inaccurate as a result of this PR.

If you'd still like a proactive doc note added (e.g. calling out image-format guidance for receipt uploads), let me know and I'll draft one.

@IuliiaHerets

Copy link
Copy Markdown

Hi @annaweber830. QA team failed this PR for iOS with the original issue; the HEIC image was uploaded without a modal error being shown

1786460263363.GALLERY.mp4

cc @mountiny @joekaufmanexpensify @carlosmiceli

@FitseTLT

Copy link
Copy Markdown
Contributor

Hi @annaweber830. QA team failed this PR for iOS with the original issue; the HEIC image was uploaded without a modal error being shown

1786460263363.GALLERY.mp4
cc @mountiny @joekaufmanexpensify @carlosmiceli

Nope @IuliiaHerets we clearly stated on the steps that the failing only happens intermittently and if it doesn't fail it should create a successful expense. that's the change 👍

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/roryabraham in version: 9.4.52-11 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

Bundle Size Analysis (Sentry):

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants