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.

Author: fioan89

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

@@ -37,6 +37,10 @@ open class LinkHandler(
         if (deploymentURL.isNullOrBlank()) {
             throw MissingArgumentException("Query parameter \"$URL\" is missing")
         }
+        val result = deploymentURL.validateStrictWebUrl()
+        if (result is WebUrlValidationResult.Invalid) {
+            throw IllegalArgumentException(result.reason)
+        }
 
         val queryTokenRaw = parameters.token()
         val queryToken = if (!queryTokenRaw.isNullOrBlank()) {

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

@@ -15,12 +15,18 @@ fun URL.withPath(path: String): URL = URL(
     if (path.startsWith("/")) path else "/$path",
 )
 
-fun URI.validateStrictWebUrl(): WebUrlValidationResult = try {
+
+fun String.validateStrictWebUrl(): WebUrlValidationResult = try {
+    val uri = URI(this)
+
     when {
-        isOpaque -> Invalid("$this is opaque, instead of hierarchical")
-        !isAbsolute -> Invalid("$this is relative, it must be absolute")
-        scheme?.lowercase() !in setOf("http", "https") -> Invalid("Scheme for $this must be either http or https")
-        authority.isNullOrBlank() -> Invalid("$this does not have a hostname")
+        uri.isOpaque -> Invalid("$this is opaque, instead of hierarchical")
+        !uri.isAbsolute -> Invalid("$this is relative, it must be absolute")
+        uri.scheme?.lowercase() !in setOf("http", "https") ->
+            Invalid("Scheme for $this must be either http or https")
+
+        uri.authority.isNullOrBlank() ->
+            Invalid("$this does not have a hostname")
         else -> WebUrlValidationResult.Valid
     }
 } catch (e: Exception) {

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

@@ -565,9 +565,9 @@ class CoderWorkspacesStepView :
     private fun maybeAskTokenThenConnect(error: String? = null) {
         val oldURL = fields.coderURL
         component.apply() // Force bindings to be filled.
-        val newURL = fields.coderURL.toURL()
         if (settings.requireTokenAuth) {
-            val result = newURL.toURI().validateStrictWebUrl()
+            val result = fields.coderURL.validateStrictWebUrl()
+            val newURL = fields.coderURL.toURL()
             if (result is WebUrlValidationResult.Invalid) {
                 tfUrlComment.apply {
                     this?.foreground = UIUtil.getErrorForeground()
@@ -590,7 +590,7 @@ class CoderWorkspacesStepView :
                 maybeAskTokenThenConnect(it)
             }
         } else {
-            connect(newURL, null)
+            connect(fields.coderURL.toURL(), null)
         }
     }
 

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

@@ -60,57 +60,64 @@ internal class URLExtensionsTest {
             )
         }
     }
-
     @Test
     fun `valid http URL should return Valid`() {
-        val uri = URI("http://coder.com")
-        val result = uri.validateStrictWebUrl()
+        val result = "http://coder.com".validateStrictWebUrl()
         assertEquals(WebUrlValidationResult.Valid, result)
     }
 
     @Test
     fun `valid https URL with path and query should return Valid`() {
-        val uri = URI("https://coder.com/bin/coder-linux-amd64?query=1")
-        val result = uri.validateStrictWebUrl()
+        val result = "https://coder.com/bin/coder-linux-amd64?query=1".validateStrictWebUrl()
         assertEquals(WebUrlValidationResult.Valid, result)
     }
 
     @Test
     fun `relative URL should return Invalid with appropriate message`() {
-        val uri = URI("/bin/coder-linux-amd64")
-        val result = uri.validateStrictWebUrl()
+        val url = "/bin/coder-linux-amd64"
+        val result = url.validateStrictWebUrl()
         assertEquals(
-            WebUrlValidationResult.Invalid("$uri is relative, it must be absolute"),
+            WebUrlValidationResult.Invalid("$url is relative, it must be absolute"),
             result
         )
     }
 
     @Test
     fun `opaque URI like mailto should return Invalid`() {
-        val uri = URI("mailto:[email protected]")
-        val result = uri.validateStrictWebUrl()
+        val url = "mailto:[email protected]"
+        val result = url.validateStrictWebUrl()
         assertEquals(
-            WebUrlValidationResult.Invalid("$uri is opaque, instead of hierarchical"),
+            WebUrlValidationResult.Invalid("$url is opaque, instead of hierarchical"),
             result
         )
     }
 
     @Test
     fun `unsupported scheme like ftp should return Invalid`() {
-        val uri = URI("ftp://coder.com")
-        val result = uri.validateStrictWebUrl()
+        val url = "ftp://coder.com"
+        val result = url.validateStrictWebUrl()
         assertEquals(
-            WebUrlValidationResult.Invalid("Scheme for $uri must be either http or https"),
+            WebUrlValidationResult.Invalid("Scheme for $url must be either http or https"),
             result
         )
     }
 
     @Test
     fun `http URL with missing authority should return Invalid`() {
-        val uri = URI("http:///bin/coder-linux-amd64")
-        val result = uri.validateStrictWebUrl()
+        val url = "http:///bin/coder-linux-amd64"
+        val result = url.validateStrictWebUrl()
+        assertEquals(
+            WebUrlValidationResult.Invalid("$url does not have a hostname"),
+            result
+        )
+    }
+
+    @Test
+    fun `malformed URI should return Invalid with parsing error message`() {
+        val url = "http://[invalid-uri]"
+        val result = url.validateStrictWebUrl()
         assertEquals(
-            WebUrlValidationResult.Invalid("$uri does not have a hostname"),
+            WebUrlValidationResult.Invalid("$url could not be parsed as a URI reference"),
             result
         )
     }