HyukjinKwon commented on code in PR #58622:
URL: https://github.com/apache/spark/pull/58622#discussion_r3974011213
##########
core/src/test/scala/org/apache/spark/ui/UIUtilsSuite.scala:
##########
@@ -244,4 +244,14 @@ class UIUtilsSuite extends SparkFunSuite {
assert(html.contains("data-toggle-details"), "detailsUINode should use
data-toggle-details")
assert(html.contains("stacktrace-details"), "detailsUINode should contain
stacktrace-details")
}
+
+ test("makeHref only renders http(s) or relative URLs as hyperlinks") {
+ // appUiUrl and webUiAddress come from external registrants.
+ assert(UIUtils.makeHref(proxy = false, "app-1", "http://host:4040";) ===
"http://host:4040";)
+ assert(UIUtils.makeHref(proxy = false, "app-1", "https://host:4040";) ===
"https://host:4040";)
+ assert(UIUtils.makeHref(proxy = false, "app-1", "/relative/path") ===
"/relative/path")
+ assert(UIUtils.makeHref(proxy = false, "app-1", null) === "#")
Review Comment:
The `null` input short-circuits at `href != null` before the URI parse, so
this test never exercises the two branches that implement the PR's core
behavior: a non-null value with a disallowed scheme, and a value that trips
`URISyntaxException`. Since the test is named "only renders http(s) or relative
URLs as hyperlinks", consider adding assertions for the reject path, e.g.:
```scala
assert(UIUtils.makeHref(proxy = false, "app-1", "javascript:alert(1)") ===
"#")
assert(UIUtils.makeHref(proxy = false, "app-1", "ht tp://bad url") === "#")
// malformed -> URISyntaxException
```
Non-blocking.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
