Closed Bug 1856431 Opened 1 year ago Closed 10 months ago

File input element broken when file name contains hash mark(#)

Categories

(Fenix :: General, defect)

Product:
Fenix
Component:
General
Version:
Firefox 118
Platform:
All
Android
Type:
defect

Tracking

(firefox119 wontfix, firefox120 wontfix, firefox121 wontfix, firefox122 verified)

Status:
VERIFIED FIXED
Milestone:
122 Branch
Tracking Flags:
Tracking Status
firefox119 --- wontfix
firefox120 --- wontfix
firefox121 --- wontfix
firefox122 --- verified

People

(Reporter: Vašek, Assigned: jackyzy823)

References

Details

(Whiteboard: [qa-triaged])

Attachments

(1 file)

[mozilla-mobile/firefox-android] Bug 1856431 - Encode file path in File URI scheme (#4188)
59 bytes, text/x-github-pull-request
Details | Review

Steps to reproduce:

  1. open a page with this HTML:
<html>
  <head>
    <title>Test</title>
  </head>
  <body>
    <input type="file" multiple />
  </body>
</html>

JSFiddle: https://jsfiddle.net/4o6stj9n/1/

  1. click on the "Browse..." button
  2. select multiple large files (tested with roughly 20 video files with sizes between 20-200MB; the more and larger the files are, the more certain the bug will be reproduced)
  3. confirm the file selection dialog

Tested on Pixel 6 with Firefox 118.1.0 (Build #2015976739), a8d4aabe56+ GV: 118.0.1-20230927232528 AS: 118.0

Also reproduced on an unknown Android phone with lower-specs (definitely less RAM)

Actual results:

When I confirm the file selection in the system dialog, the phone freezes. Then the screen turns black and after a second or so, the webpage renders. However, no files are selected.

Expected results:

The phone should instantly return to the webpage and the input element should display that some files have been chosen.

See Also: → 1807360

Thank you for your report! I was able to partially reproduce this issue on latest Nightly with Motorola G9 Plus (Android 11). The difference is that, in the end, the files are selected.
Could you please provide some details about the device used, such as the Android version, and also share a video demonstrating the issue?

Flags: needinfo?(bugzilla-mozilla.org)
Whiteboard: [qa-triaged]

I've tried to identify the root cause of my problems and I think it's completely unrelated different bug to the black screen... :) In short, I can't seem to be able to select a file with a name containing the '#' character. And some of the files I have been previously testing with contained # in their names as well.

Steps to reproduce this problem:

  1. download 100MB file from https://testfiledownload.com/ (i don't think this is important, but I have experimented with exactly that file)
  2. rename it to "#" or "100MB #1.bin"
  3. open https://jsfiddle.net/4o6stj9n/1/ in FF and try to select the downloaded file
  4. no file is actually selected

Tested on the latest Android 14 on Pixel 6 (build number UP1A.231005.007), with FIrefox 119.0 build #2015980371, the currently latest one from GPlay. Does this help or should I try to make a full replication video?

Getting back to the previous issue, the original bug was reported by a friend of mine who tried to upload lot of photos at once to my app. I doubt that the photo's names contained #, but I can't replicate his issue with normal files. I'll try to follow up with him and report it here, if I found something new.

Flags: needinfo?(bugzilla-mozilla.org)

OT: Sorry for the formatting, that should have been a horizontal line, not a heading. And I am not able to edit the comment to fix it...

So there're two bugs here?

  1. Upload multiple large files cause phone freeze or black screen for a few time
  2. Unable to select file with "#" (pound sign/ hash mark) in filename

Am i understand correctly?

For

  1. There's heavy IO in selecting file (especially in selecting multiple/large files) because it will copy file to a temp uploads folder under app's cache folder one by one. Iteration Code , and the copy code . There're some solutions: 1. do it in parallel 2. don't do it in IO thread.

  2. I suggest to open a new ticket to trace this bug better. The problem is here , "#" in the file scheme is treated as a "fragment" just like normal http urls. It will be ignored when getPath is called. The solution: Escape the file scheme : return Uri.parse("file:///${Uri.encode(temporalFile.absolutePath)}")

So there're two bugs here? [...] Am i understand correctly?

Yes, that's indeed what I think as well.

There's heavy IO in selecting file [...] because it will copy file to a temp uploads folder [...] There're some solutions: 1. do it in parallel 2. don't do it in IO thread.

Thanks for chasing down the cause. However I don't think your suggestions would fix the underlying issue. Why are the files even copied in the first place? That doesn't sound right... (Disclaimer: I don't know anything about FF internals)

Imagine a scenario, where I used a phone for a photoshoot and filled the storage with large videos and lots of photos. With copying, it would be impossible to upload the files anywhere. If I tried to upload all the raw media at once, I wouldn't have enough space to store the second copy.

What I am describing is also not a fictional scenario. That's almost exactly how I discovered the problem in the first place - by trying to upload lots of photos to a server using a web UI.

My guess would be that the only way how to fix this bug for good is to get rid of the copying in the first place. At the same time, I guess the copying is there for some reason and it might be complicated to abandon it completely. 🤷‍♂️

I suggest to open a new ticket to trace this bug better.

Sure, why not. At the same time, I've noticed you've already created a PR to fix the issue. If you believe a new ticket is still necessary, please create it by yourself. I don't know the rules here and I don't know if I should still do it when the PR is linked here.

Thanks for chasing down the cause. However I don't think your suggestions would fix the underlying issue. Why are the files even copied in the first place? That doesn't sound right... (Disclaimer: I don't know anything about FF internals)

You're right.

Imagine a scenario, where I used a phone for a photoshoot and filled the storage with large videos and lots of photos. With copying, it would be impossible to upload the files anywhere. If I tried to upload all the raw media at once, I wouldn't have enough space to store the second copy.

Yes, and also i found the copied file will not automatically be cleaned. See bug 1860472.

My guess would be that the only way how to fix this bug for good is to get rid of the copying in the first place. At the same time, I guess the copying is there for some reason and it might be complicated to abandon it completely. 🤷‍♂️

I'm also curious about the reason of this design.

Sure, why not. At the same time, I've noticed you've already created a PR to fix the issue. If you believe a new ticket is still necessary, please create it by yourself. I don't know the rules here and I don't know if I should still do it when the PR is linked here.

I'll rename this ticket to reflect the bug of filename , and keep the bug 1807360 to trace file upload design issue.

I've thought about the design of file upload. It would looks better if flow likes:

  1. Fenix pass content:// scheme url to GeckoView
  2. Geckoview get displayname from content:// scheme url.
  3. Geckoview pass displayName and content:// scheme url to FilePickerDelegate.sys.mjs
    3.5 use displayName for displaying in the web page
  4. When really reading file (like form submit or File API) , FilePickerDelegate.sys.mjs ask Geckoview to return a file pointer/ file stream for reading ( maybe using contentresoler.openurlinstream or openFileDescriptor )
Summary: File input element broken with multiple large files → File input element broken when file name contains hash mark(#)

Thank you for your prompt response and clarifications. I was able to reproduce this issue on the latest Firefox 119.0 build and also on the latest Nightly version- 120.0a1 from 23.10.2023, using a Google Pixel 7 PRO (running Android 14).

Severity: -- → S3
status-firefox119: --- → affected
status-firefox120: --- → affected

The bug has a release status flag that shows some version of Firefox is affected, thus it will be considered confirmed.

Status: UNCONFIRMED → NEW
Ever confirmed: true

Authored by https://github.com/jackyzy823
https://github.com/mozilla-mobile/firefox-android/commit/c14d163625def1569448d335612466e481febf91
[main] Bug 1856431 - Encode file path in File URI scheme

Status: NEW → RESOLVED
Closed: 10 months ago
status-firefox122: --- → fixed
Flags: qe-verify+
Resolution: --- → FIXED
Target Milestone: --- → 122 Branch

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit BugBot documentation.

status-firefox121: --- → affected

Verified as fixed on the latest Fenix Nightly 122.0a1 from 12/7 with Sony Xperia Z5 Premium (Android 7.1.1), and Google Pixel 6 (Android 14).

Status: RESOLVED → VERIFIED
status-firefox122: fixed → verified
Flags: qe-verify+
See Also: → 1860472
Duplicate of this bug: 1813752
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: