amahi / amahi-anywhere-fs

Amahi Anywhere file server
GNU General Public License v3.0
12 stars 10 forks source link

when a folder has a + in the name, the server cannot find content in it #21

Closed cpg closed 5 years ago

cpg commented 5 years ago

looks like there is some issue with escaping folder names or something like that.

when the server comes across a folder that has a + sign, the contents are returned as 404. it's browseable, however.

we have a hole in our testing, looks like we tested file names but not folder names?

vishwasmittal commented 5 years ago

This problem is due to unescaping while parsing the URL. In the file net/url there is a function QueryUnescape which calls unescape which is responsible for converting the + to < > (space).

vishwasmittal commented 5 years ago

This can easily be checked by making 2 folders named test folder and test+folder and whenever a request on /files?s=shareName&p=/test+folder is made, it will return data only for test folder.

csoni111 commented 5 years ago

The functionality in net/url which does the unescaping of query params was never the problem. It's absolutely correct to convert + to <space> and handling other special characters like =, ? etc. in the url.

For whenever the request is made for /files?s=shareName&p=/test+folder the correct behaviour should be that it returns the contents of test folder only and not test+folder. If one wants to access the contents of test+folder, then the request should be made for /files?s=shareName&p=/test%2Bfolder.

The difference between these two will make my point more clear:

https://www.google.com/search?q=foo+bar

and

https://www.google.com/search?q=foo%2Bbar

csoni111 commented 5 years ago

So according to me, it's up to the client to escape the query params before making the request to the server. (ie. the iOS/Android client should convert test+folder to test%2Bfolder). There's nothing to be done on the server side for this.

csoni111 commented 5 years ago

Retrofit's \@Query annotation used here does this query param escaping automatically. So we haven't had this issue with the Android client. Similar thing needs to be added to the iOS client too.

cpg commented 5 years ago

Fixed by this PR in the client. Thanks for the research @csoni111