Update ReadMe to include Uri.encode in examples

[nextcloud]

Update ReadMe to include Uri.encode in examples

[stefan-niedermann]

Summarizing:

• The caller is responsible to encode the URI components.
• The SSO lib can not handle this because it does not have path segments but only takes the complete URL.
• The README.md already makes use of Uri#encode at all parts

I'll therefore close this issue, feel free to reopen in case of new information or ideas 🙂

[burningthumb]

Just to be clear, I know I can use:
url.replaceAll(" ", "%20");

I'm just concerned if there are any other special characters that are not handled since a full URLEncode appears to fail so is it something in between or is it just spaces?

[tobiasKaminsky]

We are using https://github.com/nextcloud/android-library/blob/3a8a48a3a82e2cc87496eb6a8392ba21a15af296/src/main/java/com/owncloud/android/lib/common/network/WebdavUtils.java#L70-L83 to encode path.

[burningthumb]

That encoding should work.

But in order to download a file with spaces in the name I need to use:

String l_urlPath = "/remote.php/webdav" + a_remoteFilePath.replaceAll(" ", "%20");

And then:

NextcloudRequest nextcloudRequest = new NextcloudRequest.Builder()
                        .setMethod("GET")
                        .setUrl(l_urlPath)
                        .build();

l_inputStream = mNextcloudAPI.performNetworkRequest(nextcloudRequest);

Where a_remoteFilePath is equal to something like /folder/AAA Read Me.txt

Otherwise the exception is thrown.

Has anyone had success getting a file name that includes unescaped spaces?

[tobiasKaminsky]

Has anyone had success getting a file name that includes unescaped spaces?

You always have to escape spaces. WebDav boils in the end down to regular urls and there are no whitespaces allowed.

e.g. this is for uploading / downloading a file "com android chorme 20377014317.split.config.en.apk" in folder "__saf":

http://10.0.2.2/nc/remote.php/webdav/___saf/com%20android%20chrome%20377014317.split.config.en.apk

From help:

Encodes characters in the given string as '%'-escaped octets using the UTF-8 scheme. Leaves letters ("A-Z", "a-z"), numbers ("0-9"), and unreserved characters ("_-!.~'()*") intact. Encodes all other characters with the exception of those specified in the allow argument.

[burningthumb]

Yes I understand space must always be encoded. The question is why do I need to encode them in my code and the library does not do it. For example if I do these:

        String l_test1 = URLEncoder.encode ("folder/This is a test.txt");
        String l_test2 = Uri.encode("folder/This is a test.txt");
        Log.i(l_test1, l_test2);

The log output is this:

/folder%2FThis+is+a+test.txt: folder%2FThis%20is%20a%20test.txt

So I just don't understand why the I need to replace the spaces to make it work.

Can you tell me where in the code the library encodes the URL ?

[burningthumb]

So what I am trying to understand is that since Uri.encode already encodes spaces, and the library is calling Uri.encode, why do I need to replace the spaces using replace( " " , "%20") before setUrl in the NextcloudRequest.Builder because Uri.encode should do it (or maybe the builder should do it).

I would have expected either I need to encode the URL before setting it or I don't need to encode it because the library handles it but I just never expected I need to handle only the spaces explicitly in my code. It just seems odd.

[burningthumb]

So I think there is a broader issue here. Say I have a folder on my drive and it is named folder%20name, in this case the %20 is part of the file name not an encoded space. Running Url.encode would produce folder%2520name where the % is encoded as %25. But I'm getting an HTTP 404 error because this is being incorrectly interpreted as folder name.

[burningthumb]

OK I think now I have understood your answer. When I do

NextcloudRequest nextcloudRequest = new NextcloudRequest.Builder()
                        .setMethod("GET")
                        .setUrl(l_urlPath)
                        .build();

It actually needs to be:

NextcloudRequest nextcloudRequest = new NextcloudRequest.Builder()
                        .setMethod("GET")
                        .setUrl(Uri.encode(l_urlPath,"/"))
                        .build();

So I am responsible to encode the URL but must exclude the slash (/) from being encoded.

I will update the read me file to make this obvious to people like me.

[tobiasKaminsky]

The reference I linked to is nextcloud-android-library, which is not used in SSO.
So, yes, you have to take care of the url by yourself.
Sorry that I answer only now, but it was weekend ;-)

Thank you for updating the readme 🎉

[burningthumb]

No worries. I thought I may need to encode the URL but what I did not understand was that I also needed to allow the slashes. I had just done Uri.encode(theURL), instead of Uri.encode(theURL,"/") and so the exception was being thrown. I updated the README.md in my fork to include the Uri.encode that allows slashes in the examples.

[David-Development]

@burningthumb Thanks Rob for pointing this out and for including it in your PR.

I think your question regarding "why do I have to do this and why isn't the sso library taking care of it" is a valid question. I didn't encounter any cases where this was a problem during the development of this library as my endpoints never contained any whitespaces (for the nextcloud news app).

@tobiasKaminsky I think we should do the url encoding in the sso library when calling setUrl(...) as otherwise the devs have to remember to do the encoding themself and they might run into issues that take a long time to debug if they forget to do it and users start complaining.
What do you think? Are there any edge cases that I can't think of right now where this would break things?

[burningthumb]

There is a possible problem of a double encode if you do it in the setUrl(...) method. In other words if I do the encode and then you encode it again it can break because of the double call to encode. If you do add it to the library I suggest leaving (and deprecating) setUrl(...) and creating a new method like encodeAndSetUrl(...) or parseUrl(...) or something like that anyway where its more explicit that the method is doing the encode (and people can look at it to see how the encode is being done). Most people should know about encoding Urls, it really was the allowing of the slash that caught me since its an odd mix of encode "except" one character...

[tobiasKaminsky]

Nice idea 👍

[stefan-niedermann]

Wow, lot's of text. I read the thread and to summarize the key question is:

Should NextcloudRequest.Builder#setUrl() automatically perform Uri.encode() or is it up to the 3rd party apps developer to ensure the given URI is encoded?

It seems like a quick fix (even considering some breaking changes), so I just tried to replace

ncr.url = url;

with

ncr.url = Uri.encode(url);

This broke the sample app (using Retrofit), because also the / have been replaced: %252Focs%252Fv2.php%252Fcloud%252Fusers%252F%257Bsearch%257D

My recommendation is to keep the behavior as it is, but add some JavaDoc to NextcloudRequest.Builder#setUrl() to provide a quick documentation about the behavior.