Skip to content

Commit f8414f9

Browse files
fioan89fioan89
committed
impl: strict URL validation for the URI handling
This commit rejects any URL that is opaque, not hierarchical, not using http or https protocol, or it misses the hostname.
1 parent 06f0feb commit f8414f9

File tree

4 files changed

+42
-25
lines changed

4 files changed

+42
-25
lines changed

‎src/main/kotlin/com/coder/gateway/util/LinkHandler.kt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ open class LinkHandler(
3737
if (deploymentURL.isNullOrBlank()) {
3838
throw MissingArgumentException("Query parameter \"$URL\" is missing")
3939
}
40+
val result = deploymentURL.validateStrictWebUrl()
41+
if (result is WebUrlValidationResult.Invalid) {
42+
throw IllegalArgumentException(result.reason)
43+
}
4044

4145
val queryTokenRaw = parameters.token()
4246
val queryToken = if (!queryTokenRaw.isNullOrBlank()) {

‎src/main/kotlin/com/coder/gateway/util/URLExtensions.kt

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,18 @@ fun URL.withPath(path: String): URL = URL(
1515
if (path.startsWith("/")) path else "/$path",
1616
)
1717

18-
fun URI.validateStrictWebUrl(): WebUrlValidationResult = try {
18+
19+
fun String.validateStrictWebUrl(): WebUrlValidationResult = try {
20+
val uri = URI(this)
21+
1922
when {
20-
isOpaque -> Invalid("$this is opaque, instead of hierarchical")
21-
!isAbsolute -> Invalid("$this is relative, it must be absolute")
22-
scheme?.lowercase() !in setOf("http", "https") -> Invalid("Scheme for $this must be either http or https")
23-
authority.isNullOrBlank() -> Invalid("$this does not have a hostname")
23+
uri.isOpaque -> Invalid("$this is opaque, instead of hierarchical")
24+
!uri.isAbsolute -> Invalid("$this is relative, it must be absolute")
25+
uri.scheme?.lowercase() !in setOf("http", "https") ->
26+
Invalid("Scheme for $this must be either http or https")
27+
28+
uri.authority.isNullOrBlank() ->
29+
Invalid("$this does not have a hostname")
2430
else -> WebUrlValidationResult.Valid
2531
}
2632
} catch (e: Exception) {

‎src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -565,9 +565,9 @@ class CoderWorkspacesStepView :
565565
private fun maybeAskTokenThenConnect(error: String? = null) {
566566
val oldURL = fields.coderURL
567567
component.apply() // Force bindings to be filled.
568-
val newURL = fields.coderURL.toURL()
569568
if (settings.requireTokenAuth) {
570-
val result = newURL.toURI().validateStrictWebUrl()
569+
val result = fields.coderURL.validateStrictWebUrl()
570+
val newURL = fields.coderURL.toURL()
571571
if (result is WebUrlValidationResult.Invalid) {
572572
tfUrlComment.apply {
573573
this?.foreground = UIUtil.getErrorForeground()
@@ -590,7 +590,7 @@ class CoderWorkspacesStepView :
590590
maybeAskTokenThenConnect(it)
591591
}
592592
} else {
593-
connect(newURL, null)
593+
connect(fields.coderURL.toURL(), null)
594594
}
595595
}
596596

‎src/test/kotlin/com/coder/gateway/util/URLExtensionsTest.kt

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -60,57 +60,64 @@ internal class URLExtensionsTest {
6060
)
6161
}
6262
}
63-
6463
@Test
6564
fun `valid http URL should return Valid`() {
66-
val uri = URI("http://coder.com")
67-
val result = uri.validateStrictWebUrl()
65+
val result = "http://coder.com".validateStrictWebUrl()
6866
assertEquals(WebUrlValidationResult.Valid, result)
6967
}
7068

7169
@Test
7270
fun `valid https URL with path and query should return Valid`() {
73-
val uri = URI("https://coder.com/bin/coder-linux-amd64?query=1")
74-
val result = uri.validateStrictWebUrl()
71+
val result = "https://coder.com/bin/coder-linux-amd64?query=1".validateStrictWebUrl()
7572
assertEquals(WebUrlValidationResult.Valid, result)
7673
}
7774

7875
@Test
7976
fun `relative URL should return Invalid with appropriate message`() {
80-
val uri = URI("/bin/coder-linux-amd64")
81-
val result = uri.validateStrictWebUrl()
77+
val url = "/bin/coder-linux-amd64"
78+
val result = url.validateStrictWebUrl()
8279
assertEquals(
83-
WebUrlValidationResult.Invalid("$uri is relative, it must be absolute"),
80+
WebUrlValidationResult.Invalid("$url is relative, it must be absolute"),
8481
result
8582
)
8683
}
8784

8885
@Test
8986
fun `opaque URI like mailto should return Invalid`() {
90-
val uri = URI("mailto:user@coder.com")
91-
val result = uri.validateStrictWebUrl()
87+
val url = "mailto:user@coder.com"
88+
val result = url.validateStrictWebUrl()
9289
assertEquals(
93-
WebUrlValidationResult.Invalid("$uri is opaque, instead of hierarchical"),
90+
WebUrlValidationResult.Invalid("$url is opaque, instead of hierarchical"),
9491
result
9592
)
9693
}
9794

9895
@Test
9996
fun `unsupported scheme like ftp should return Invalid`() {
100-
val uri = URI("ftp://coder.com")
101-
val result = uri.validateStrictWebUrl()
97+
val url = "ftp://coder.com"
98+
val result = url.validateStrictWebUrl()
10299
assertEquals(
103-
WebUrlValidationResult.Invalid("Scheme for $uri must be either http or https"),
100+
WebUrlValidationResult.Invalid("Scheme for $url must be either http or https"),
104101
result
105102
)
106103
}
107104

108105
@Test
109106
fun `http URL with missing authority should return Invalid`() {
110-
val uri = URI("http:///bin/coder-linux-amd64")
111-
val result = uri.validateStrictWebUrl()
107+
val url = "http:///bin/coder-linux-amd64"
108+
val result = url.validateStrictWebUrl()
109+
assertEquals(
110+
WebUrlValidationResult.Invalid("$url does not have a hostname"),
111+
result
112+
)
113+
}
114+
115+
@Test
116+
fun `malformed URI should return Invalid with parsing error message`() {
117+
val url = "http://[invalid-uri]"
118+
val result = url.validateStrictWebUrl()
112119
assertEquals(
113-
WebUrlValidationResult.Invalid("$uri does not have a hostname"),
120+
WebUrlValidationResult.Invalid("$url could not be parsed as a URI reference"),
114121
result
115122
)
116123
}

0 commit comments

Comments
 (0)