Merge pull request #15678 from cbodley/wip-rgw-string-join

rgw: add variadic string join for s3 signature generation

Reviewed-by: Adam C. Emerson <aemerson@redhat.com>
Reviewed-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>

src/rgw/rgw_auth_s3.cc

@@ -646,21 +646,13 @@ get_v4_canon_req_hash(CephContext* cct,
 {
   ldout(cct, 10) << "payload request hash = " << request_payload_hash << dendl;
 
-  const size_t total_len = http_verb.length() + canonical_uri.length() + \
-    canonical_qs.length() + canonical_hdrs.length() + signed_hdrs.length() + \
-    request_payload_hash.length() + std::strlen("\n") * 5;
-  const auto canonical_req = create_n_reserve<std::string>(total_len)
-    .append(http_verb.data(), http_verb.length())
-    .append("\n", std::strlen("\n"))
-    .append(canonical_uri)
-    .append("\n", std::strlen("\n"))
-    .append(canonical_qs)
-    .append("\n", std::strlen("\n"))
-    .append(canonical_hdrs)
-    .append("\n", std::strlen("\n"))
-    .append(signed_hdrs.data(), signed_hdrs.length())
-    .append("\n", std::strlen("\n"))
-    .append(request_payload_hash.data(), request_payload_hash.length());
+  const auto canonical_req = string_join_reserve("\n",
+    http_verb,
+    canonical_uri,
+    canonical_qs,
+    canonical_hdrs,
+    signed_hdrs,
+    request_payload_hash);
 
   const auto canonical_req_hash = calc_hash_sha256(canonical_req);
 
@@ -684,17 +676,14 @@ get_v4_string_to_sign(CephContext* const cct,
                       const sha256_digest_t& canonreq_hash)
 {
   const auto hexed_cr_hash = buf_to_hex(canonreq_hash);
+  const boost::string_view hexed_cr_hash_str(hexed_cr_hash.data(),
+                                             hexed_cr_hash.size() - 1);
 
-  const size_t total_len = algorithm.length() + request_date.length() + \
-    credential_scope.length() + hexed_cr_hash.size() - 1 + std::strlen("\n") * 3;
-  const auto string_to_sign = create_n_reserve<std::string>(total_len)
-    .append(algorithm.data(), algorithm.length())
-    .append("\n", std::strlen("\n"))
-    .append(request_date.data(), request_date.length())
-    .append("\n", std::strlen("\n"))
-    .append(credential_scope.data(), credential_scope.length())
-    .append("\n", std::strlen("\n"))
-    .append(hexed_cr_hash.data(), hexed_cr_hash.size() - 1);
+  const auto string_to_sign = string_join_reserve("\n",
+    algorithm,
+    request_date,
+    credential_scope,
+    hexed_cr_hash_str);
 
   ldout(cct, 10) << "string to sign = "
                  << rgw::crypt_sanitize::log_content{string_to_sign}
@@ -919,25 +908,13 @@ AWSv4ComplMulti::ChunkMeta::create_next(CephContext* const cct,
 std::string
 AWSv4ComplMulti::calc_chunk_signature(const std::string& payload_hash) const
 {
-  const size_t algorithm_len = std::strlen(AWS4_HMAC_SHA256_STR);
-  const size_t empty_hash_len = std::strlen(AWS4_EMPTY_PAYLOAD_HASH);
-
-  /* We want to avoid reallocations when concatenating the string_to_sign. */
-  const size_t total_len = algorithm_len + date.length() + \
-    credential_scope.length() + prev_chunk_signature.length() + \
-    empty_hash_len + payload_hash.length() + std::strlen("\n") * 5;
-  const auto string_to_sign = create_n_reserve<std::string>(total_len)
-    .append(AWS4_HMAC_SHA256_STR, algorithm_len)
-    .append("\n", std::strlen("\n"))
-    .append(date.data(), date.length())
-    .append("\n", std::strlen("\n"))
-    .append(credential_scope.data(), credential_scope.length())
-    .append("\n", std::strlen("\n"))
-    .append(prev_chunk_signature)
-    .append("\n", std::strlen("\n"))
-    .append(AWS4_EMPTY_PAYLOAD_HASH, empty_hash_len)
-    .append("\n", std::strlen("\n"))
-    .append(payload_hash);
+  const auto string_to_sign = string_join_reserve("\n",
+    AWS4_HMAC_SHA256_STR,
+    date,
+    credential_scope,
+    prev_chunk_signature,
+    AWS4_EMPTY_PAYLOAD_HASH,
+    payload_hash);
 
   ldout(cct, 20) << "AWSv4ComplMulti: string_to_sign=\n" << string_to_sign
                  << dendl;

src/rgw/rgw_string.h

@@ -9,30 +9,31 @@
 
 #include <boost/container/small_vector.hpp>
 #include <boost/utility/string_view.hpp>
+
 struct ltstr_nocase
 {
-  bool operator()(const string& s1, const string& s2) const
+  bool operator()(const std::string& s1, const std::string& s2) const
   {
     return strcasecmp(s1.c_str(), s2.c_str()) < 0;
   }
 };
 
-static inline int stringcasecmp(const string& s1, const string& s2)
+static inline int stringcasecmp(const std::string& s1, const std::string& s2)
 {
   return strcasecmp(s1.c_str(), s2.c_str());
 }
 
-static inline int stringcasecmp(const string& s1, const char *s2)
+static inline int stringcasecmp(const std::string& s1, const char *s2)
 {
   return strcasecmp(s1.c_str(), s2);
 }
 
-static inline int stringcasecmp(const string& s1, int ofs, int size, const string& s2)
+static inline int stringcasecmp(const std::string& s1, int ofs, int size, const std::string& s2)
 {
   return strncasecmp(s1.c_str() + ofs, s2.c_str(), size);
 }
 
-static inline int stringtoll(const string& s, int64_t *val)
+static inline int stringtoll(const std::string& s, int64_t *val)
 {
   char *end;
 
@@ -48,7 +49,7 @@ static inline int stringtoll(const string& s, int64_t *val)
   return 0;
 }
 
-static inline int stringtoull(const string& s, uint64_t *val)
+static inline int stringtoull(const std::string& s, uint64_t *val)
 {
   char *end;
 
@@ -64,7 +65,7 @@ static inline int stringtoull(const string& s, uint64_t *val)
   return 0;
 }
 
-static inline int stringtol(const string& s, int32_t *val)
+static inline int stringtol(const std::string& s, int32_t *val)
 {
   char *end;
 
@@ -80,7 +81,7 @@ static inline int stringtol(const string& s, int32_t *val)
   return 0;
 }
 
-static inline int stringtoul(const string& s, uint32_t *val)
+static inline int stringtoul(const std::string& s, uint32_t *val)
 {
   char *end;
 
@@ -112,19 +113,6 @@ sview2cstr(const boost::string_view& sv)
   return cstr;
 }
 
-/* We need this helper function because the interface of std::string::reserve
- * doesn't provide the chaining ability in the type append(). It's required
- * to concatenate string without reallocations in a way const-correct manner. */
-template <class StringT>
-static inline StringT create_n_reserve(const size_t reserve_len)
-{
-  StringT ret;
-  /* I would love to see reserve() returning "basic_string&" instead of "void"
-   * in the standard library! */
-  ret.reserve(reserve_len);
-  return ret;
-}
-
 /* std::strlen() isn't guaranteed to be computable at compile-time. Although
  * newer GCCs actually do that, Clang doesn't. Please be aware this function
  * IS NOT A DROP-IN REPLACEMENT FOR STRLEN -- it returns a different result
@@ -134,4 +122,98 @@ static inline constexpr size_t sarrlen(const char (&arr)[N]) {
   return N - 1;
 }
 
+namespace detail {
+
+// variadic sum() to add up string lengths for reserve()
+static inline constexpr size_t sum() { return 0; }
+template <typename... Args>
+constexpr size_t sum(size_t v, Args... args) { return v + sum(args...); }
+
+// traits for string_size()
+template <typename T>
+struct string_traits {
+  static constexpr size_t size(const T& s) { return s.size(); }
+};
+// specializations for char*/const char* use strlen()
+template <>
+struct string_traits<const char*> {
+  static constexpr size_t size(const char* s) { return std::strlen(s); }
+};
+template <>
+struct string_traits<char*> : string_traits<const char*> {};
+// specializations for char[]/const char[] also use strlen()
+template <std::size_t N>
+struct string_traits<const char[N]> : string_traits<const char*> {};
+template <std::size_t N>
+struct string_traits<char[N]> : string_traits<const char*> {};
+
+// helpers for string_cat_reserve()
+static inline void append_to(std::string& s) {}
+template <typename... Args>
+void append_to(std::string& s, const boost::string_view& v, const Args&... args)
+{
+  s.append(v.begin(), v.end());
+  append_to(s, args...);
+}
+
+// helpers for string_join_reserve()
+static inline void join_next(std::string& s, const boost::string_view& d) {}
+template <typename... Args>
+void join_next(std::string& s, const boost::string_view& d,
+               const boost::string_view& v, const Args&... args)
+{
+  s.append(d.begin(), d.end());
+  s.append(v.begin(), v.end());
+  join_next(s, d, args...);
+}
+
+static inline void join(std::string& s, const boost::string_view& d) {}
+template <typename... Args>
+void join(std::string& s, const boost::string_view& d,
+          const boost::string_view& v, const Args&... args)
+{
+  s.append(v.begin(), v.end());
+  join_next(s, d, args...);
+}
+
+} // namespace detail
+
+/// return the length of a c string, string literal, or string type
+template <typename T>
+constexpr size_t string_size(const T& s)
+{
+  return detail::string_traits<T>::size(s);
+}
+
+/// concatenates the given string arguments, returning as a std::string that
+/// gets preallocated with reserve()
+template <typename... Args>
+std::string string_cat_reserve(const Args&... args)
+{
+  size_t total_size = detail::sum(string_size(args)...);
+  std::string result;
+  result.reserve(total_size);
+  detail::append_to(result, args...);
+  return result;
+}
+
+/// joins the given string arguments with a delimiter, returning as a
+/// std::string that gets preallocated with reserve()
+template <typename... Args>
+std::string string_join_reserve(const boost::string_view& delim,
+                                const Args&... args)
+{
+  size_t delim_size = delim.size() * std::max<ssize_t>(0, sizeof...(args) - 1);
+  size_t total_size = detail::sum(string_size(args)...) + delim_size;
+  std::string result;
+  result.reserve(total_size);
+  detail::join(result, delim, args...);
+  return result;
+}
+template <typename... Args>
+std::string string_join_reserve(char delim, const Args&... args)
+{
+  return string_join_reserve(boost::string_view{&delim, 1}, args...);
+}
+
 #endif

src/test/rgw/CMakeLists.txt

@@ -131,3 +131,7 @@ target_link_libraries(ceph_test_rgw_iam_policy
   ${CRYPTO_LIBS}
   )
 set_target_properties(ceph_test_rgw_iam_policy PROPERTIES COMPILE_FLAGS ${UNITTEST_CXX_FLAGS})
+
+# unitttest_rgw_string
+add_executable(unittest_rgw_string test_rgw_string.cc)
+add_ceph_unittest(unittest_rgw_string ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/unittest_rgw_string)

src/test/rgw/test_rgw_string.cc

@@ -0,0 +1,71 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+/*
+ * Ceph - scalable distributed file system
+ *
+ * Copyright (C) 2017 Red Hat
+ *
+ * This is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License version 2.1, as published by the Free Software
+ * Foundation. See file COPYING.
+ *
+ */
+
+#include "rgw/rgw_string.h"
+#include <gtest/gtest.h>
+
+const std::string abc{"abc"};
+const char *def{"def"}; // const char*
+char ghi_arr[] = {'g', 'h', 'i', '\0'};
+char *ghi{ghi_arr}; // char*
+constexpr boost::string_view jkl{"jkl", 3};
+#define mno "mno" // string literal (char[4])
+char pqr[] = {'p', 'q', 'r', '\0'};
+
+TEST(string_size, types)
+{
+  ASSERT_EQ(3u, string_size(abc));
+  ASSERT_EQ(3u, string_size(def));
+  ASSERT_EQ(3u, string_size(ghi));
+  ASSERT_EQ(3u, string_size(jkl));
+  ASSERT_EQ(3u, string_size(mno));
+  ASSERT_EQ(3u, string_size(pqr));
+
+  constexpr auto compile_time_size = string_size(jkl);
+  ASSERT_EQ(3u, compile_time_size);
+}
+
+TEST(string_cat_reserve, types)
+{
+  ASSERT_EQ("abcdefghijklmnopqr",
+            string_cat_reserve(abc, def, ghi, jkl, mno, pqr));
+}
+
+TEST(string_cat_reserve, count)
+{
+  ASSERT_EQ("", string_cat_reserve());
+  ASSERT_EQ("abc", string_cat_reserve(abc));
+  ASSERT_EQ("abcdef", string_cat_reserve(abc, def));
+}
+
+TEST(string_join_reserve, types)
+{
+  ASSERT_EQ("abc, def, ghi, jkl, mno, pqr",
+            string_join_reserve(", ", abc, def, ghi, jkl, mno, pqr));
+}
+
+TEST(string_join_reserve, count)
+{
+  ASSERT_EQ("", string_join_reserve(", "));
+  ASSERT_EQ("abc", string_join_reserve(", ", abc));
+  ASSERT_EQ("abc, def", string_join_reserve(", ", abc, def));
+}
+
+TEST(string_join_reserve, delim)
+{
+  ASSERT_EQ("abcdef", string_join_reserve("", abc, def));
+  ASSERT_EQ("abc def", string_join_reserve(' ', abc, def));
+  ASSERT_EQ("abc\ndef", string_join_reserve('\n', abc, def));
+  ASSERT_EQ("abcfoodef", string_join_reserve(std::string{"foo"}, abc, def));
+}