From fa02054684ca01376de389fe9caf29bc58612b4b Mon Sep 17 00:00:00 2001 From: Radoslaw Zarzynski Date: Wed, 4 Jan 2017 20:24:09 +0100 Subject: [PATCH] rgw: improve const-correctness and refactor S3 canonized string crafting. Signed-off-by: Radoslaw Zarzynski --- src/rgw/rgw_auth_s3.cc | 133 ++++++++++++++++++++++------------------- src/rgw/rgw_auth_s3.h | 33 +++++++--- src/rgw/rgw_common.h | 4 +- 3 files changed, 100 insertions(+), 70 deletions(-) diff --git a/src/rgw/rgw_auth_s3.cc b/src/rgw/rgw_auth_s3.cc index 501dbeb356b..6cebdbab942 100644 --- a/src/rgw/rgw_auth_s3.cc +++ b/src/rgw/rgw_auth_s3.cc @@ -1,6 +1,9 @@ // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- // vim: ts=8 sw=2 smarttab +#include +#include + #include "common/armor.h" #include "common/utf8.h" #include "rgw_common.h" @@ -10,7 +13,7 @@ #define dout_context g_ceph_context #define dout_subsys ceph_subsys_rgw -static const char *signed_subresources[] = { +static const auto signed_subresources = { "acl", "cors", "delete", @@ -35,75 +38,85 @@ static const char *signed_subresources[] = { "versionId", "versioning", "versions", - "website", - NULL + "website" }; /* * ?get the canonical amazon-style header for something? */ -static void get_canon_amz_hdr(map& meta_map, string& dest) +static std::string +get_canon_amz_hdr(const std::map& meta_map) { - dest = ""; - map::iterator iter; - for (iter = meta_map.begin(); iter != meta_map.end(); ++iter) { - dest.append(iter->first); + std::string dest; + + for (const auto& kv : meta_map) { + dest.append(kv.first); dest.append(":"); - dest.append(iter->second); + dest.append(kv.second); dest.append("\n"); } + + return std::move(dest); } /* * ?get the canonical representation of the object's location */ -static void get_canon_resource(const char *request_uri, map& sub_resources, string& dest) +static std::string +get_canon_resource(const char* const request_uri, + const std::map& sub_resources) { - string s; - - if (request_uri) - s.append(request_uri); - - string append_str; + std::string dest; - const char **p = signed_subresources; + if (request_uri) { + dest.append(request_uri); + } - for (; *p; ++p) { - map::iterator iter = sub_resources.find(*p); - if (iter == sub_resources.end()) + bool initial = true; + for (const auto& subresource : signed_subresources) { + const auto iter = sub_resources.find(subresource); + if (iter == std::end(sub_resources)) { continue; + } - if (append_str.empty()) - append_str.append("?"); - else - append_str.append("&"); - append_str.append(iter->first); - if (!iter->second.empty()) { - append_str.append("="); - append_str.append(iter->second); + if (initial) { + dest.append("?"); + initial = false; + } else { + dest.append("&"); + } + + dest.append(iter->first); + if (! iter->second.empty()) { + dest.append("="); + dest.append(iter->second); } } - if (!append_str.empty()) { - s.append(append_str); - } - dest = s; dout(10) << "get_canon_resource(): dest=" << dest << dendl; + return std::move(dest); } /* * get the header authentication information required to * compute a request's signature */ -void rgw_create_s3_canonical_header(const char *method, const char *content_md5, const char *content_type, const char *date, - map& meta_map, const char *request_uri, map& sub_resources, - string& dest_str) +void rgw_create_s3_canonical_header( + const char* const method, + const char* const content_md5, + const char* const content_type, + const char* const date, + const std::map& meta_map, + const char* const request_uri, + const std::map& sub_resources, + std::string& dest_str) { - string dest; + std::string dest; - if (method) + if (method) { dest = method; + } dest.append("\n"); if (content_md5) { @@ -111,22 +124,18 @@ void rgw_create_s3_canonical_header(const char *method, const char *content_md5, } dest.append("\n"); - if (content_type) + if (content_type) { dest.append(content_type); + } dest.append("\n"); - if (date) + if (date) { dest.append(date); + } dest.append("\n"); - string canon_amz_hdr; - get_canon_amz_hdr(meta_map, canon_amz_hdr); - dest.append(canon_amz_hdr); - - string canon_resource; - get_canon_resource(request_uri, sub_resources, canon_resource); - - dest.append(canon_resource); + dest.append(get_canon_amz_hdr(meta_map)); + dest.append(get_canon_resource(request_uri, sub_resources)); dest_str = dest; } @@ -166,13 +175,17 @@ static inline bool is_base64_for_content_md5(unsigned char c) { * get the header authentication information required to * compute a request's signature */ -bool rgw_create_s3_canonical_header(req_info& info, utime_t *header_time, string& dest, bool qsr) +bool rgw_create_s3_canonical_header(const req_info& info, + utime_t* const header_time, + std::string& dest, + const bool qsr) { - const char *content_md5 = info.env->get("HTTP_CONTENT_MD5"); + const char* const content_md5 = info.env->get("HTTP_CONTENT_MD5"); if (content_md5) { for (const char *p = content_md5; *p; p++) { if (!is_base64_for_content_md5(*p)) { - dout(0) << "NOTICE: bad content-md5 provided (not base64), aborting request p=" << *p << " " << (int)*p << dendl; + dout(0) << "NOTICE: bad content-md5 provided (not base64)," + << " aborting request p=" << *p << " " << (int)*p << dendl; return false; } } @@ -180,7 +193,7 @@ bool rgw_create_s3_canonical_header(req_info& info, utime_t *header_time, string const char *content_type = info.env->get("CONTENT_TYPE"); - string date; + std::string date; if (qsr) { date = info.args.get("Expires"); } else { @@ -210,19 +223,19 @@ bool rgw_create_s3_canonical_header(req_info& info, utime_t *header_time, string } } - map& meta_map = info.x_meta_map; - map& sub_resources = info.args.get_sub_resources(); + const auto& meta_map = info.x_meta_map; + const auto& sub_resources = info.args.get_sub_resources(); - string request_uri; - if (info.effective_uri.empty()) + std::string request_uri; + if (info.effective_uri.empty()) { request_uri = info.request_uri; - else + } else { request_uri = info.effective_uri; + } - rgw_create_s3_canonical_header(info.method, content_md5, content_type, date.c_str(), - meta_map, request_uri.c_str(), sub_resources, - dest); - + rgw_create_s3_canonical_header(info.method, content_md5, content_type, + date.c_str(), meta_map, request_uri.c_str(), + sub_resources, dest); return true; } diff --git a/src/rgw/rgw_auth_s3.h b/src/rgw/rgw_auth_s3.h index 4e8dd1ca5c8..f5728caea23 100644 --- a/src/rgw/rgw_auth_s3.h +++ b/src/rgw/rgw_auth_s3.h @@ -4,18 +4,33 @@ #ifndef CEPH_RGW_AUTH_S3_H #define CEPH_RGW_AUTH_S3_H +#include +#include #include "rgw_common.h" -void rgw_create_s3_canonical_header(const char *method, - const char *content_md5, - const char *content_type, const char *date, - map& meta_map, - const char *request_uri, - map& sub_resources, - string& dest_str); -bool rgw_create_s3_canonical_header(req_info& info, utime_t *header_time, - string& dest, bool qsr); +void rgw_create_s3_canonical_header( + const char *method, + const char *content_md5, + const char *content_type, + const char *date, + const std::map& meta_map, + const char *request_uri, + const std::map& sub_resources, + std::string& dest_str); +bool rgw_create_s3_canonical_header(const req_info& info, + utime_t *header_time, /* out */ + std::string& dest, /* out */ + bool qsr); +static inline std::tuple +rgw_create_s3_canonical_header(const req_info& info, const bool qsr) { + std::string dest; + utime_t header_time; + + const bool ok = rgw_create_s3_canonical_header(info, &header_time, dest, qsr); + return std::make_tuple(ok, dest, header_time); +} + int rgw_get_s3_header_digest(const string& auth_hdr, const string& key, string& dest); int rgw_get_s3_header_digest(const string& auth_hdr, const string& key, string& dest); diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h index 3fbe8df82d1..ae622bd31bb 100644 --- a/src/rgw/rgw_common.h +++ b/src/rgw/rgw_common.h @@ -337,7 +337,9 @@ class RGWHTTPArgs map& get_params() { return val_map; } - map& get_sub_resources() { return sub_resources; } + const std::map& get_sub_resources() const { + return sub_resources; + } unsigned get_num_params() const { return val_map.size(); } -- 2.39.5