Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Download result in 404 if file name contains hash(a.k.a. #) (also when using the webdav path) #10810

Closed
mmis1000 opened this issue Apr 21, 2024 · 14 comments
Labels
Priority:p3-medium Normal priority Type:Bug Something isn't working

Comments

@mmis1000
Copy link

mmis1000 commented Apr 21, 2024

Describe the bug

Download and the webdav in detail from web won't work if the file or folder contains # in its file name.

Steps to reproduce

  1. upload a file that contains # in file name or rename a existing one to a name that contains #
  2. download it by click the filename
  3. or download with webdav path

Expected behavior

You downloaded the file

Actual behavior

Error occurred and 404 show up in web requests

Setup

All default setup.
Version 5.0.2

Additional context

it occurred to me when I am trying to upload the osu beamap pack Beatmap Pack #1114.7z which happens to have a hash in its filename

edited

You probably shouldn't treat file path as url path directly here

/valid/file#1.zip is a valid file path but not a valid url path, # need to be encoded as %23 or it will be treated as url hash.

/valid%20.txt is also a valid path but download won't work either because it will be decoded as /valid .txt if you do not encode before use it as url path.

@mmis1000 mmis1000 added the Type:Bug Something isn't working label Apr 21, 2024
@kulmann kulmann added the Priority:p3-medium Normal priority label Apr 22, 2024
@kulmann kulmann moved this from Qualification to Prio 3 or less in Infinite Scale Team Board Apr 22, 2024
@kulmann
Copy link
Contributor

kulmann commented Apr 22, 2024

Hey @mmis1000 - thank you for the bug report! I can reproduce this with your steps on ocis 5.0.2. It's not reproducible on current web master. We need to check what exactly happened to have this fixed and if we can easily backport the fix to web v8 / ocis v5. We'll keep you posted here.

@phil-davis
Copy link
Contributor

phil-davis commented Apr 22, 2024

@PrajwolAmatya @Salipa-Gurung
please ask someone to check the test coverage of this.
IMO we need to know that there won't be regressions for the cases of file names that have special characters that need to be encoded.
(it is easy to accidentally introduce problems with this sort of thing at various levels of the end-to-end software layers)

@kulmann
Copy link
Contributor

kulmann commented Apr 22, 2024

Quite weird behaviour: when trying to download a file asdf#asdf.txt:

  • we do a HEAD request to check if the file really exists before we initiated the url signing for the download
  • axios receives the same download url in both web stable-8.0 and web master ([...]/asdf#asdf.txt,
  • the HEAD goes to [...]/asdf in stable-8.0 (incorrect, cut off before the #) and to [...]/asdf#asdf.txt in master (correct)

Now my first thought was, that this is a dependency issue with axios, but that is the same version on both branches: 1.6.8. Need to debug further.

@kulmann
Copy link
Contributor

kulmann commented Apr 22, 2024

Meh, ok, in master we're encoding the path and in stable-8.0 we don't. 🤦🏻‍♂️

@kulmann
Copy link
Contributor

kulmann commented Apr 22, 2024

@mmis1000 I've created a pull request to fix the issue for the next ocis v5 release.

Can you please tell me what you mean by or download with webdav path? The webdav path in the Details right sidebar panel is showing correct for me. Are you referring to a download in a third party webdav client application?

@mmis1000
Copy link
Author

mmis1000 commented Apr 22, 2024

@mmis1000 I've created a pull request to fix the issue for the next ocis v5 release.

Can you please tell me what you mean by or download with webdav path? The webdav path in the Details right sidebar panel is showing correct for me. Are you referring to a download in a third party webdav client application?

圖片

I mean the one in detail panel.
It is https://xxx.example.com/dav/spaces/428c5aa0-5305-4980-92ea-730c478dfb05$411b8c1e-840c-4777-a5e0-1d9b995e1c93/Beatmap Pack #1114.7z in this case, which don't work.

@kulmann
Copy link
Contributor

kulmann commented Apr 22, 2024

I mean the one in detail panel. It is https://xxx.example.com/dav/spaces/428c5aa0-5305-4980-92ea-730c478dfb05$411b8c1e-840c-4777-a5e0-1d9b995e1c93/Beatmap Pack #1114.7z in this case, which don't work.

Thanks for clarifying! This URL is not for a browser download. It is for webdav clients like e.g. cyberduck/mountainduck, dolphin, etc. For a browser download you need to use the Download file action from the right click context menu (or for files which don't have a viewer/editor the left click will start the download as well).

@kulmann
Copy link
Contributor

kulmann commented Apr 22, 2024

Fixed via #10813

@kulmann kulmann closed this as completed Apr 22, 2024
@github-project-automation github-project-automation bot moved this from Prio 3 or less to Done in Infinite Scale Team Board Apr 22, 2024
@mmis1000
Copy link
Author

I mean the one in detail panel. It is https://xxx.example.com/dav/spaces/428c5aa0-5305-4980-92ea-730c478dfb05$411b8c1e-840c-4777-a5e0-1d9b995e1c93/Beatmap Pack #1114.7z in this case, which don't work.

Thanks for clarifying! This URL is not for a browser download. It is for webdav clients like e.g. cyberduck/mountainduck, dolphin, etc. For a browser download you need to use the Download file action from the right click context menu (or for files which don't have a viewer/editor the left click will start the download as well).

It will still be affected though, cause you can add any dav directory to windows explore or other tools. And folder name that contain '#', '%20' ...etc will break.

@PrajwolAmatya
Copy link
Contributor

PrajwolAmatya commented Apr 23, 2024

With the fix in #10813, the file/folder containing # in its name is downloaded successfully, the test is covered in PR #10812.

I tried with other cases as well, to download file with a "%" in its name, text%20.txt. The download is successful but the downloaded file name does not contain the %20 in its name.
The downloaded filename contains space instead of %20, text .txt.

Screenshot

image

Downloaded file:
image

I also tried to download file with filename text?.txt and the downloaded filename is text_.txt

Tested on latest web master and also in web stable-8.0.

Is this the expected behavior?
CC. @kulmann @ScharfViktor

@kulmann
Copy link
Contributor

kulmann commented Apr 23, 2024

I mean the one in detail panel. It is https://xxx.example.com/dav/spaces/428c5aa0-5305-4980-92ea-730c478dfb05$411b8c1e-840c-4777-a5e0-1d9b995e1c93/Beatmap Pack #1114.7z in this case, which don't work.

Thanks for clarifying! This URL is not for a browser download. It is for webdav clients like e.g. cyberduck/mountainduck, dolphin, etc. For a browser download you need to use the Download file action from the right click context menu (or for files which don't have a viewer/editor the left click will start the download as well).

It will still be affected though, cause you can add any dav directory to windows explore or other tools. And folder name that contain '#', '%20' ...etc will break.

Yep, you're right! Made #10817 to fix that as well. Tested it successfully in Cyberduck.

@kulmann
Copy link
Contributor

kulmann commented Apr 23, 2024

Is this the expected behavior? CC. @kulmann @ScharfViktor

Seems like we can't enforce other filenames. I tried to escape the file names as well, but the resulting file names remained unchanged.

@saw-jan
Copy link
Member

saw-jan commented Apr 23, 2024

Is this the expected behavior? CC. @kulmann @ScharfViktor

Seems like we can't enforce other filenames. I tried to escape the file names as well, but the resulting file names remained unchanged.

So it's a browser thing and expected ones?

@kulmann
Copy link
Contributor

kulmann commented Apr 23, 2024

So it's a browser thing and expected ones?

yep

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:p3-medium Normal priority Type:Bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

5 participants