Bring parity with cli to header command (#405)

- Do not include stderr.
- Allow no output.

Author: code-asher

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

@@ -1,7 +1,8 @@
 package com.coder.gateway.util
 
-import java.net.URL
 import org.zeroturnaround.exec.ProcessExecutor
+import java.io.OutputStream
+import java.net.URL
 
 private val newlineRegex = "\r?\n".toRegex()
 private val endingNewlineRegex = "\r?\n$".toRegex()
@@ -14,24 +15,36 @@ fun getHeaders(url: URL, headerCommand: String?): Map<String, String> {
         OS.WINDOWS -> Pair("cmd.exe", "/c")
         else -> Pair("sh", "-c")
     }
-    return ProcessExecutor()
+    val output = ProcessExecutor()
         .command(shell, caller, headerCommand)
         .environment("CODER_URL", url.toString())
+        // By default stderr is in the output, but we want to ignore it.  stderr
+        // will still be included in the exception if something goes wrong.
+        .redirectError(OutputStream.nullOutputStream())
         .exitValues(0)
         .readOutput(true)
         .execute()
         .outputUTF8()
+
+    // The Coder CLI will allow no output, but not blank lines.  Possibly we
+    // should skip blank lines, but it is better to have parity so commands will
+    // not sometimes work in one context and not another.
+    return if (output == "") mapOf() else output
         .replaceFirst(endingNewlineRegex, "")
         .split(newlineRegex)
         .associate {
             // Header names cannot be blank or contain whitespace and the Coder
-            // CLI requires that there be an equals sign (the value can be blank
-            // though).  The second case is taken care of by the destructure
-            // here, as it will throw if there are not enough parts.
-            val (name, value) = it.split("=", limit=2)
-            if (name.contains(" ") || name == "") {
-                throw Exception("\"$name\" is not a valid header name")
+            // CLI requires there be an equals sign (the value can be blank).
+            val parts = it.split("=", limit=2)
+            if (it.isBlank()) {
+                throw Exception("Blank lines are not allowed")
+            } else if (parts.size != 2) {
+                throw Exception("Header \"$it\" does not have two parts")
+            } else if (parts[0].isBlank()) {
+                throw Exception("Header name is missing in \"$it\"")
+            } else if (parts[0].contains(" ")) {
+                throw Exception("Header name cannot contain spaces, got \"${parts[0]}\"")
             }
-            name to value
+            parts[0] to parts[1]
         }
 }

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

@@ -1,11 +1,11 @@
 package com.coder.gateway.util
 
+import java.net.URL
 import kotlin.test.Test
+import kotlin.test.assertContains
 import kotlin.test.assertEquals
 import kotlin.test.assertFailsWith
 
-import java.net.URL
-
 internal class HeadersTest {
     @Test
     fun testGetHeadersOK() {
@@ -19,6 +19,11 @@ internal class HeadersTest {
             "printf 'foo=bar='"             to mapOf("foo" to "bar="),
             "printf 'foo=bar=baz'"          to mapOf("foo" to "bar=baz"),
             "printf 'foo='"                 to mapOf("foo" to ""),
+            "printf 'foo=bar   '"           to mapOf("foo" to "bar   "),
+            "exit 0"                        to mapOf(),
+            "printf ''"                     to mapOf(),
+            "printf 'ignore me' >&2"        to mapOf(),
+            "printf 'foo=bar' && printf 'ignore me' >&2" to mapOf("foo" to "bar"),
         )
         tests.forEach{
             assertEquals(
@@ -30,22 +35,25 @@ internal class HeadersTest {
 
     @Test
     fun testGetHeadersFail() {
-        val tests = listOf(
-            "printf 'foo=bar\\r\\n\\r\\n'",
-            "printf '\\r\\nfoo=bar'",
-            "printf '=foo'",
-            "printf 'foo'",
-            "printf '  =foo'",
-            "printf 'foo  =bar'",
-            "printf 'foo  foo=bar'",
-            "printf ''",
-            "exit 0",
-            "exit 1",
+        val tests = mapOf(
+            "printf '=foo'"                 to "Header name is missing in \"=foo\"",
+            "printf 'foo'"                  to "Header \"foo\" does not have two parts",
+            "printf '  =foo'"               to "Header name is missing in \"  =foo\"",
+            "printf 'foo  =bar'"            to "Header name cannot contain spaces, got \"foo  \"",
+            "printf 'foo  foo=bar'"         to "Header name cannot contain spaces, got \"foo  foo\"",
+            "printf '  foo=bar  '"          to "Header name cannot contain spaces, got \"  foo\"",
+            "exit 1"                        to "Unexpected exit value: 1",
+            "printf 'foobar' >&2 && exit 1" to "foobar",
+            "printf 'foo=bar\\r\\n\\r\\n'"  to "Blank lines are not allowed",
+            "printf '\\r\\nfoo=bar'"        to "Blank lines are not allowed",
+            "printf '\\r\\n'"               to "Blank lines are not allowed",
+            "printf 'f=b\\r\\n\\r\\nb=q'"   to "Blank lines are not allowed"
         )
         tests.forEach{
-            assertFailsWith(
+            val ex = assertFailsWith(
                 exceptionClass = Exception::class,
-                block = { getHeaders(URL("http://localhost"), it) })
+                block = { getHeaders(URL("http://localhost"), it.key) })
+            assertContains(ex.message.toString(), it.value)
         }
     }