From 5ef2e442de2be5446002945d8308314592d7c3fd Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Mon, 2 Jan 2023 16:24:40 +0100 Subject: [PATCH 1/4] internal/flags: use filepath.Clean instead of path.Clean --- internal/flags/flags.go | 4 ++-- internal/flags/flags_test.go | 12 +++++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/internal/flags/flags.go b/internal/flags/flags.go index 0ae2c6a512e..9e371963297 100644 --- a/internal/flags/flags.go +++ b/internal/flags/flags.go @@ -23,7 +23,7 @@ import ( "math/big" "os" "os/user" - "path" + "path/filepath" "strings" "github.com/ethereum/go-ethereum/common/math" @@ -319,7 +319,7 @@ func expandPath(p string) string { p = home + p[1:] } } - return path.Clean(os.ExpandEnv(p)) + return filepath.Clean(os.ExpandEnv(p)) } func HomeDir() string { diff --git a/internal/flags/flags_test.go b/internal/flags/flags_test.go index a0d4af7ca36..f9073cc97e3 100644 --- a/internal/flags/flags_test.go +++ b/internal/flags/flags_test.go @@ -25,11 +25,13 @@ import ( func TestPathExpansion(t *testing.T) { user, _ := user.Current() tests := map[string]string{ - "/home/someuser/tmp": "/home/someuser/tmp", - "~/tmp": user.HomeDir + "/tmp", - "~thisOtherUser/b/": "~thisOtherUser/b", - "$DDDXXX/a/b": "/tmp/a/b", - "/a/b/": "/a/b", + "/home/someuser/tmp": "/home/someuser/tmp", + "~/tmp": user.HomeDir + "/tmp", + "~thisOtherUser/b/": "~thisOtherUser/b", + "$DDDXXX/a/b": "/tmp/a/b", + "/a/b/": "/a/b", + "C:\\Documents\\Newsletters\\": "C:\\Documents\\Newsletters\\", + "C:\\": "C:\\", } os.Setenv("DDDXXX", "/tmp") for test, expected := range tests { From 77d277814fa25e483081308c9b81d68bab3e53b9 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Tue, 3 Jan 2023 15:14:02 +0100 Subject: [PATCH 2/4] internal/flags: fix windows pipe issue --- internal/flags/flags.go | 4 ++++ internal/flags/flags_test.go | 15 ++++++++------- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/internal/flags/flags.go b/internal/flags/flags.go index 9e371963297..8e4f5e0d930 100644 --- a/internal/flags/flags.go +++ b/internal/flags/flags.go @@ -314,6 +314,10 @@ func GlobalBig(ctx *cli.Context, name string) *big.Int { // 3. cleans the path, e.g. /a/b/../c -> /a/c // Note, it has limitations, e.g. ~someuser/tmp will not be expanded func expandPath(p string) string { + // Named pipes are not file paths on windows, ignore + if strings.HasPrefix(p, "\\\\.\\pipe") { + return p + } if strings.HasPrefix(p, "~/") || strings.HasPrefix(p, "~\\") { if home := HomeDir(); home != "" { p = home + p[1:] diff --git a/internal/flags/flags_test.go b/internal/flags/flags_test.go index f9073cc97e3..7fc80d178bd 100644 --- a/internal/flags/flags_test.go +++ b/internal/flags/flags_test.go @@ -25,13 +25,14 @@ import ( func TestPathExpansion(t *testing.T) { user, _ := user.Current() tests := map[string]string{ - "/home/someuser/tmp": "/home/someuser/tmp", - "~/tmp": user.HomeDir + "/tmp", - "~thisOtherUser/b/": "~thisOtherUser/b", - "$DDDXXX/a/b": "/tmp/a/b", - "/a/b/": "/a/b", - "C:\\Documents\\Newsletters\\": "C:\\Documents\\Newsletters\\", - "C:\\": "C:\\", + "/home/someuser/tmp": "/home/someuser/tmp", + "~/tmp": user.HomeDir + "/tmp", + "~thisOtherUser/b/": "~thisOtherUser/b", + "$DDDXXX/a/b": "/tmp/a/b", + "/a/b/": "/a/b", + "C:\\Documents\\Newsletters\\": "C:\\Documents\\Newsletters\\", + "C:\\": "C:\\", + "\\\\.\\pipe\\\\pipe\\geth621383": "\\\\.\\pipe\\\\pipe\\geth621383", } os.Setenv("DDDXXX", "/tmp") for test, expected := range tests { From 7c9aa53e34c7098085893a5fba4fef23bb1b60a0 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Mon, 9 Jan 2023 11:17:41 +0100 Subject: [PATCH 3/4] internal/flags: modify test for windows --- internal/flags/flags_test.go | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/internal/flags/flags_test.go b/internal/flags/flags_test.go index 7fc80d178bd..1ca512f84aa 100644 --- a/internal/flags/flags_test.go +++ b/internal/flags/flags_test.go @@ -19,21 +19,38 @@ package flags import ( "os" "os/user" + "runtime" "testing" ) func TestPathExpansion(t *testing.T) { user, _ := user.Current() - tests := map[string]string{ - "/home/someuser/tmp": "/home/someuser/tmp", - "~/tmp": user.HomeDir + "/tmp", - "~thisOtherUser/b/": "~thisOtherUser/b", - "$DDDXXX/a/b": "/tmp/a/b", - "/a/b/": "/a/b", - "C:\\Documents\\Newsletters\\": "C:\\Documents\\Newsletters\\", - "C:\\": "C:\\", - "\\\\.\\pipe\\\\pipe\\geth621383": "\\\\.\\pipe\\\\pipe\\geth621383", + var tests map[string]string + + if runtime.GOOS == "windows" { + tests = map[string]string{ + "/home/someuser/tmp": "\\home\\someuser\\tmp", + "~/tmp": user.HomeDir + "\\tmp", + "~thisOtherUser/b/": "~thisOtherUser\\b", + "$DDDXXX/a/b": "\\tmp\\a\\b", + "/a/b/": "\\a\\b", + "C:\\Documents\\Newsletters\\": "C:\\Documents\\Newsletters\\", + "C:\\": "C:\\", + "\\\\.\\pipe\\\\pipe\\geth621383": "\\\\.\\pipe\\\\pipe\\geth621383", + } + } else { + tests = map[string]string{ + "/home/someuser/tmp": "/home/someuser/tmp", + "~/tmp": user.HomeDir + "/tmp", + "~thisOtherUser/b/": "~thisOtherUser/b", + "$DDDXXX/a/b": "/tmp/a/b", + "/a/b/": "/a/b", + "C:\\Documents\\Newsletters\\": "C:\\Documents\\Newsletters\\", + "C:\\": "C:\\", + "\\\\.\\pipe\\\\pipe\\geth621383": "\\\\.\\pipe\\\\pipe\\geth621383", + } } + os.Setenv("DDDXXX", "/tmp") for test, expected := range tests { got := expandPath(test) From cd559a48bef51681e861aad50bcb1371e2921df5 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Mon, 9 Jan 2023 12:47:51 +0100 Subject: [PATCH 4/4] internal/flags: use backticks, fix test --- internal/flags/flags.go | 2 +- internal/flags/flags_test.go | 36 ++++++++++++++++++------------------ 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/internal/flags/flags.go b/internal/flags/flags.go index 8e4f5e0d930..b0756b4e0a1 100644 --- a/internal/flags/flags.go +++ b/internal/flags/flags.go @@ -315,7 +315,7 @@ func GlobalBig(ctx *cli.Context, name string) *big.Int { // Note, it has limitations, e.g. ~someuser/tmp will not be expanded func expandPath(p string) string { // Named pipes are not file paths on windows, ignore - if strings.HasPrefix(p, "\\\\.\\pipe") { + if strings.HasPrefix(p, `\\.\pipe`) { return p } if strings.HasPrefix(p, "~/") || strings.HasPrefix(p, "~\\") { diff --git a/internal/flags/flags_test.go b/internal/flags/flags_test.go index 1ca512f84aa..681586b46c7 100644 --- a/internal/flags/flags_test.go +++ b/internal/flags/flags_test.go @@ -29,33 +29,33 @@ func TestPathExpansion(t *testing.T) { if runtime.GOOS == "windows" { tests = map[string]string{ - "/home/someuser/tmp": "\\home\\someuser\\tmp", - "~/tmp": user.HomeDir + "\\tmp", - "~thisOtherUser/b/": "~thisOtherUser\\b", - "$DDDXXX/a/b": "\\tmp\\a\\b", - "/a/b/": "\\a\\b", - "C:\\Documents\\Newsletters\\": "C:\\Documents\\Newsletters\\", - "C:\\": "C:\\", - "\\\\.\\pipe\\\\pipe\\geth621383": "\\\\.\\pipe\\\\pipe\\geth621383", + `/home/someuser/tmp`: `\home\someuser\tmp`, + `~/tmp`: user.HomeDir + `\tmp`, + `~thisOtherUser/b/`: `~thisOtherUser\b`, + `$DDDXXX/a/b`: `\tmp\a\b`, + `/a/b/`: `\a\b`, + `C:\Documents\Newsletters\`: `C:\Documents\Newsletters`, + `C:\`: `C:\`, + `\\.\pipe\\pipe\geth621383`: `\\.\pipe\\pipe\geth621383`, } } else { tests = map[string]string{ - "/home/someuser/tmp": "/home/someuser/tmp", - "~/tmp": user.HomeDir + "/tmp", - "~thisOtherUser/b/": "~thisOtherUser/b", - "$DDDXXX/a/b": "/tmp/a/b", - "/a/b/": "/a/b", - "C:\\Documents\\Newsletters\\": "C:\\Documents\\Newsletters\\", - "C:\\": "C:\\", - "\\\\.\\pipe\\\\pipe\\geth621383": "\\\\.\\pipe\\\\pipe\\geth621383", + `/home/someuser/tmp`: `/home/someuser/tmp`, + `~/tmp`: user.HomeDir + `/tmp`, + `~thisOtherUser/b/`: `~thisOtherUser/b`, + `$DDDXXX/a/b`: `/tmp/a/b`, + `/a/b/`: `/a/b`, + `C:\Documents\Newsletters\`: `C:\Documents\Newsletters\`, + `C:\`: `C:\`, + `\\.\pipe\\pipe\geth621383`: `\\.\pipe\\pipe\geth621383`, } } - os.Setenv("DDDXXX", "/tmp") + os.Setenv(`DDDXXX`, `/tmp`) for test, expected := range tests { got := expandPath(test) if got != expected { - t.Errorf("test %s, got %s, expected %s\n", test, got, expected) + t.Errorf(`test %s, got %s, expected %s\n`, test, got, expected) } } }