From 5354d4f0025aba4649e32fa63d4894b430d19d1a Mon Sep 17 00:00:00 2001 From: Andrew Mason Date: Tue, 28 Mar 2023 13:46:08 -0400 Subject: [PATCH] broader restrictions via allow-list Signed-off-by: Andrew Mason --- changelog/17.0/17.0.0/summary.md | 3 +- go/vt/topo/keyspace.go | 8 +++--- go/vt/topo/keyspace_test.go | 48 ++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/changelog/17.0/17.0.0/summary.md b/changelog/17.0/17.0.0/summary.md index 1c1ec2e1daa..4e9a0e25275 100644 --- a/changelog/17.0/17.0.0/summary.md +++ b/changelog/17.0/17.0.0/summary.md @@ -57,8 +57,7 @@ Prior to v17, it was possible to create a keyspace with invalid characters, whic Now, the TopoServer's `GetKeyspace` and `CreateKeyspace` methods return an error if given an invalid name. -Keyspace names may no longer contain the following characters: -- Forward slash ("/"). +Keyspace names may now only contain alphanumerics, dashes (`-`), and underscores (`_`), and must begin with an alphabetical character. ### New command line flags and behavior diff --git a/go/vt/topo/keyspace.go b/go/vt/topo/keyspace.go index dc0af6e11ec..8b62972ad48 100755 --- a/go/vt/topo/keyspace.go +++ b/go/vt/topo/keyspace.go @@ -18,7 +18,7 @@ package topo import ( "path" - "strings" + "regexp" "context" @@ -54,15 +54,15 @@ func (ki *KeyspaceInfo) SetKeyspaceName(name string) { ki.keyspace = name } -var invalidKeyspaceNameChars = "/" +var keyspaceNameRegexp = regexp.MustCompile("^[a-zA-Z][a-zA-Z0-9_-]*$") // ValidateKeyspaceNames checks if the provided name is a valid name for a // keyspace. // // As of v17, "all invalid characters" is just the forward slash ("/"). func ValidateKeyspaceName(name string) error { - if strings.ContainsAny(name, invalidKeyspaceNameChars) { - return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "keyspace name %s contains invalid characters; may not contain any of the following: %+v", name, strings.Split(invalidKeyspaceNameChars, "")) + if !keyspaceNameRegexp.MatchString(name) { + return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "keyspace name %s contains invalid characters; may only contain alphanumerics, dashes and underscores", name) } return nil diff --git a/go/vt/topo/keyspace_test.go b/go/vt/topo/keyspace_test.go index 1abf873b8e0..4a96649d593 100644 --- a/go/vt/topo/keyspace_test.go +++ b/go/vt/topo/keyspace_test.go @@ -20,6 +20,8 @@ import ( "reflect" "testing" + "github.com/stretchr/testify/assert" + topodatapb "vitess.io/vitess/go/vt/proto/topodata" ) @@ -175,3 +177,49 @@ func TestComputeCellServedFrom(t *testing.T) { t.Fatalf("c2 failed: %v", m) } } + +func TestValidateKeyspaceName(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + valid bool + }{ + { + name: "testks", + valid: true, + }, + { + name: "alphnum_ks1-with_a_dash", + valid: true, + }, + { + name: "no/slashes/allowed", + valid: false, + }, + { + name: "wë!rd ch@r@ct3r$", + valid: false, + }, + { + name: "0_cant-start_with-a-number", + valid: false, + }, + } + + for _, tcase := range cases { + tcase := tcase + t.Run(tcase.name, func(t *testing.T) { + t.Parallel() + + var assertion func(t assert.TestingT, err error, msgAndArgs ...any) bool + if tcase.valid { + assertion = assert.NoError + } else { + assertion = assert.Error + } + + assertion(t, ValidateKeyspaceName(tcase.name)) + }) + } +}