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

Cannot edit or download file with comma on Web #8361

Closed
streaminganger opened this issue Feb 4, 2024 · 9 comments
Closed

Cannot edit or download file with comma on Web #8361

streaminganger opened this issue Feb 4, 2024 · 9 comments
Assignees
Labels
Platform:Web Priority:p2-high Escalation, on top of current planning, release blocker Type:Bug
Milestone

Comments

@streaminganger
Copy link

streaminganger commented Feb 4, 2024

Describe the bug

I have a file New file,.txt with a comma in the file name. Using the web client I'm unable to edit or download.

Steps to reproduce

  1. Use web client running ocis backend
  2. Create text file with comma
  3. Edit and download fails
  4. Repeat with file rename without comma to see that it works again

Expected behavior

Edit and download to work
image

Actual behavior

Edit comma file:
image

TypeError: Cannot read properties of undefined (reading 'status')
    at Object.getFileContents (index.html-ZHQpx7wf.mjs:1:42232)
Failed to load resource: net::ERR_RESPONSE_HEADERS_MULTIPLE_CONTENT_DISPOSITION

Download comma file:
image

Setup

Docker compose using mostly default settings from examples in ocis repo.
5.0.0-rc.3

Additional context

Edit and download of same comma file works on desktop client and android client

Update: #8361 (comment) - this issue is Chromium specific as found by Jan

@phil-davis
Copy link
Contributor

@saw-jan are there API and/or webUI acceptance tests that cover common "interesting characters" in file and folder names?

If the API tests pass but web tests fail, then that will narrow done where the problem might be.

@saw-jan
Copy link
Member

saw-jan commented Feb 5, 2024

are there API and/or webUI acceptance tests that cover common "interesting characters" in file and folder names?

We do have API test for file upload/download having comma in its name

Scenario Outline: upload a file with comma in the filename and check download content
Given using <dav-path-version> DAV path
When user "Alice" uploads file with content "file with comma" to <file_name> using the WebDAV API
Then the HTTP status code should be "201"
And the content of file <file_name> for user "Alice" should be "file with comma"
Examples:
| dav-path-version | file_name |
| old | "sample,1.txt" |
| old | ",,,.txt" |
| old | ",,,.," |
| new | "sample,1.txt" |
| new | ",,,.txt" |
| new | ",,,.," |
@skipOnRevaMaster
Examples:
| dav-path-version | file_name |
| spaces | "sample,1.txt" |
| spaces | ",,,.txt" |
| spaces | ",,,.," |

And we also have webUI test but that doesn't check the file content in the editor.
https://github.com/owncloud/web/blob/59b5bbe6d77af2a530a224abd351f738d2237966/tests/acceptance/features/webUICreateFilesFolders/createFile.feature#L19-L22
and https://github.com/owncloud/web/blob/59b5bbe6d77af2a530a224abd351f738d2237966/tests/acceptance/features/webUIFiles/download.feature#L24-L28

@phil-davis
Copy link
Contributor

phil-davis commented Feb 5, 2024

Those acceptance tests are passing, so we can upload and download file names that have a comma in them - good.
The web UI seems to have some problem with it. Either it is doing a different sequence of API requests than our API acceptance tests (and coming across some problem in those API requests), or the web code has a problem.

@saw-jan can someone be assigned to make a test scenario that demonstrates this issue.

@saw-jan
Copy link
Member

saw-jan commented Feb 5, 2024

Docker compose using mostly default settings from examples in ocis repo. 5.0.0-rc.3

@streaminganger Which docker-compose file did you use? I couldn't reproduce it with these versions:

ownCloud Web UI 8.0.0-rc.2 
Infinite Scale 5.0.0-rc.3 Community 

@micbar
Copy link
Contributor

micbar commented Feb 5, 2024

@JammingBen @kulmann FYI

@kulmann kulmann added the Priority:p2-high Escalation, on top of current planning, release blocker label Feb 5, 2024
@kulmann kulmann moved this from Qualification to Prio 2 in Infinite Scale Team Board Feb 5, 2024
@AlexAndBear AlexAndBear self-assigned this Feb 5, 2024
@AlexAndBear
Copy link
Contributor

AlexAndBear commented Feb 5, 2024

I can't reproduce the download and viewing issue with the current version of ocis.

But I can observe, that we have issues with the filename, while downloading a file with the name si,claro.txt will result in downloading the file as si not even with the extension if using chrome, but works with firefox

@micbar from my understanding we can't fix this in web, ocis needs to provide the Content-Disposition Header with the file name, which is currently missing https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition

image

It could be possible that you need to wrap the filename in double quotes in order to work with chrome: https://stackoverflow.com/questions/49003459/file-download-error-only-in-file-name-with-comma

@streaminganger
Copy link
Author

streaminganger commented Feb 5, 2024

I can't reproduce the download and viewing issue with the current version of ocis.

But I can observe, that we have issues with the filename, while downloading a file with the name si,claro.txt will result in downloading the file as si not even with the extension if using chrome, but works with firefox

@micbar from my understanding we can't fix this in web, ocis needs to provide the Content-Disposition Header with the file name, which is currently missing https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition

It could be possible that you need to wrap the filename in double quotes in order to work with chrome: https://stackoverflow.com/questions/49003459/file-download-error-only-in-file-name-with-comma

Oh you are right, the issue only happens on chromium based browsers.

With firefox I'm able to edit and download said file
image

@dragonchaser
Copy link
Member

The Content-Disposition header is present, it seems like there were quotes missing from the UTF-8 part of the header PR above should fix it.

@AlexAndBear
Copy link
Contributor

AlexAndBear commented Feb 6, 2024

Which resulted in chrome or chromium disposed the the header sometimes, kind of a weird behavior 🤷‍♀️

With the bugfix @dragonchaser provided, the issue won't be present in any browser anymore.

Closing when merged.

Thanks for the report @streaminganger 💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform:Web Priority:p2-high Escalation, on top of current planning, release blocker Type:Bug
Projects
Archived in project
Development

No branches or pull requests

7 participants