[GitHub] [spark] gengliangwang commented on a change in pull request #30552: [SPARK-33611][UI] Avoid encoding twice on the query parameter of rewritten proxy URL

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org