-
Notifications
You must be signed in to change notification settings - Fork 6
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
Fix issue with host header and canonicalURI #9
Conversation
There were actually two issues that broke authenticated buckets. The first was that we weren't properly setting the host header. The second was that we weren't generating a valid canonical URI that gets signed and sent as a part of the Authorization header. The solution to both of these was to parse the hostUrl more carefully, and in only one spot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good except for wanting an added comment at the top of the function. Mostly to slowly update the documentation for this code as changes happen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[lint] reported by reviewdog 🐶
// Takes in the configured `s3.service_url` and uses the bucket/object requested | |
// to generate the virtual host URL, as well as the canonical URI (which is the | |
// path to the object). | |
bool AmazonRequest::parseURL(const std::string &url, const std::string &bucket, | |
const std::string &object, std::string &host, | |
std::string &path) { | |
auto i = url.find("://"); | |
if (i == std::string::npos) { | |
return false; | |
} | |
// protocol = substring( url, 0, i ); | |
auto j = url.find("/", i + 3); | |
if (j == std::string::npos) { | |
host = bucket + "." + substring(url, i + 3); | |
path = "/" + object; |
@@ -34,7 +34,11 @@ std::string AmazonRequest::canonicalizeQueryString() { | |||
return AWSv4Impl::canonicalizeQueryString( query_parameters ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[lint] reported by reviewdog 🐶
return AWSv4Impl::canonicalizeQueryString( query_parameters ); | |
return AWSv4Impl::canonicalizeQueryString(query_parameters); |
There were actually two issues that broke authenticated buckets. The first was that we weren't properly setting the host header. The second was that we weren't generating a valid canonical URI that gets signed and sent as a part of the Authorization header. The solution to both of these was to parse the hostUrl more carefully, and in only one spot.
Testing this is a bit of a challenging setup, so I'll try to loosely sketch this out here. This is from my notes, and it assumes you're running in a linux container as root. The container will need all the tools for building c++, as well as xrootd and the minio server binary. You'll also need a non-root user to run the xrootd process under. I'm also assuming you have some experience dealing with XRootD and all its funkiness.
After building the plugin, you can test by using this xrootd config:
Run the plugin under your non-root user:
xrootd -c <configfile name>
. You should get through all of the xrootd initialization.Phew, you made it through all that! Give yourself a pat on the shoulder and do a quick stretch.
Finally, you should be able to get the test file from your s3 endpoint via your browser at:
http://localhost:1094/s3.amazonaws.com/us-east-1/<bucket name>/<object name>
Closes #7