fix for upstream/#116

[stefanofornari]

Issue
#116

Description of changes:
Fix for #116

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[markjschreiber]

Hmm,

Fun fact, S3 URIs aren't strictly speaking URIs because they can contain characters that are illegal in URIs. For example in an S3 bucket you can create

"file with spaces.txt"
"file+with+spaces.txt"
"file%20with%20spaces.txt"

All of these will be unique files and their respective URIs are:

s3://bucket/file with spaces.txt
s3://bucket/file+with+spaces.txt
s3://bucket/file%20with%20spaces.txt

And S3 will not consider these to be the same URI (which is fun)

As you have spotted, implementations of the Path interface do need to return URI and that class will require URL encoding. URI is final (as it should be) so we cannot override this.

I need to add a note into the README.md to explain the potential differences that people may see and advise that, where possible people should where possible avoid using characters that need to be URL encoded so they don't see these conflicts.

[markjschreiber]

Can you add a note in the Java doc how this URI might not be equal to the "S3 URI" that will be seen in the S3 console due to the need for the URI to be encoded to avoid illegal characters?

[markjschreiber]

Thanks for this. If you can add a note to the JavaDocs about how true URIs and S3 URIs might differ then we can merge this.

Having a similar note in the README.md would also be great.

[stefanofornari]

Fun fact, S3 URIs aren't strictly speaking URIs because they can contain characters that are illegal in URIs. For example in an S3 bucket you can create

ah interesting. we need to think a little bit more then. Paths.get(URI.create("s3://bucket/file with space")); breaks because URI.create() fails. We need to figure out then how Paths.get(uri) translates into a S3Path object and make sure it works if the urls is encoded.

[stefanofornari]

I amended the javadoc, @markjschreiber does it work?
What I think we should be clear about is that a URI has a defined syntax, specified in one or more RFCs. It is somehow improper to say that "s3://something with space" is a S3 URI (of course we can use that expression for simplicity).
Anyway, like I tried to say in the comment, the current implementation is coherent:

S3Path p = (S3Path)Paths.get("s3://mybucket/with+blank+and+%25"); // -> s3://mybucket/with blank and %
String s = p.toString; // -> /mybucket/with blank and %
URI u = p.toUri(); --> // -> s3://mybucket/with+blank+and+%25
...
String s = p.getFileSystem().get("with space").toString(); // -> /with space 

[markjschreiber]

Excellent! Thanks for the contribution