[HOLD for payment 2024-10-11] [$250] Android/iOS - Attachment error displayed when uploading pdf with grave accent in name

[IuliiaHerets]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 9.0.37.3
Reproducible in staging?: Y
Reproducible in production?: Y
Issue reported by: Applause Internal Team

Action Performed:

1. Open the app and log in
2. Navigate to any report
3. Tap "+" > Add attachment > Choose file
4. Select a pdf file with a name containing a grave accent `

Expected Result:

The pdf file is uploaded

Actual Result:

Unable to upload the pdf, the attachment error is displayed

Workaround:

Unknown

Platforms:

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Ragù alla bolognese - updated recipe_20 April 2023.pdf

Bug6607697_1726667305160.video_2024-09-18_09-47-32.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021838993266760256220
• Upwork Job ID: 1838993266760256220
• Last Price Increase: 2024-09-25
• Automatic offers:
• jjcoffee | Reviewer | 104216292

Issue Owner

Current Issue Owner: @JmillsExpensify

[melvin-bot]

Triggered auto assignment to @JmillsExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[IuliiaHerets]

@JmillsExpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[melvin-bot]

📣 @Buqra55! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[bernhardoj]

Proposal

Please re-state the problem that we are trying to solve in this issue.

An attachment error is shown when trying to upload a PDF with an accented character in the name.

What is the root cause of that problem?

When we select the attachment from the document picker, the file source URI is encoded. In our case, ù character is encoded to %C3%B9. Then, we try to show the PDF using the component from react-native-pdf library.

The react-native-pdf already tries to decode it using unescape method,
https://github.com/wonday/react-native-pdf/blob/d545ff27b49850c773f6c443dfc65891881c0ed2/index.js#L238

however, it decodes %C3%B9 to ù which is a wrong character. So, the PDF component throws a file not found error.

What changes do you think we should make in order to solve the problem?

We can replace unescape with decodeURIComponent. Based on the MDN doc, the unescape is also deprecated.

The upstream repo looks inactive, so I think we can just patch it.

[melvin-bot]

@JmillsExpensify Huh... This is 4 days overdue. Who can take care of this?

[JmillsExpensify]

Opening up to the community. Pending proposal review.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~021838993266760256220

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @jjcoffee (External)

[mahaidery]

Contributor details:
Your Expensify account email: mahaidery@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~01232d5f52fda26a6c

[melvin-bot]

📣 @mahaidery! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[jjcoffee]

@bernhardoj's proposal LGTM! I'm not 100% sure what we'd do here in terms of patching vs upstream fix, but I'd think since this is a fairly simple change we can patch it now but also open a PR upstream and try to get that merged simultaneously?

🎀👀🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @neil-marcellini, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[neil-marcellini]

@bernhardoj's proposal LGTM! I'm not 100% sure what we'd do here in terms of patching vs upstream fix, but I'd think since this is a fairly simple change we can patch it now but also open a PR upstream and try to get that merged simultaneously?

🎀👀🎀 C+ reviewed

I agree, hiring

[melvin-bot]

📣 @jjcoffee 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[bernhardoj]

@jjcoffee PR is ready

Upstream PR: wonday/react-native-pdf#873

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

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

• Fix failed to load pdf when the file name contains accented character #50043

If no regressions arise, payment will be issued on 2024-10-11. 🎊

For reference, here are some details about the assignees on this issue:

• @jjcoffee requires payment automatic offer (Reviewer)
• @bernhardoj requires payment through NewDot Manual Requests

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@jjcoffee] The PR that introduced the bug has been identified. Link to the PR:
• [@jjcoffee] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@jjcoffee] A discussion in #expensify-bugs 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:
• [@jjcoffee] Determine if we should create a regression test for this bug.
• [@jjcoffee] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@JmillsExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[jjcoffee]

• The PR that introduced the bug has been identified. Link to the PR: N/A - always existed in react-native-pdf
• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N/A
• A discussion in #expensify-bugs 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: N/A
• Determine if we should create a regression test for this bug. Yes
• If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

1. In any chat add a PDF attachment with accented characters in the filename, e.g. Ragù.alla.bolognese.-.updated.recipe_20.April.2023.pdf
2. Verify the PDF loads correctly

Do we agree 👍 or 👎

[melvin-bot]

@JmillsExpensify, @neil-marcellini, @jjcoffee, @bernhardoj Whoops! This issue is 2 days overdue. Let's get this updated quick!

[JmillsExpensify]

Payment summary:

• Contributor: $250 to @bernhardoj via NewDot
• Reviewer: $250 to @jjcoffee via Upwork

[JmillsExpensify]

Closing this one out!

[bernhardoj]

Requested in ND.

[trjExpensify]

Confirming the payment summary here is correct.

[JmillsExpensify]

$250 approved for @bernhardoj