Damans227 commented on code in PR #13677:
URL: https://github.com/apache/cloudstack/pull/13677#discussion_r4284652972


##########
utils/src/main/java/com/cloud/utils/StringUtils.java:
##########
@@ -25,9 +25,10 @@
 
 import java.nio.charset.Charset;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
+import java.util.Locale;

Review Comment:
   can we keep the Map import? the build fails without it since the map helpers 
further down still use it



##########
server/src/test/java/com/cloud/api/ApiServletTest.java:
##########
@@ -432,4 +432,29 @@ public void testVerify2FAWhenExpectedCommandIsNotCalled() 
throws UnknownHostExce
 
         Assert.assertEquals(false, result);
     }
+    @Test
+    public void testFormatValuesForLogMasksSensitiveParameters() {
+        String result = ApiServlet.formatValuesForLog(

Review Comment:
   should these tests move to the StringUtils tests? the helpers moved there so 
these calls wont compile



##########
server/src/main/java/com/cloud/api/ApiServer.java:
##########
@@ -520,8 +520,9 @@ public void handle(final HttpRequest request, final 
HttpResponse response, final
                         continue;
                     }
                     if(parameterMap.putIfAbsent(param.getName(), new 
String[]{param.getValue()}) != null) {
-                        String message = String.format("Query parameter '%s' 
has multiple values [%s, %s]. Only the last value will be respected." +
-                            "It is advised to pass only a single parameter", 
param.getName(), param.getValue(), parameterMap.get(param.getName()));
+                        String message = String.format("Query parameter '%s' 
has multiple values %s. Only the last value will be respected." +
+                            "It is advised to pass only a single parameter", 
param.getName(),
+                            StringUtils.formatValuesForLog(param.getName(), 
(String[]) parameterMap.get(param.getName())));

Review Comment:
   should this still list both values like before? it only shows the one 
already saved now



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to