You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/04/24 04:48:57 UTC
[GitHub] [spark] mdianjun opened a new pull request #32317: [SPARK-33195] Fix stages/stage UI page fails because of uri parameters encoded twice
mdianjun opened a new pull request #32317:
URL: https://github.com/apache/spark/pull/32317
### What changes were proposed in this pull request?
The similar PR is: https://github.com/apache/spark/pull/29271, which is reproduced with HTTPS reverse proxy.
This PR is used to fix a similar issue in HTTP reverse proxy, described at https://issues.apache.org/jira/browse/SPARK-33195.
### Why are the changes needed?
Fix a UI issue when HTTP is enabled
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
A new Unit test + manually test on a cluster.
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32317:
URL: https://github.com/apache/spark/pull/32317#issuecomment-828094767
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42534/
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32317:
URL: https://github.com/apache/spark/pull/32317#issuecomment-828135395
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138015/
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32317:
URL: https://github.com/apache/spark/pull/32317#issuecomment-828090714
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #32317:
URL: https://github.com/apache/spark/pull/32317#issuecomment-826523987
@mdianjun
> A new Unit test + manually test on a cluster (Added later).
Could you add the new test?
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #32317:
URL: https://github.com/apache/spark/pull/32317#issuecomment-828065952
Hi @mdianjun , could you re-trigger the test on your repository?
https://github.com/marsno1/spark/runs/2446289248?check_suite_focus=true
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #32317:
URL: https://github.com/apache/spark/pull/32317#issuecomment-837075235
@mdianjun Sorry for the late reply.
This PR is for the case that `spark.ui.proxyRedirectUri` is `true` right?
If so, should we rather fix `ProxyRedirectHandler` ?
I think the current solution seems a little bit dangerous.
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32317:
URL: https://github.com/apache/spark/pull/32317#issuecomment-828135395
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138015/
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak edited a comment on pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
sarutak edited a comment on pull request #32317:
URL: https://github.com/apache/spark/pull/32317#issuecomment-826523987
@mdianjun
> A new Unit test + manually test on a cluster (Added later).
Could you add the new test?
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #32317:
URL: https://github.com/apache/spark/pull/32317#issuecomment-826273782
cc @sarutak and @gengliangwang FYI
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32317:
URL: https://github.com/apache/spark/pull/32317#issuecomment-828072747
**[Test build #138015 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138015/testReport)** for PR 32317 at commit [`7f7adb8`](https://github.com/apache/spark/commit/7f7adb8d3b8905c76d89100a92f25ceee70fceeb).
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] github-actions[bot] commented on pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #32317:
URL: https://github.com/apache/spark/pull/32317#issuecomment-901511991
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] github-actions[bot] closed pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #32317:
URL: https://github.com/apache/spark/pull/32317
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #32317:
URL: https://github.com/apache/spark/pull/32317#issuecomment-828066077
ok to test.
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #32317:
URL: https://github.com/apache/spark/pull/32317#issuecomment-826523987
@mdianjun
> A new Unit test + manually test on a cluster (Added later).
Could you add the new test?
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #32317:
URL: https://github.com/apache/spark/pull/32317#issuecomment-826273782
cc @sarutak and @gengliangwang FYI
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32317:
URL: https://github.com/apache/spark/pull/32317#issuecomment-828072747
**[Test build #138015 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138015/testReport)** for PR 32317 at commit [`7f7adb8`](https://github.com/apache/spark/commit/7f7adb8d3b8905c76d89100a92f25ceee70fceeb).
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak edited a comment on pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
sarutak edited a comment on pull request #32317:
URL: https://github.com/apache/spark/pull/32317#issuecomment-837075235
@mdianjun Sorry for the late reply.
I left some comments.
I think the current solution seems a little bit dangerous.
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32317:
URL: https://github.com/apache/spark/pull/32317#issuecomment-828094767
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42534/
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32317:
URL: https://github.com/apache/spark/pull/32317#issuecomment-826036743
Can one of the admins verify this patch?
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32317:
URL: https://github.com/apache/spark/pull/32317#issuecomment-826036743
Can one of the admins verify this patch?
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak edited a comment on pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
sarutak edited a comment on pull request #32317:
URL: https://github.com/apache/spark/pull/32317#issuecomment-826523987
@mdianjun
> A new Unit test + manually test on a cluster (Added later).
Could you add the new test?
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32317:
URL: https://github.com/apache/spark/pull/32317#issuecomment-828131030
**[Test build #138015 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138015/testReport)** for PR 32317 at commit [`7f7adb8`](https://github.com/apache/spark/commit/7f7adb8d3b8905c76d89100a92f25ceee70fceeb).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on a change in pull request #32317: [SPARK-33195][UI] Fix stages/stage UI page fails because of uri parameters encoded twice
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org