Skip to content

Commit

Permalink
bug: fix clientID validation regex to conform closer to spec (#2510)
Browse files Browse the repository at this point in the history
Rejects non-ASCII plus whitespaces and slashes to get it much closer to
the specifications per
https://github.com/cosmos/ibc/tree/main/spec/core/ics-024-host-requirements#paths-identifiers-separators

while here added test vectors that were previously failing. However, we
still need much better specifications for how long of values for
{client-type} are accepted.

Fixes #2269
  • Loading branch information
odeke-em committed Nov 30, 2022
1 parent ba1e50d commit 53dbc41
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 14 deletions.
4 changes: 3 additions & 1 deletion modules/core/02-client/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ func FormatClientIdentifier(clientType string, sequence uint64) string {

// IsClientIDFormat checks if a clientID is in the format required on the SDK for
// parsing client identifiers. The client identifier must be in the form: `{client-type}-{N}
var IsClientIDFormat = regexp.MustCompile(`^.*[^\n-]-[0-9]{1,20}$`).MatchString
// which per the specification only permits ASCII for the {client-type} segment and
// 1 to 20 digits for the {N} segment.
var IsClientIDFormat = regexp.MustCompile(`^\w+([\w-]+\w)?-[0-9]{1,20}$`).MatchString

// IsValidClientID checks if the clientID is valid and can be parsed into the client
// identifier format.
Expand Down
35 changes: 22 additions & 13 deletions modules/core/02-client/types/keys_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,22 +35,31 @@ func TestParseClientIdentifier(t *testing.T) {
{"negative sequence", "tendermint--1", "tendermint", 0, false},
{"invalid format", "tendermint-tm", "tendermint", 0, false},
{"empty clientype", " -100", "tendermint", 0, false},
{"with in the middle tabs", "a\t\t\t-100", "tendermint", 0, false},
{"leading tabs", "\t\t\ta-100", "tendermint", 0, false},
{"with whitespace", " a-100", "tendermint", 0, false},
{"leading hyphens", "-----a-100", "tendermint", 0, false},
{"with slash", "tendermint/-100", "tendermint", 0, false},
{"non-ASCII:: emoji", "🚨😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎😎-100", "tendermint", 0, false},
{"non-ASCII:: others", "世界-100", "tendermint", 0, false},
}

for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
clientType, seq, err := types.ParseClientIdentifier(tc.clientID)
valid := types.IsValidClientID(tc.clientID)
require.Equal(t, tc.expSeq, seq, tc.clientID)

clientType, seq, err := types.ParseClientIdentifier(tc.clientID)
valid := types.IsValidClientID(tc.clientID)
require.Equal(t, tc.expSeq, seq, tc.clientID)

if tc.expPass {
require.NoError(t, err, tc.name)
require.True(t, valid)
require.Equal(t, tc.clientType, clientType)
} else {
require.Error(t, err, tc.name, tc.clientID)
require.False(t, valid)
require.Equal(t, "", clientType)
}
if tc.expPass {
require.NoError(t, err, tc.name)
require.True(t, valid)
require.Equal(t, tc.clientType, clientType)
} else {
require.Error(t, err, tc.name, tc.clientID)
require.False(t, valid)
require.Equal(t, "", clientType)
}
})
}
}

0 comments on commit 53dbc41

Please sign in to comment.