From 649cdff7e81846a68f25d062914e132f9c0df9d6 Mon Sep 17 00:00:00 2001 From: Asher Date: Tue, 23 Apr 2024 09:25:24 -0800 Subject: [PATCH] Bring parity with cli to header command - Do not include stderr. - Allow no output. --- .../kotlin/com/coder/gateway/util/Headers.kt | 31 ++++++++++----- .../com/coder/gateway/util/HeadersTest.kt | 38 +++++++++++-------- 2 files changed, 45 insertions(+), 24 deletions(-) diff --git a/src/main/kotlin/com/coder/gateway/util/Headers.kt b/src/main/kotlin/com/coder/gateway/util/Headers.kt index d398199c..bed8a357 100644 --- a/src/main/kotlin/com/coder/gateway/util/Headers.kt +++ b/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 { 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] } } diff --git a/src/test/kotlin/com/coder/gateway/util/HeadersTest.kt b/src/test/kotlin/com/coder/gateway/util/HeadersTest.kt index 1ed21ea1..c34fbf8e 100644 --- a/src/test/kotlin/com/coder/gateway/util/HeadersTest.kt +++ b/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) } }