From 84fe451ec750a946330285154bb41c98d4139a6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Thu, 9 Mar 2023 15:40:25 +0100 Subject: [PATCH 1/2] stack/loader: Ignore cmd.exe special env variables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows, ignore all variables that start with "=" when building an environment variables map for stack. For MS-DOS compatibility cmd.exe can set some special environment variables that start with a "=" characters, which breaks the general assumption that the first encountered "=" separates a variable name from variable value and causes trouble when parsing. These variables don't seem to be documented anywhere, but they are described by some third-party sources and confirmed empirically on my Windows installation. Useful sources: https://devblogs.microsoft.com/oldnewthing/20100506-00/?p=14133 https://ss64.com/nt/syntax-variables.html Known variables: - `=ExitCode` stores the exit code returned by external command (in hex format) - `=ExitCodeAscii` - same as above, except the value is the ASCII representation of the code (so exit code 65 (0x41) becomes 'A'). - `=::=::\` and friends - store drive specific working directory. There is one env variable for each separate drive letter that was accessed in the shell session and stores the working directory for that specific drive. The general format for these is: `=:=` (key=`=:`, value=``) where is a working directory for the drive that is assigned to the letter A couple of examples: `=C:=C:\some\dir` (key: `=C:`, value: `C:\some\dir`) `=D:=D:\some\other\dir` (key: `=C:`, value: `C:\some\dir`) `=Z:=Z:\` (key: `=Z:`, value: `Z:\`) `=::=::\` is the one that seems to be always set and I'm not exactly sure what this one is for (what's drive `::`?). Others are set as soon as you CD to a path on some drive. Considering that you start a cmd.exe also has some working directory, there are 2 of these on start. All these variables can be safely ignored because they can't be deliberately set by the user, their meaning is only relevant to the cmd.exe session and they're all are related to the MS-DOS/Batch feature that are irrelevant for us. Signed-off-by: Paweł Gronowski (cherry picked from commit a47058bbd511cd98ea81db512abddb09c9eb1365) Signed-off-by: Sebastiaan van Stijn --- cli/command/stack/loader/loader.go | 14 +++++++++++ cli/command/stack/loader/loader_test.go | 32 +++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/cli/command/stack/loader/loader.go b/cli/command/stack/loader/loader.go index 105e84e644..8b5359dd95 100644 --- a/cli/command/stack/loader/loader.go +++ b/cli/command/stack/loader/loader.go @@ -5,6 +5,7 @@ import ( "io" "os" "path/filepath" + "runtime" "sort" "strings" @@ -104,6 +105,19 @@ func GetConfigDetails(composefiles []string, stdin io.Reader) (composetypes.Conf func buildEnvironment(env []string) (map[string]string, error) { result := make(map[string]string, len(env)) for _, s := range env { + if runtime.GOOS == "windows" && len(s) > 0 { + // cmd.exe can have special environment variables which names start with "=". + // They are only there for MS-DOS compatibility and we should ignore them. + // See TestBuildEnvironment for examples. + // + // https://ss64.com/nt/syntax-variables.html + // https://devblogs.microsoft.com/oldnewthing/20100506-00/?p=14133 + // https://github.com/docker/cli/issues/4078 + if s[0] == '=' { + continue + } + } + k, v, ok := strings.Cut(s, "=") if !ok || k == "" { return result, errors.Errorf("unexpected environment %q", s) diff --git a/cli/command/stack/loader/loader_test.go b/cli/command/stack/loader/loader_test.go index 6ddca65bb8..6c0da17aa3 100644 --- a/cli/command/stack/loader/loader_test.go +++ b/cli/command/stack/loader/loader_test.go @@ -3,6 +3,7 @@ package loader import ( "os" "path/filepath" + "runtime" "strings" "testing" @@ -45,3 +46,34 @@ services: assert.Check(t, is.Equal("3.0", details.ConfigFiles[0].Config["version"])) assert.Check(t, is.Len(details.Environment, len(os.Environ()))) } + +func TestBuildEnvironment(t *testing.T) { + inputEnv := []string{ + "LEGIT_VAR=LEGIT_VALUE", + "EMPTY_VARIABLE=", + } + + if runtime.GOOS == "windows" { + inputEnv = []string{ + "LEGIT_VAR=LEGIT_VALUE", + + // cmd.exe has some special environment variables which start with "=". + // These should be ignored as they're only there for MS-DOS compatibility. + "=ExitCode=00000041", + "=ExitCodeAscii=A", + `=C:=C:\some\dir`, + `=D:=D:\some\different\dir`, + `=X:=X:\`, + `=::=::\`, + + "EMPTY_VARIABLE=", + } + } + + env, err := buildEnvironment(inputEnv) + assert.NilError(t, err) + + assert.Check(t, is.Len(env, 2)) + assert.Check(t, is.Equal("LEGIT_VALUE", env["LEGIT_VAR"])) + assert.Check(t, is.Equal("", env["EMPTY_VARIABLE"])) +} From b61b5a9878d7948b96e9c8f8a1665e1e1eb615d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Thu, 9 Mar 2023 16:56:34 +0100 Subject: [PATCH 2/2] stack: Change unexpected environment variable error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the error more specific by stating that it's caused by a specific environment variable and not an environment as a whole. Also don't escape the variable to make it more readable. Signed-off-by: Paweł Gronowski (cherry picked from commit 012b77952e6b2acad5f4df5d9f015830e572f6f5) Signed-off-by: Sebastiaan van Stijn --- cli/command/stack/loader/loader.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/command/stack/loader/loader.go b/cli/command/stack/loader/loader.go index 8b5359dd95..39810d8832 100644 --- a/cli/command/stack/loader/loader.go +++ b/cli/command/stack/loader/loader.go @@ -120,7 +120,7 @@ func buildEnvironment(env []string) (map[string]string, error) { k, v, ok := strings.Cut(s, "=") if !ok || k == "" { - return result, errors.Errorf("unexpected environment %q", s) + return result, errors.Errorf("unexpected environment variable '%s'", s) } // value may be set, but empty if "s" is like "K=", not "K". result[k] = v