search

dom111 / webdav-js

A simple WebDAV client written in JS for use as a bookmarklet, or integration into a web server.
MIT License
85 stars 17 forks source link

Fixes for Non-ASCII filenames #117

Open Lex-2008 opened 1 year ago

Lex-2008 commented 1 year ago

on the other side, I wonder if, instead of finding all places where decodeURI must be used, - maybe it's better to simply decodeURI filenames in server response to PROPFIND request?

dom111 commented 1 year ago

Hey there, thanks again for flagging this! It'd be great to have a test-case for this (that we can test against different server implementations if required perhaps?) Do you have a specific filename/server configuration that this fails with? If we could confirm this, write a test and then fix, that'd be ideal in my opinion!

dom111 commented 1 year ago

on the other side, I wonder if, instead of finding all places where decodeURI must be used, - maybe it's better to simply decodeURI filenames in server response to PROPFIND request?

Yeah, this makes sense. I don't know if we'd ever need the "wrong" filename. Happy for this approach too!

Lex-2008 commented 1 year ago

Hi again and sorry for the delayed response!

Thanks for suggestion regarding testcase! I now found them in tests/functional subdir, wonder if I can run them manually step-by-step.

I tried to explain filenames in commit messages, but not clear enough :) Let me try again:

First commit, Properly compare filenames before uploading:

  1. upload a file called a file.txt.
  2. Try to upload a file with the same name again.

Expected: you should be asked if you want to overwrite the file.

Currently, file is uploaded (and overwritten) without warning, and UI shows several files with same filename.

Second commit, URI-encode destination filename when renaming:

  1. Upload a file or create directory with any name, say, test.
  2. Rename it to teẞt.

Expected: it should be renamed both in UI and on server.

Currently, you get an error in console, like this:

TypeError: Request constructor: Cannot convert value in record<ByteString, ByteString> branch of (sequence<sequence> or record<ByteString, ByteString>) to ByteString because the character at index 53 has value 1099 which is greater than 255.

Third commit, Decode percent-encoded symbols in download file name

  1. Upload a file called a file.txt.
  2. Reload the page.
  3. Download it.

Expected: downloaded file is called a file.txt.

Currently, it's called a%20file.txt.

But now I also found another issue, to which I don't have a fix: Percent-encoded strings in filenames are decoded during upload:

  1. Upload a file called a%20file.txt.

Expected: uploaded file is called a%20file.txt both in UI and on the server.

Currently, it's called a file.txt both in UI and on the server.

I suspect that it's because filename is passed as-is to the server, and when server sees percent-encoded strings like %20 in URL, it decodes it to `. I guess we need to applyencodeURIorencodeURIComponent` to filename before sending it to server.

Lex-2008 commented 1 year ago

Note to self: also, "Download" links should %-encode square brackets [ in filenames. Otherwise, clicking the "Download" button works, but right_click-copy_link doesn't: it copies square brackets as-is, which later failed to be properly parsed if pasted into plaintext emails.