Skip to content

PXB-3498: [xbcloud] Fix HTTP request signing when retrying #1650

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.

Sign up for GitHub

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

Open
imrivera wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

PXB-3498: [xbcloud] Fix HTTP request signing when retrying #1650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions storage/innobase/xtrabackup/src/xbcloud/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ IF (WITH_UNIT_TESTS AND CMAKE_VERSION VERSION_GREATER 3.22.1)
ADD_EXECUTABLE(xbcloud-t xbcloud-t.cc
http.cc
s3.cc
s3_ec2.cc
azure.cc
swift.cc)

Expand Down
49 changes: 31 additions & 18 deletions storage/innobase/xtrabackup/src/xbcloud/azure.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,9 +226,10 @@ void Azure_client::set_endpoint(const std::string &ep, bool development_storage,

bool Azure_client::delete_object(const std::string &container,
const std::string &name) {
auto sign_fun = get_sign_fun(container, name);
Http_request req(Http_request::DELETE, protocol, host,
"/" + container + "/" + name);
signer->sign_request(container, name, req, time(0));
"/" + container + "/" + name, sign_fun);
req.sign();

Http_response resp;
if (!http_client->make_request(req, resp)) {
Expand Down Expand Up @@ -259,14 +260,15 @@ bool Azure_client::async_delete_object(const std::string &container,
const std::string &name,
Event_handler *h,
const async_delete_callback_t callback) {
auto sign_fun = get_sign_fun(container, name);
Http_request *req = new Http_request(Http_request::DELETE, protocol, host,
"/" + container + "/" + name);
"/" + container + "/" + name, sign_fun);
if (req == nullptr) {
msg_ts("%s: Failed to delere object %s/%s. Out of memory.\n", my_progname,
container.c_str(), name.c_str());
return false;
}
signer->sign_request(container, name, *req, time(0));
req->sign();

Http_response *resp = new Http_response();
if (resp == nullptr) {
Expand Down Expand Up @@ -306,8 +308,10 @@ bool Azure_client::async_delete_object(const std::string &container,
Http_buffer Azure_client::download_object(const std::string &container,
const std::string &name,
bool &success) {
Http_request req(Http_request::GET, protocol, host, container + "/" + name);
signer->sign_request(container, name, req, time(0));
auto sign_fun = get_sign_fun(container, name);
Http_request req(Http_request::GET, protocol, host, container + "/" + name,
sign_fun);
req.sign();

Http_response resp;
if (!http_client->make_request(req, resp)) {
Expand All @@ -325,9 +329,10 @@ Http_buffer Azure_client::download_object(const std::string &container,
}

bool Azure_client::create_container(const std::string &name) {
Http_request req(Http_request::PUT, protocol, host, "/" + name);
auto sign_fun = get_sign_fun(name, "");
Http_request req(Http_request::PUT, protocol, host, "/" + name, sign_fun);
req.add_param("restype", "container");
signer->sign_request(name, "", req, time(0));
req.sign();

Http_response resp;
if (!http_client->make_request(req, resp)) {
Expand Down Expand Up @@ -355,10 +360,11 @@ bool Azure_client::create_container(const std::string &name) {
}

bool Azure_client::container_exists(const std::string &name, bool &exists) {
Http_request req(Http_request::HEAD, protocol, host, "/" + name);
auto sign_fun = get_sign_fun(name, "");
Http_request req(Http_request::HEAD, protocol, host, "/" + name, sign_fun);
req.add_param("comp", "metadata");
req.add_param("restype", "container");
signer->sign_request(name, "", req, time(0));
req.sign();

Http_response resp;
if (!http_client->make_request(req, resp)) {
Expand All @@ -381,11 +387,13 @@ bool Azure_client::container_exists(const std::string &name, bool &exists) {
bool Azure_client::upload_object(const std::string &container,
const std::string &name,
const Http_buffer &contents) {
Http_request req(Http_request::PUT, protocol, host, container + "/" + name);
auto sign_fun = get_sign_fun(container, name);
Http_request req(Http_request::PUT, protocol, host, container + "/" + name,
sign_fun);
req.add_header("Content-Length", std::to_string(contents.size()));
req.add_header(AZURE_BLOB_TYPE_HEADER, "BlockBlob");
req.append_payload(contents);
signer->sign_request(container, name, req, time(0));
req.sign();

Http_response resp;

Expand Down Expand Up @@ -430,8 +438,10 @@ bool Azure_client::async_upload_object(
const Http_buffer &contents, Event_handler *h,
async_upload_callback_t callback,
const Http_request::headers_t &extra_http_headers) {
auto sign_fun = get_sign_fun(container, name);

Http_request *req = new Http_request(Http_request::PUT, protocol, host,
"/" + container + "/" + name);
"/" + container + "/" + name, sign_fun);
if (req == nullptr) {
msg_ts("%s: Failed to upload object %s/%s. Out of memory.\n", my_progname,
container.c_str(), name.c_str());
Expand All @@ -445,7 +455,7 @@ bool Azure_client::async_upload_object(
req->add_header(h.first, h.second);
}
req->append_payload(contents);
signer->sign_request(container, name, *req, time(0));
req->sign();

Http_response *resp = new Http_response();
if (resp == nullptr) {
Expand Down Expand Up @@ -477,8 +487,9 @@ bool Azure_client::async_download_object(
const std::string &container, const std::string &name, Event_handler *h,
const async_download_callback_t callback,
const Http_request::headers_t &extra_http_headers) {
auto sign_fun = get_sign_fun(container, name);
Http_request *req = new Http_request(Http_request::GET, protocol, host,
"/" + container + "/" + name);
"/" + container + "/" + name, sign_fun);
if (req == nullptr) {
msg_ts("%s: Failed to download object %s/%s. Out of memory.\n", my_progname,
container.c_str(), name.c_str());
Expand All @@ -487,7 +498,7 @@ bool Azure_client::async_download_object(
for (const auto &h : extra_http_headers) {
req->add_header(h.first, h.second);
}
signer->sign_request(container, name, *req, time(0));
req->sign();

Http_response *resp = new Http_response();
if (resp == nullptr) {
Expand Down Expand Up @@ -538,15 +549,17 @@ bool Azure_client::list_objects_with_prefix(const std::string &container,
std::string next_marker;

while (truncated) {
Http_request req(Http_request::GET, protocol, host, "/" + container);
auto sign_fun = get_sign_fun(container, "");
Http_request req(Http_request::GET, protocol, host, "/" + container,
sign_fun);
req.add_param("comp", "list");
req.add_param("restype", "container");
req.add_param("maxresults", "1000");
req.add_param("prefix", prefix);
if (truncated == true) {
req.add_param("marker", next_marker);
}
signer->sign_request(container, "", req, time(0));
req.sign();

Http_response resp;
if (!http_client->make_request(req, resp)) {
Expand Down
9 changes: 8 additions & 1 deletion storage/innobase/xtrabackup/src/xbcloud/azure.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,9 @@ class Azure_client {
std::function<void(bool, const Http_buffer &)>;
using async_delete_callback_t = std::function<void(bool)>;

private:
std::unique_ptr<Azure_signer> signer;

private:
const Http_client *http_client;

std::string endpoint;
Expand Down Expand Up @@ -107,6 +107,13 @@ class Azure_client {
Event_handler *h, Azure_client::async_download_callback_t callback,
CURLcode rc, const Http_connection *conn, ulong count);

Http_request::sign_fun_t get_sign_fun(const std::string &container,
const std::string &name) {
return [this, container, name](Http_request &req) {
signer->sign_request(container, name, req, time(0));
};
}

public:
Azure_client(const Http_client *client, const std::string &storage_account,
const std::string &access_key, bool development_storage,
Expand Down
3 changes: 1 addition & 2 deletions storage/innobase/xtrabackup/src/xbcloud/http.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -634,8 +634,7 @@ void Http_client::callback(CLIENT *client, std::string container,
delay, name.c_str(), count);
std::this_thread::sleep_for(std::chrono::milliseconds(delay));
resp->reset_body();
client->signer->sign_request(client->hostname(container), container, *req,
Copy link
Author

@imrivera imrivera Apr 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is the main problem. These parameters are correct for S3, but not for Azure (for Azure they should be container and name)

time(0));
req->sign();
http_client->make_async_request(
*req, *resp, h,
std::bind(&Http_client::callback<CLIENT, CALLBACK>, this, client,
Expand Down
13 changes: 11 additions & 2 deletions storage/innobase/xtrabackup/src/xbcloud/http.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ class Http_request {
using headers_t = std::map<std::string, std::string>;
using param_t = std::pair<std::string, std::string>;
using params_t = std::map<std::string, std::string>;
using sign_fun_t = std::function<void(Http_request &)>;

private:
method_t method_;
Expand All @@ -150,14 +151,16 @@ class Http_request {
headers_t headers_;
params_t params_;
Http_buffer payload_;
sign_fun_t sign_fun_;

public:
Http_request(method_t method, protocol_t protocol, const std::string host,
const std::string &path)
const std::string &path, const sign_fun_t &sign_fun)
: method_(method),
protocol_(protocol),
host_(host),
path_(uri_escape_path(path)) {}
path_(uri_escape_path(path)),
sign_fun_(sign_fun) {}
void add_header(const std::string &name, const std::string &value) {
headers_[name] = value;
}
Expand Down Expand Up @@ -190,6 +193,11 @@ class Http_request {
protocol_t protocol() const { return protocol_; }
const Http_buffer &payload() const { return payload_; }
std::string query_string() const;
void sign() {
if (sign_fun_) {
sign_fun_(*this);
}
}
};

class Http_response {
Expand Down Expand Up @@ -339,6 +347,7 @@ class Event_handler {
class Http_client {
public:
using async_callback_t = std::function<void(CURLcode, Http_connection *)>;

private:
bool insecure{false};
bool verbose{false};
Expand Down
Loading