libs/strings: cleanup string helper function package (#7808)

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

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) {

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"

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"
 )

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

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)
+			}
+		})
+
 	}
 
 }