search

mickael-kerjean / filestash

🦄 A file manager / web client for SFTP, S3, FTP, WebDAV, Git, Minio, LDAP, CalDAV, CardDAV, Mysql, Backblaze, ...
https://www.filestash.app/
GNU Affero General Public License v3.0
10.55k stars 782 forks source link

Download filename issue on Firefox: "cat" file #255

Open lchanouha opened 4 years ago

lchanouha commented 4 years ago

On Firefox (74 Windows, 75 Linux), downloaded filename have an invalid file name: "cat". It works well on latest version of Safari and Chrome.

The response headers of the download query seems incomplete to me (or there is a JS treatment that i'm not aware of): GET: https://url.fqdn/api/files/cat?path=%2Fxxxx%2Fsetup.tgz

HTTP/1.1 200 OK
Accept-Ranges: bytes
Cache-Control: no-cache
Content-Security-Policy: default-src 'none'; img-src 'self'; media-src 'self'; style-src 'unsafe-inline'; font-src data:
Content-Type: application/octet-stream
Set-Cookie: download=; Path=/; Max-Age=0
Strict-Transport-Security: max-age=31536000; includeSubDomains; preload
X-Content-Type-Options: nosniff
X-Xss-Protection: 1; mode=block
Date: Thu, 23 Apr 2020 13:36:14 GMT
Transfer-Encoding: chunked

Maybe adding the Content-Disposition header could fix the problem for non-webkit browser ? Content-Disposition: attachment; filename="filename.jpg"

Regards, Louis

pagdot commented 4 years ago

I can replicate this issue

mickael-kerjean commented 4 years ago

There seem to be issues in how Firefox in handling the download HTML attribute.

lchanouha commented 4 years ago

i didn't knew download attribute

w3schools demo works with Firefox & Chrome: https://www.w3schools.com/TAGS/tryit.asp?filename=tryhtml5_a_download2 I tried to remove the JS event without success. What is the diff between two envs ?

mickael-kerjean commented 4 years ago

What is the diff between two envs ?

No obvious difference. I did play around and haven't been able to pin point why it gives different results yet

lchanouha commented 4 years ago

In the specs the browser adds the extension of file, probably with content type. I think that using Content-Disposition is better for a file manager with unpredictable, custom and exotics file extensions.

You could add the API: /api/files/cat?path=%2Fxxxx%2Fsetup.tgz&download=true for example.

mickael-kerjean commented 4 years ago

Content Disposition sounds like a good option, but a lot of things needs to be verified to ensure it doesn't introduce more issues than it solves. Before going there, we first need to get to know what's going wrong with the download attribute in the way it's used on Firefox as it works fine on Chrome and its derivatives

lchanouha commented 4 years ago

I would say invalid (generic) Content-Type header. So browser cannot guess file extension.

jimbojim commented 4 years ago

I'm seeing this issue in Firefox 76 but only going through an nginx proxy. Direct to filestash on 8334, the issue does not appear.

mickael-kerjean commented 4 years ago

That's quite weird, the behavior on Firefox 75 is still with a file named cat

mickael-kerjean commented 4 years ago

The PR above fix the problem but it's quite a shitty hack. The root cause seem to be a bug in how Firefox handle the download attribute while using service worker.

If someone has time, the better approach is to fix the Firefox code but considering I'm not a Firefox user, this sounds way too much hassle.

lchanouha commented 4 years ago

Bonjour Mickael, I would like to test it. I did

 docker build -t machines/filestash .

But actually the Dockerfile download app from your private server and does not use the git repo.

curl --resolve downloads.filestash.app:443:94.23.200.66 -s https://downloads.filestash.app/latest[...]

is there a way to have an image with latest commits ? If there are already included, the fix isn't working. Thanks

mickael-kerjean commented 4 years ago

At the moment, the docker layer is a convenience layer for distribution, it doesn't do the build, the CI environment does. To work with the above PR you need to [create your own build] (https://github.com/mickael-kerjean/filestash/blob/master/CONTRIBUTING.md#building-from-source).

Ultimately, there's a choice now between:

lchanouha commented 4 years ago

OK, i will try. We don't need offine service. I would still add a third choice:

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

kisst commented 1 year ago

Still an active issue.

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

kisst commented 1 year ago

Hi stale bot! 👋🏻 Yep it's still a problem...

mickael-kerjean commented 1 year ago

This issue to my knowledge has been fix a long time ago. I just opened a random storage from Firefox and did download the file and it did show the right name, not sure what you see is a problem, can you elaborate?

kisst commented 1 year ago

Just tested with a simple zip file and got cat.zip as a download on Mozilla/5.0 (Android 12; Mobile; rv:109.0) Gecko/114.0 Firefox/114.0 But I can confirm that on Desktop version it worked fine with the same file Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:109.0) Gecko/20100101 Firefox/114.0

mickael-kerjean commented 1 year ago

I tried that and got the correct filename, can you send all the steps you are going through so I can reproduce this?

kisst commented 1 year ago
kisst commented 1 year ago

btw tested on FF mobile with png and others too the file name still stays cat 🐱

mickael-kerjean commented 1 year ago

I don't have access to such device, feel free to make a PR

sevmonster commented 9 months ago

So this is only a mobile issue at this point? I noticed this as well on Mull (Firefox fork) on Android. Desktop doesn't exhibit issues. Interesting that both codebases had the bug but only one was fixed 🤷

I wouldn't begin to know where to debug this issue, but it is still present.

mickael-kerjean commented 9 months ago

Just a mind dump of how this could be worked on by anyone who have that kind of device:

  1. create a dummy page with some plain html that would replicate that issue
  2. post the result here
  3. experiment with things like:
    • make some tweak on the html side to make it work, publish your findings here
    • make some tweak on the js side
    • lastly if nothing works, try to pinpoint the RFC that the behavior violates so we actually understand what is going on as doing a server fix to fix a weird issue on a device shouldn't mandatory.