diff --git a/libs/strings/string.go b/libs/strings/string.go index 6cc0b18ee..95ea03b5a 100644 --- a/libs/strings/string.go +++ b/libs/strings/string.go @@ -28,54 +28,12 @@ func SplitAndTrimEmpty(s, sep, cutset string) []string { return nonEmptyStrings } -// StringInSlice returns true if a is found the list. -func StringInSlice(a string, list []string) bool { - for _, b := range list { - if b == a { - return true - } - } - return false -} - -// SplitAndTrim slices s into all subslices separated by sep and returns a -// slice of the string s with all leading and trailing Unicode code points -// contained in cutset removed. If sep is empty, SplitAndTrim splits after each -// UTF-8 sequence. First part is equivalent to strings.SplitN with a count of -// -1. -func SplitAndTrim(s, sep, cutset string) []string { - if s == "" { - return []string{} - } - - spl := strings.Split(s, sep) - for i := 0; i < len(spl); i++ { - spl[i] = strings.Trim(spl[i], cutset) - } - return spl -} - -// TrimSpace removes all leading and trailing whitespace from the -// string. -func TrimSpace(s string) string { return strings.TrimSpace(s) } - -// Returns true if s is a non-empty printable non-tab ascii character. -func IsASCIIText(s string) bool { +// ASCIITrim removes spaces from an a ASCII string, erroring if the +// sequence is not an ASCII string. +func ASCIITrim(s string) (string, error) { if len(s) == 0 { - return false - } - for _, b := range []byte(s) { - if 32 <= b && b <= 126 { - // good - } else { - return false - } + return "", nil } - return true -} - -// NOTE: Assumes that s is ASCII as per IsASCIIText(), otherwise panics. -func ASCIITrim(s string) string { r := make([]byte, 0, len(s)) for _, b := range []byte(s) { switch { @@ -84,10 +42,10 @@ func ASCIITrim(s string) string { case 32 < b && b <= 126: r = append(r, b) default: - panic(fmt.Sprintf("non-ASCII (non-tab) char 0x%X", b)) + return "", fmt.Errorf("non-ASCII (non-tab) char 0x%X", b) } } - return string(r) + return string(r), nil } // StringSliceEqual checks if string slices a and b are equal diff --git a/libs/strings/string_test.go b/libs/strings/string_test.go index c56116393..79caf5901 100644 --- a/libs/strings/string_test.go +++ b/libs/strings/string_test.go @@ -25,34 +25,48 @@ func TestSplitAndTrimEmpty(t *testing.T) { } } -func TestStringInSlice(t *testing.T) { - require.True(t, StringInSlice("a", []string{"a", "b", "c"})) - require.False(t, StringInSlice("d", []string{"a", "b", "c"})) - require.True(t, StringInSlice("", []string{""})) - require.False(t, StringInSlice("", []string{})) -} - -func TestIsASCIIText(t *testing.T) { - notASCIIText := []string{ - "", "\xC2", "\xC2\xA2", "\xFF", "\x80", "\xF0", "\n", "\t", - } - for _, v := range notASCIIText { - require.False(t, IsASCIIText(v), "%q is not ascii-text", v) - } - asciiText := []string{ - " ", ".", "x", "$", "_", "abcdefg;", "-", "0x00", "0", "123", - } - for _, v := range asciiText { - require.True(t, IsASCIIText(v), "%q is ascii-text", v) - } +func assertCorrectTrim(t *testing.T, input, expected string) { + t.Helper() + output, err := ASCIITrim(input) + require.NoError(t, err) + require.Equal(t, expected, output) } func TestASCIITrim(t *testing.T) { - require.Equal(t, ASCIITrim(" "), "") - require.Equal(t, ASCIITrim(" a"), "a") - require.Equal(t, ASCIITrim("a "), "a") - require.Equal(t, ASCIITrim(" a "), "a") - require.Panics(t, func() { ASCIITrim("\xC2\xA2") }) + t.Run("Validation", func(t *testing.T) { + t.Run("NonASCII", func(t *testing.T) { + notASCIIText := []string{ + "\xC2", "\xC2\xA2", "\xFF", "\x80", "\xF0", "\n", "\t", + } + for _, v := range notASCIIText { + _, err := ASCIITrim(v) + require.Error(t, err, "%q is not ascii-text", v) + } + }) + t.Run("EmptyString", func(t *testing.T) { + out, err := ASCIITrim("") + require.NoError(t, err) + require.Zero(t, out) + }) + t.Run("ASCIIText", func(t *testing.T) { + asciiText := []string{ + " ", ".", "x", "$", "_", "abcdefg;", "-", "0x00", "0", "123", + } + for _, v := range asciiText { + _, err := ASCIITrim(v) + require.NoError(t, err, "%q is ascii-text", v) + } + }) + _, err := ASCIITrim("\xC2\xA2") + require.Error(t, err) + }) + t.Run("Trimming", func(t *testing.T) { + assertCorrectTrim(t, " ", "") + assertCorrectTrim(t, " a", "a") + assertCorrectTrim(t, "a ", "a") + assertCorrectTrim(t, " a ", "a") + }) + } func TestStringSliceEqual(t *testing.T) { diff --git a/node/node.go b/node/node.go index 4a900368d..84929a58c 100644 --- a/node/node.go +++ b/node/node.go @@ -7,6 +7,7 @@ import ( "net" "net/http" "strconv" + "strings" "time" "github.com/prometheus/client_golang/prometheus" @@ -29,7 +30,6 @@ import ( "github.com/tendermint/tendermint/internal/store" "github.com/tendermint/tendermint/libs/log" "github.com/tendermint/tendermint/libs/service" - "github.com/tendermint/tendermint/libs/strings" tmtime "github.com/tendermint/tendermint/libs/time" "github.com/tendermint/tendermint/privval" "github.com/tendermint/tendermint/types" diff --git a/node/seed.go b/node/seed.go index ef3e61df0..970896cc6 100644 --- a/node/seed.go +++ b/node/seed.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "net/http" + "strings" "time" "github.com/tendermint/tendermint/config" @@ -13,7 +14,6 @@ import ( sm "github.com/tendermint/tendermint/internal/state" "github.com/tendermint/tendermint/libs/log" "github.com/tendermint/tendermint/libs/service" - "github.com/tendermint/tendermint/libs/strings" tmtime "github.com/tendermint/tendermint/libs/time" "github.com/tendermint/tendermint/types" ) diff --git a/types/node_info.go b/types/node_info.go index 57aced054..fd47816e2 100644 --- a/types/node_info.go +++ b/types/node_info.go @@ -82,10 +82,10 @@ func (info NodeInfo) Validate() error { } // Validate Version - if len(info.Version) > 0 && - (!tmstrings.IsASCIIText(info.Version) || tmstrings.ASCIITrim(info.Version) == "") { - - return fmt.Errorf("info.Version must be valid ASCII text without tabs, but got %v", info.Version) + if len(info.Version) > 0 { + if ver, err := tmstrings.ASCIITrim(info.Version); err != nil || ver == "" { + return fmt.Errorf("info.Version must be valid ASCII text without tabs, but got, %q [%s]", info.Version, ver) + } } // Validate Channels - ensure max and check for duplicates. @@ -101,8 +101,7 @@ func (info NodeInfo) Validate() error { channels[ch] = struct{}{} } - // Validate Moniker. - if !tmstrings.IsASCIIText(info.Moniker) || tmstrings.ASCIITrim(info.Moniker) == "" { + if m, err := tmstrings.ASCIITrim(info.Moniker); err != nil || m == "" { return fmt.Errorf("info.Moniker must be valid non-empty ASCII text without tabs, but got %v", info.Moniker) } @@ -116,8 +115,10 @@ func (info NodeInfo) Validate() error { } // XXX: Should we be more strict about address formats? rpcAddr := other.RPCAddress - if len(rpcAddr) > 0 && (!tmstrings.IsASCIIText(rpcAddr) || tmstrings.ASCIITrim(rpcAddr) == "") { - return fmt.Errorf("info.Other.RPCAddress=%v must be valid ASCII text without tabs", rpcAddr) + if len(rpcAddr) > 0 { + if a, err := tmstrings.ASCIITrim(rpcAddr); err != nil || a == "" { + return fmt.Errorf("info.Other.RPCAddress=%v must be valid ASCII text without tabs", rpcAddr) + } } return nil diff --git a/types/node_info_test.go b/types/node_info_test.go index c14570c96..d8aebdeb0 100644 --- a/types/node_info_test.go +++ b/types/node_info_test.go @@ -80,15 +80,18 @@ func TestNodeInfoValidate(t *testing.T) { assert.NoError(t, ni.Validate()) for _, tc := range testCases { - ni := testNodeInfo(t, nodeKeyID, name) - ni.Channels = channels - tc.malleateNodeInfo(&ni) - err := ni.Validate() - if tc.expectErr { - assert.Error(t, err, tc.testName) - } else { - assert.NoError(t, err, tc.testName) - } + t.Run(tc.testName, func(t *testing.T) { + ni := testNodeInfo(t, nodeKeyID, name) + ni.Channels = channels + tc.malleateNodeInfo(&ni) + err := ni.Validate() + if tc.expectErr { + assert.Error(t, err, tc.testName) + } else { + assert.NoError(t, err, tc.testName) + } + }) + } }