gengliangwang commented on a change in pull request #30552:
URL: https://github.com/apache/spark/pull/30552#discussion_r533475978
##########
File path: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala
##########
@@ -401,17 +401,13 @@ private[spark] object JettyUtils extends Logging {
uri.append(rest)
}
- val rewrittenURI = URI.create(uri.toString())
- if (query != null) {
- return new URI(
- rewrittenURI.getScheme(),
- rewrittenURI.getAuthority(),
- rewrittenURI.getPath(),
- query,
- rewrittenURI.getFragment()
- ).normalize()
+ val queryString = if (query == null) {
+ ""
+ } else {
+ s"?$query"
}
- rewrittenURI.normalize()
+ // SPARK-33611: use method `URI.create` to avoid percent-encoding twice on
the query string.
+ URI.create(uri.toString() + queryString).normalize()
Review comment:
Thanks for the suggestion, Sean.
> toString() is unnecesary here BTW.
`uri` is `StringBuilder` here. The original code also use toString().
> So this relies on query already being escaped. That may be fine for the
logic of this code, although in another world it would have been better to let
this method take unescaped query strings and manage it here.
The query string here should be encoded already. I am not very familiar
with this topic. Is there any better solution to handle both escaped/unescaped
query string?
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
