Skip to content

[#203] Handle special characters in request targets#211

Closed
alanking wants to merge 3 commits into
irods:mainfrom
alanking:203.m
Closed

[#203] Handle special characters in request targets#211
alanking wants to merge 3 commits into
irods:mainfrom
alanking:203.m

Conversation

@alanking
Copy link
Copy Markdown
Contributor

@alanking alanking commented Mar 2, 2026

Addresses #203

Tests are very basic and are based on existing putobject tests, but I'm willing to do a more thorough sweep. Also confirmed working with S3 Browser and Cyberduck.

Comment thread core/src/session.cpp
Comment on lines +50 to +51
// If no query parameters delimiter is in the target, then the target and the path are one and the same.
const auto encoded_path = (params_pos != std::string::npos) ? target.substr(0, params_pos) : target;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an optimization in which target does not need to be copied here. It makes the code slightly messier, but might be worth doing.

@alanking
Copy link
Copy Markdown
Contributor Author

alanking commented Mar 2, 2026

Hmmm... actually, Cyberduck appears to still be having problems with the +. Sorry, need to put this in draft for now.

@alanking alanking marked this pull request as draft March 2, 2026 17:21
alanking added 3 commits March 3, 2026 17:21
Some clients request URL-encoded object Keys, so these test putting and
listing objects with reserved characters which would be URL-encoded if
needed. The default encoding type is UTF-8.
Whenever a request is parsed, it encodes special characters so that
different parts of the URL can be parsed. While this is helpful and
good, these special characters need to be decoded in the target's path
so that the object or prefix in question can be referenced properly.
@alanking
Copy link
Copy Markdown
Contributor Author

alanking commented Mar 3, 2026

Added some tests for ListObjectsV2 as well. Again, I think this whole repo needs a refactor as well as a more comprehensive set of tests in addition to testing the S3 clients, but this is all in an effort to get basic functionality for popular Windows GUIs for S3.

@alanking alanking marked this pull request as ready for review March 3, 2026 22:25
@alanking
Copy link
Copy Markdown
Contributor Author

alanking commented Mar 3, 2026

Gah, sorry for all the noise. I'm going to close this for now. Found some more problems.

@alanking alanking closed this Mar 3, 2026
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

Successfully merging this pull request may close these issues.

1 participant