[$2000] File upload in Safari: changing the filename in the filesystem changes what file is going to be uploaded

[youssef-lr]

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

---

Action Performed:

Prerequisites: Both images used in testing should be in the same folder.

1. Open Expensify
2. Open any chat
3. Click Attach button
4. Attach the first image you want to upload and stop at the preview step, do not click Send.
5. Go to your filesystem and rename the 2nd image to the filename of the image you chose in Step 4 and that is shown in the preview.
6. Click Send.

Expected Result:

The image displayed in the preview should be uploaded.

Actual Result:

The image we changed its filename to match the filename shown in the preview is uploaded instead.

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Safari
• MacOS / Desktop

Version Number: v1.2.61-0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2023-01-30.at.16.22.23.mov

Expensify/Expensify Issue URL:
Issue reported by: Slack oesayan
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1673424557657069

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01f7cf66b7d484960a
• Upwork Job ID: 1624647662713012224
• Last Price Increase: 2023-02-19

[sonialiap]

Unassigning myself since Youssef self assigned :)

[MelvinBot]

@youssef-lr Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[MelvinBot]

@youssef-lr Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[MelvinBot]

@youssef-lr Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[MelvinBot]

@youssef-lr Huh... This is 4 days overdue. Who can take care of this?

[youssef-lr]

Investigating this, will probably make it external.

[MelvinBot]

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

[MelvinBot]

Triggered auto assignment to @isabelastisser (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[MelvinBot]

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

[MelvinBot]

Current assignee @youssef-lr is eligible for the External assigner, not assigning anyone new.

[youssef-lr]

Unable to look into this further at the moment as I need to finish up a few more priority tasks. This is open for porposals.

[bernhardoj]

Proposal

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

The file sent to a chat is wrong if we rename another file to the first file name. No issue on native.

What is the root cause of that problem?

When we select a file, we receive a File object. A File object works like a pointer to the file and we can get the data of it by using FileReader. If we rename the file for example (a.jpg -> b.jpg), we will encounter an error while uploading it. I'm guessing on the backend, we are using sort of (or maybe the same) FileReader to read the data, but because the file itself has been renamed to b.jpg, thus the file reader can't find it.

In this particular issue case, we rename the selected file (a.jpg -> b.jpg) and also another file to be the first file name (c.jpg -> a.jpg). So, when we upload it, the file reader will read the second file. However, on chromium web-based, it will throw an error ERR_UPLOAD_FILE_CHANGED on console and on Firefox, it will throw a CORS error (still idk why).

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

We should make a copy of the underlying data of the selected file (instead of the File object as I did on my previous proposal) for both attachment picker and drag and drop. We can create a new asynchronous (promise based) util function called cloneFile which accepts a file as parameter. Inside it, we will read the file data using FileReader and resolve the new file object pointing to the new data when it is done.

By making a copy of the file, we won't need to worry any modification (rename, delete, move) done to the file we selected.

Reference:

1. https://stackoverflow.com/a/53707567/11488790
2. https://stackoverflow.com/a/24834417/11488790

[parasharrajat]

Ok, I see. I need to reproduce it myself before I analyze your proposal. Is there anything that you want to share to speed up the review @bernhardoj like more details, links for useful info, research done, etc?

I am traveling today so ETA is 20 hours.

[PankajAS]

Proposal

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

The file sent to a chat is wrong if we rename or delete file from local storage.

What is the root cause of that problem?

Root cause of this problem is related to file Uri(local path) which we are accessing 2 time first when we showing image crop and second time we are access Uri to upload on server. so when we select file for crop and change name of file or delete then it cant access file Uri when uploading on server.

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

We can solve this problem we can store the image data in-memory or device storage so while upload file on server we can access stored file.

What alternative solutions did you explore? (Optional)

None

[parasharrajat]

Thanks for the proposal @PankajAS but it sounds the same as the previous proposal. In not, more details will help.

[PankajAS]

@parasharrajat for more technical details we can do this:

1. When Image open with AttachmentModal we can create a function which store file in copy of file in cache using react-native-fs
   Example:
   const localPath = this.state.imageuri;
const cachePath = RNFS.CachesDirectoryPath + '/filename.jpg';
await RNFS.copyFile(localPath, cachePath);
2. Now we can use cachePath to upload file details on server.

[parasharrajat]

@PankajAS How will you use that on the web? Did you face the same issue on native as well?

[MelvinBot]

@youssef-lr @parasharrajat @isabelastisser this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[MelvinBot]

@youssef-lr @parasharrajat @isabelastisser this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[bernhardoj]

@parasharrajat while looking into this issue, I was trying to find an official explanation/documentation about the ERR_UPLOAD_FILE_CHANGED, but I can't find one. I only able to find some reported issues related to the error and the other answered that the error will be thrown if the file they are trying to upload did get changed. I also find the chromium source code that possibly will throw the error when the file did change.

I can't find a source to why Firefox throw a cors blocked error and I just assume on Safari it picked the file by the path 😅.

[PankajAS]

for web we can use

@PankajAS How will you use that on the web? Did you face the same issue on native as well?

@parasharrajat for web we have to use FileReader which result data we can store in localstorage/sessionstorage as react-native-fs not support web. On native side there is no issue related to image upload.

[parasharrajat]

Googled a couple of things today about the relative path and file references. I will dig a little more tomorrow

[isabelastisser]

Any updates, @parasharrajat ?

[MelvinBot]

@youssef-lr @parasharrajat @isabelastisser this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[s77rt]

I'm not sure if we should consider this as a bug, isn't that the browser behaviour(bug) on every other site? Given the complexity of the reproduction steps and the unlikelihood to happen with a normal user I'd say do nothing. Storing file in memory seems as an overkill solution for me.

[bernhardoj]

Proposal

Updated

[bernhardoj]

I think technically it's not a bug, but more like a bad UX? On every other site, I guess when we upload a file, it will immediately upload it to the server and show the preview afterwards, Slack for example. Copying the file means we are going to read a maximum of 24mb of file.

[oesayan]

The original bug was that it is possible to upload unsupported files (e.g. 1x1px image, so there is no post-preview file check) - you can read the discussion here.

https://expensify.slack.com/archives/C049HHMV9SM/p1673424557657069

[isabelastisser]

@youssef-lr , based on the comments above, should we do nothing and close this issue?

[youssef-lr]

@isabelastisser I'm fine with closing this as well. Considering how unlikely a user run into this, and that it's only occurring on Safari.

[oesayan]

@youssef-lr hello! The original issue I reported was a bug that allowed to upload pictures smaller (1x1 for example) than requirements by swapping the filename. I am not sure why the bug was reported as file spoofing only.
The bug I reported is more about the post-preview file check.