sarutak commented on a change in pull request #32317:
URL: https://github.com/apache/spark/pull/32317#discussion_r629564122
##########
File path:
core/src/main/scala/org/apache/spark/status/api/v1/StagesResource.scala
##########
@@ -152,10 +153,11 @@ private[v1] class StagesResource extends BaseAppResource {
// information like the columns to be sorted, search value typed by the
user in the search
// box, pagination index etc. For more information on these query
parameters,
// refer https://datatables.net/manual/server-side.
- if (uriQueryParameters.getFirst("search[value]") != null &&
- uriQueryParameters.getFirst("search[value]").length > 0) {
+ searchValue = encodeKeyAndGetValue(uriQueryParameters, "search[value]",
null)
Review comment:
Should we consider the case that `searchValue` is also encoded?
##########
File path:
core/src/main/scala/org/apache/spark/status/api/v1/StagesResource.scala
##########
@@ -152,10 +153,11 @@ private[v1] class StagesResource extends BaseAppResource {
// information like the columns to be sorted, search value typed by the
user in the search
// box, pagination index etc. For more information on these query
parameters,
// refer https://datatables.net/manual/server-side.
- if (uriQueryParameters.getFirst("search[value]") != null &&
- uriQueryParameters.getFirst("search[value]").length > 0) {
+ searchValue = encodeKeyAndGetValue(uriQueryParameters, "search[value]",
null)
+ if (searchValue != null && searchValue.length > 0) {
isSearch = true
- searchValue = uriQueryParameters.getFirst("search[value]")
+ } else {
+ searchValue = null
Review comment:
Is this redundant right?
##########
File path:
core/src/main/scala/org/apache/spark/status/api/v1/StagesResource.scala
##########
@@ -185,15 +187,37 @@ private[v1] class StagesResource extends BaseAppResource {
}
}
+ // The request URL can be raw or encoded. To avoid the parameter key being
+ // encoded twice in queryParameters, try to encode it at most twice and
lookup
+ // it in the queryParameters.
+ def encodeKeyAndGetValue(queryParameters: MultivaluedMap[String, String],
+ key: String, defaultValue: String): String = {
+ var value = queryParameters.getFirst(key)
+ if (value == null) {
+ var encodedKey = URLEncoder.encode(key, "UTF-8")
+ value = queryParameters.getFirst(encodedKey)
+ if (value == null) {
+ encodedKey = URLEncoder.encode(encodedKey, "UTF-8")
+ value = queryParameters.getFirst(encodedKey)
+ if (value == null) {
+ value = defaultValue
+ }
+ }
+ }
+ value
+ }
+
// Performs pagination on the server side
def doPagination(queryParameters: MultivaluedMap[String, String], stageId:
Int,
stageAttemptId: Int, isSearch: Boolean, totalRecords: Int): Seq[TaskData]
= {
var columnNameToSort = queryParameters.getFirst("columnNameToSort")
+ columnNameToSort = URLDecoder.decode(columnNameToSort, "UTF-8")
+ columnNameToSort = URLDecoder.decode(columnNameToSort, "UTF-8")
Review comment:
It's not obvious that the reason why `columnNameToSort` is decoded twice
here.
Also, it may be safe for now but it's difficult to ensure the safety of
decoding twice here.
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
