You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2022/07/07 17:03:20 UTC

[trafficserver] 01/02: Fixes compacting spaces in S3 auth plugin (#8579)

This is an automated email from the ASF dual-hosted git repository.

zwoop pushed a commit to branch 9.2.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git

commit 34a79dcb76766c377b0fe4b1bb86e695d154f7aa
Author: Julian Rubin <ru...@gmail.com>
AuthorDate: Tue Jan 11 19:37:57 2022 +0100

    Fixes compacting spaces in S3 auth plugin (#8579)
    
    Co-authored-by: Julian Rubin <ju...@rtbhouse.com>
    (cherry picked from commit ca2a06ef1ef2adac278c143085309ed9c4598dd9)
---
 plugins/s3_auth/aws_auth_v4.cc                 | 36 +++++++----
 plugins/s3_auth/unit_tests/test_aws_auth_v4.cc | 86 +++++++++++++++++---------
 plugins/s3_auth/unit_tests/test_aws_auth_v4.h  |  2 +-
 3 files changed, 84 insertions(+), 40 deletions(-)

diff --git a/plugins/s3_auth/aws_auth_v4.cc b/plugins/s3_auth/aws_auth_v4.cc
index 3f9aea077..89ebb07e3 100644
--- a/plugins/s3_auth/aws_auth_v4.cc
+++ b/plugins/s3_auth/aws_auth_v4.cc
@@ -168,18 +168,19 @@ canonicalEncode(const String &in, bool isObjectName)
 }
 
 /**
- * @brief trim the white-space character from the beginning and the end of the string ("in-place", just moving pointers around)
+ * @brief Trim white spaces from beginning and end. Squeeze consecutive spaces from middle.
+ *
+ * @see AWS spec: https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html
  *
  * @param in ptr to an input string
  * @param inLen input character count
- * @param newLen trimmed string character count.
  * @return pointer to the trimmed string.
  */
-const char *
-trimWhiteSpaces(const char *in, size_t inLen, size_t &newLen)
+std::string
+trimWhiteSpacesAndSqueezeInnerSpaces(const char *in, size_t inLen)
 {
   if (nullptr == in || inLen == 0) {
-    return in;
+    return std::string(in, inLen);
   }
 
   const char *first = in;
@@ -192,8 +193,22 @@ trimWhiteSpaces(const char *in, size_t inLen, size_t &newLen)
     last--;
   }
 
-  newLen = last - first + 1;
-  return first;
+  std::stringstream result;
+  int consecutiveSpaces = 0;
+  while (first <= last) {
+    if (*first == ' ') {
+      consecutiveSpaces++;
+    } else {
+      if (consecutiveSpaces > 0) {
+        result << ' ';
+      }
+      consecutiveSpaces = 0;
+      result << *first;
+    }
+    first++;
+  }
+
+  return result.str();
 }
 
 /**
@@ -381,14 +396,13 @@ getCanonicalRequestSha256Hash(TsInterface &api, bool signPayload, const StringSe
       }
     }
 
-    size_t trimValueLen   = 0;
-    const char *trimValue = trimWhiteSpaces(value, valueLen, trimValueLen);
+    std::string trimValue = trimWhiteSpacesAndSqueezeInnerSpaces(value, valueLen);
 
     signedHeadersSet.insert(lowercaseName);
     if (headersMap.find(lowercaseName) == headersMap.end()) {
-      headersMap[lowercaseName] = String(trimValue, trimValueLen);
+      headersMap[lowercaseName] = trimValue;
     } else {
-      headersMap[lowercaseName].append(",").append(String(trimValue, trimValueLen));
+      headersMap[lowercaseName].append(",").append(trimValue);
     }
   }
 
diff --git a/plugins/s3_auth/unit_tests/test_aws_auth_v4.cc b/plugins/s3_auth/unit_tests/test_aws_auth_v4.cc
index 595fe003e..635dc63e4 100644
--- a/plugins/s3_auth/unit_tests/test_aws_auth_v4.cc
+++ b/plugins/s3_auth/unit_tests/test_aws_auth_v4.cc
@@ -172,74 +172,104 @@ TEST_CASE("base16Encode(): base16 encode RFC4648 test vectors", "[utility]")
 
 /* trimWhiteSpaces() ******************************************************************************************************** */
 
-TEST_CASE("trimWhiteSpaces(): trim invalid arguments, check pointers", "[utility]")
+TEST_CASE("trimWhiteSpacesAndSqueezeInnerSpaces(): trim invalid arguments, check string", "[utility]")
 {
   const char *in = nullptr;
   size_t inLen   = 0;
-  size_t outLen  = 0;
 
-  const char *start = trimWhiteSpaces(in, inLen, outLen);
+  const std::string trimmed = trimWhiteSpacesAndSqueezeInnerSpaces(in, inLen);
 
-  CHECK(in == start);
+  CHECK_FALSE(trimmed.compare(""));
+  CHECK(inLen == trimmed.length());
 }
 
-TEST_CASE("trimWhiteSpaces(): trim empty input, check pointers", "[utility]")
+TEST_CASE("trimWhiteSpacesAndSqueezeInnerSpaces(): trim empty input, check string", "[utility]")
 {
   const char *in = "";
   size_t inLen   = 0;
-  size_t outLen  = 0;
 
-  const char *start = trimWhiteSpaces(in, inLen, outLen);
+  const std::string trimmed = trimWhiteSpacesAndSqueezeInnerSpaces(in, inLen);
 
-  CHECK(in == start);
+  CHECK_FALSE(trimmed.compare(""));
+  CHECK(inLen == trimmed.length());
 }
 
-TEST_CASE("trimWhiteSpaces(): trim nothing to trim, check pointers", "[utility]")
+TEST_CASE("trimWhiteSpacesAndSqueezeInnerSpaces(): trim nothing to trim, check string", "[utility]")
 {
   const char in[] = "Important Message";
   size_t inLen    = strlen(in);
-  size_t newLen   = 0;
 
-  const char *start = trimWhiteSpaces(in, inLen, newLen);
+  const std::string trimmed = trimWhiteSpacesAndSqueezeInnerSpaces(in, inLen);
 
-  CHECK(in == start);
-  CHECK(inLen == newLen);
+  CHECK_FALSE(trimmed.compare("Important Message"));
+  CHECK(inLen == trimmed.length());
 }
 
-TEST_CASE("trimWhiteSpaces(): trim beginning, check pointers", "[utility]")
+TEST_CASE("trimWhiteSpacesAndSqueezeInnerSpaces(): trim beginning, check string", "[utility]")
 {
   const char in[] = " \t\nImportant Message";
   size_t inLen    = strlen(in);
-  size_t newLen   = 0;
 
-  const char *start = trimWhiteSpaces(in, inLen, newLen);
+  const std::string trimmed = trimWhiteSpacesAndSqueezeInnerSpaces(in, inLen);
 
-  CHECK(in + 3 == start);
-  CHECK(inLen - 3 == newLen);
+  CHECK_FALSE(trimmed.compare("Important Message"));
+  CHECK(inLen - 3 == trimmed.length());
 }
 
-TEST_CASE("trimWhiteSpaces(): trim end, check pointers", "[utility]")
+TEST_CASE("trimWhiteSpacesAndSqueezeInnerSpaces(): trim end, check string", "[utility]")
 {
   const char in[] = "Important Message \t\n";
   size_t inLen    = strlen(in);
-  size_t newLen   = 0;
 
-  const char *start = trimWhiteSpaces(in, inLen, newLen);
+  const std::string trimmed = trimWhiteSpacesAndSqueezeInnerSpaces(in, inLen);
 
-  CHECK(in == start);
-  CHECK(inLen - 3 == newLen);
+  CHECK_FALSE(trimmed.compare("Important Message"));
+  CHECK(inLen - 3 == trimmed.length());
 }
 
-TEST_CASE("trimWhiteSpaces(): trim both ends, check pointers", "[utility]")
+TEST_CASE("trimWhiteSpacesAndSqueezeInnerSpaces(): trim both ends, check string", "[utility]")
 {
   const char in[] = "\v\t\n Important Message \t\n";
   size_t inLen    = strlen(in);
-  size_t newLen   = 0;
 
-  const char *start = trimWhiteSpaces(in, inLen, newLen);
+  const std::string trimmed = trimWhiteSpacesAndSqueezeInnerSpaces(in, inLen);
+
+  CHECK_FALSE(trimmed.compare("Important Message"));
+  CHECK(inLen - 7 == trimmed.length());
+}
+
+TEST_CASE("trimWhiteSpacesAndSqueezeInnerSpaces(): trim both ends and squeeze middle spaces, check string", "[utility]")
+{
+  const char in[] = "\v\t\n Important     Message \t\n";
+  size_t inLen    = strlen(in);
+
+  const std::string trimmed = trimWhiteSpacesAndSqueezeInnerSpaces(in, inLen);
+
+  CHECK_FALSE(trimmed.compare("Important Message"));
+  CHECK(inLen - 11 == trimmed.length());
+}
+
+TEST_CASE("trimWhiteSpacesAndSqueezeInnerSpaces(): squeeze middle spaces multiple groups, check string", "[utility]")
+{
+  const char in[] = "Very   Important     Message";
+  size_t inLen    = strlen(in);
+
+  const std::string trimmed = trimWhiteSpacesAndSqueezeInnerSpaces(in, inLen);
+
+  CHECK_FALSE(trimmed.compare("Very Important Message"));
+  CHECK(inLen - 6 == trimmed.length());
+}
+
+TEST_CASE("trimWhiteSpacesAndSqueezeInnerSpaces(): does not squeeze middle whitespaces different from spaces, check string",
+          "[utility]")
+{
+  const char in[] = "Very \t\tImportant \t\t\tMessage";
+  size_t inLen    = strlen(in);
+
+  const std::string trimmed = trimWhiteSpacesAndSqueezeInnerSpaces(in, inLen);
 
-  CHECK(in + 4 == start);
-  CHECK(inLen - 7 == newLen);
+  CHECK_FALSE(trimmed.compare("Very \t\tImportant \t\t\tMessage"));
+  CHECK(inLen == trimmed.length());
 }
 
 TEST_CASE("trimWhiteSpaces(): trim both, check string", "[utility]")
diff --git a/plugins/s3_auth/unit_tests/test_aws_auth_v4.h b/plugins/s3_auth/unit_tests/test_aws_auth_v4.h
index e295d750f..42f415716 100644
--- a/plugins/s3_auth/unit_tests/test_aws_auth_v4.h
+++ b/plugins/s3_auth/unit_tests/test_aws_auth_v4.h
@@ -123,7 +123,7 @@ String base16Encode(const char *in, size_t inLen);
 String uriEncode(const String &in, bool isObjectName = false);
 bool isUriEncoded(const String &in, bool isObjectName = false);
 String lowercase(const char *in, size_t inLen);
-const char *trimWhiteSpaces(const char *in, size_t inLen, size_t &newLen);
+String trimWhiteSpacesAndSqueezeInnerSpaces(const char *in, size_t inLen);
 
 String getCanonicalRequestSha256Hash(TsInterface &api, bool signPayload, const StringSet &includeHeaders,
                                      const StringSet &excludeHeaders, String &signedHeaders);