From 4d4d8d59ee725cce9a2c71eebeecd9b8e5f44f63 Mon Sep 17 00:00:00 2001 From: davidjumani Date: Mon, 10 May 2021 16:55:22 +0530 Subject: [PATCH 1/3] api: Ensure single query parameter value --- .../src/main/java/com/cloud/api/ApiServer.java | 6 +++++- .../src/main/java/com/cloud/api/ApiServlet.java | 16 +++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index cf0891fb606b..0640da7bd2f3 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -456,7 +456,11 @@ public void handle(final HttpRequest request, final HttpResponse response, final responseType = param.getValue(); continue; } - parameterMap.put(param.getName(), new String[]{param.getValue()}); + if(parameterMap.putIfAbsent(param.getName(), new String[]{param.getValue()}) != null) { + String message = String.format("Query parameter '%s' has multiple values", param.getName()); + s_logger.error(message); + throw new ServerApiException(ApiErrorCode.MALFORMED_PARAMETER_ERROR, message); + } } } diff --git a/server/src/main/java/com/cloud/api/ApiServlet.java b/server/src/main/java/com/cloud/api/ApiServlet.java index e9786c9d88f7..1ac74388a718 100644 --- a/server/src/main/java/com/cloud/api/ApiServlet.java +++ b/server/src/main/java/com/cloud/api/ApiServlet.java @@ -36,6 +36,7 @@ import javax.servlet.http.HttpSession; import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.ApiErrorCode; import org.apache.cloudstack.api.ApiServerService; import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.auth.APIAuthenticationManager; @@ -136,6 +137,17 @@ public void run() { }); } + private void ensureSingleQueryParameterValue(Map params) { + params.forEach((k, v) -> { + if (v.length > 1) { + String message = String.format("Query parameter '%s' has multiple values", k); + s_logger.error(message); + throw new ServerApiException(ApiErrorCode.MALFORMED_PARAMETER_ERROR, message); + } + }); + + } + void processRequestInContext(final HttpServletRequest req, final HttpServletResponse resp) { InetAddress remoteAddress = null; try { @@ -156,7 +168,9 @@ void processRequestInContext(final HttpServletRequest req, final HttpServletResp // get the response format since we'll need it in a couple of places String responseType = HttpUtils.RESPONSE_TYPE_XML; final Map params = new HashMap(); - params.putAll(req.getParameterMap()); + Map reqParams = req.getParameterMap(); + ensureSingleQueryParameterValue(reqParams); + params.putAll(reqParams); // For HTTP GET requests, it seems that HttpServletRequest.getParameterMap() actually tries // to unwrap URL encoded content from ISO-9959-1. From a314adcaa62014169499302f40a592e015fd2abc Mon Sep 17 00:00:00 2001 From: davidjumani Date: Fri, 4 Mar 2022 11:10:58 +0530 Subject: [PATCH 2/3] change error to warn --- server/src/main/java/com/cloud/api/ApiServer.java | 5 ++--- server/src/main/java/com/cloud/api/ApiServlet.java | 6 ++---- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index 5b7aa2ebe130..d077a0e2525a 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -465,9 +465,8 @@ 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", param.getName()); - s_logger.error(message); - throw new ServerApiException(ApiErrorCode.MALFORMED_PARAMETER_ERROR, message); + String message = String.format("Query parameter '%s' has multiple values [%s, %s]", param.getName(), param.getValue(), parameterMap.get(param.getName())); + s_logger.warn(message); } } } diff --git a/server/src/main/java/com/cloud/api/ApiServlet.java b/server/src/main/java/com/cloud/api/ApiServlet.java index 3ecaf4e9f78f..0ff8009ccb85 100644 --- a/server/src/main/java/com/cloud/api/ApiServlet.java +++ b/server/src/main/java/com/cloud/api/ApiServlet.java @@ -36,7 +36,6 @@ import javax.servlet.http.HttpSession; import org.apache.cloudstack.api.ApiConstants; -import org.apache.cloudstack.api.ApiErrorCode; import org.apache.cloudstack.api.ApiServerService; import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.auth.APIAuthenticationManager; @@ -140,9 +139,8 @@ public void run() { private void ensureSingleQueryParameterValue(Map params) { params.forEach((k, v) -> { if (v.length > 1) { - String message = String.format("Query parameter '%s' has multiple values", k); - s_logger.error(message); - throw new ServerApiException(ApiErrorCode.MALFORMED_PARAMETER_ERROR, message); + String message = String.format("Query parameter '%s' has multiple values %s", k, Arrays.toString(v)); + s_logger.warn(message); } }); From cf2c1c4dcb9a17fe7a525ad760596b3dc7f2a4b4 Mon Sep 17 00:00:00 2001 From: davidjumani Date: Tue, 8 Mar 2022 12:30:53 +0530 Subject: [PATCH 3/3] Address comments --- server/src/main/java/com/cloud/api/ApiServer.java | 3 ++- server/src/main/java/com/cloud/api/ApiServlet.java | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index d077a0e2525a..4ba246c40479 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -465,7 +465,8 @@ 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]", param.getName(), param.getValue(), parameterMap.get(param.getName())); + 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())); s_logger.warn(message); } } diff --git a/server/src/main/java/com/cloud/api/ApiServlet.java b/server/src/main/java/com/cloud/api/ApiServlet.java index 0ff8009ccb85..09195e0fe227 100644 --- a/server/src/main/java/com/cloud/api/ApiServlet.java +++ b/server/src/main/java/com/cloud/api/ApiServlet.java @@ -136,10 +136,11 @@ public void run() { }); } - private void ensureSingleQueryParameterValue(Map params) { + private void checkSingleQueryParameterValue(Map params) { params.forEach((k, v) -> { if (v.length > 1) { - String message = String.format("Query parameter '%s' has multiple values %s", k, Arrays.toString(v)); + 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", k, Arrays.toString(v)); s_logger.warn(message); } }); @@ -167,7 +168,7 @@ void processRequestInContext(final HttpServletRequest req, final HttpServletResp String responseType = HttpUtils.RESPONSE_TYPE_XML; final Map params = new HashMap(); Map reqParams = req.getParameterMap(); - ensureSingleQueryParameterValue(reqParams); + checkSingleQueryParameterValue(reqParams); params.putAll(reqParams); // For HTTP GET requests, it seems that HttpServletRequest.getParameterMap() actually tries