srowen commented on a change in pull request #30552:
URL: https://github.com/apache/spark/pull/30552#discussion_r533479958
##########
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:
Oh right, OK, ignore the toString() comment.
I just mean that semantically, encoding is a URI detail. The program should
probably not worry about it except when going to and from URIs. But that's not
how this code is structured. It clearly already expects and escaped string as
argument, which then means callers have to deal with it. But, I don't think
it's necessary to change. If it is escaped then yes this is a fix.
----------------------------------------------------------------
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]
