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

S3Path.from_uri does not respect urlencoded URIs #150

Closed
schneidemar opened this issue Oct 23, 2023 · 4 comments
Closed

S3Path.from_uri does not respect urlencoded URIs #150

schneidemar opened this issue Oct 23, 2023 · 4 comments

Comments

@schneidemar
Copy link
Contributor

When creating an S3Path object with a path, which contains characters not representable by URIs, the to_uri function apparently call urlencode to encode those characters. This is fine. But when using this URI to create a new instance of S3Path, it should call urldecode, to decode the URI to the correct path. This is currently not done and results in errors of S3 being not able to find the key (of course, the key is wrong.)
Example

path = S3Path.from_uri("s3://bucket/test/2023-09-10T00:00:00.000Z.txt")
# S3Path('/bucket/test/2023-09-10T00:00:00.000Z.txt'), still correct

path.as_uri()
# s3://bucket/test/2023-09-10T00%3A00%3A00.000Z.txt  now urlencoded

S3Path.from_uri(path.as_uri())
# S3Path('/bucket/test/2023-09-10T00%3A00%3A00.000Z.txt')  still encoded, S3 does not find this file

Workaround is of course to manually call urldecode on the string first, but IMO S3Path.from_uri should handle this, as it also calls urlencode when creating the URI.

@maresb
Copy link
Contributor

maresb commented Oct 23, 2023

See #77 for context. I'm not sure if we converged on the correct solution or not.

@liormizr liormizr mentioned this issue Dec 5, 2023
@liormizr
Copy link
Owner

liormizr commented Dec 6, 2023

I merged a fix to master
I'll update here after deploying new version

@liormizr
Copy link
Owner

New Version deployed 0.5.1 with this card fix

@maresb
Copy link
Contributor

maresb commented Dec 18, 2023

Seems to be working in the new version, thanks!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants