[GitHub] [trafficserver] duke8253 commented on a change in pull request #8579: Fixes compacting spaces in S3 auth plugin

[GitBox <gi...@apache.org>]

duke8253 commented on a change in pull request #8579:
URL: https://github.com/apache/trafficserver/pull/8579#discussion_r779158805



##########
File path: plugins/s3_auth/aws_auth_v4.cc
##########
@@ -192,8 +192,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 == ' ') {

Review comment:
       Not sure this will work. Correct me if I'm wrong, but to me it looks like you're replacing consecutive spaces with one space, and putting it back in the string. So `access_key=    foo` will become `access_key= foo`. But from what I understand, #7888 wants all space removed. Also, I believe `isspace` checks for all white-space characters, not just space.
   
   Btw, if we're removing all white-spaces in the string, why have separate loops to remove the leading and trailing spaces, this could all be done in one go.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org