From e9627abc546cdf46b2437bf6d376434604f81463 Mon Sep 17 00:00:00 2001 From: colin axner <25233464+colin-axner@users.noreply.github.com> Date: Wed, 22 Jul 2020 12:20:41 +0200 Subject: [PATCH] bump identifier maximum to 64 characters (#6812) * bump identifier maximum to 64 characters * fix tests Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- x/ibc-transfer/types/msgs_test.go | 4 ++-- x/ibc/04-channel/types/msgs_test.go | 6 +++--- x/ibc/24-host/validate.go | 23 ++++++++++++++++------- x/ibc/24-host/validate_test.go | 5 +++-- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/x/ibc-transfer/types/msgs_test.go b/x/ibc-transfer/types/msgs_test.go index 027b3f944a..ed20aac257 100644 --- a/x/ibc-transfer/types/msgs_test.go +++ b/x/ibc-transfer/types/msgs_test.go @@ -14,12 +14,12 @@ const ( validPort = "testportid" invalidPort = "(invalidport1)" invalidShortPort = "p" - invalidLongPort = "invalidlongportinvalidlongport" + invalidLongPort = "invalidlongportinvalidlongportinvalidlongportinvalidlongportinvalid" validChannel = "testchannel" invalidChannel = "(invalidchannel1)" invalidShortChannel = "invalidch" - invalidLongChannel = "invalidlongchannelinvalidlongchannel" + invalidLongChannel = "invalidlongchannelinvalidlongchannelinvalidlongchannelinvalidlongchannel" ) var ( diff --git a/x/ibc/04-channel/types/msgs_test.go b/x/ibc/04-channel/types/msgs_test.go index 210fa163b5..a615a11b91 100644 --- a/x/ibc/04-channel/types/msgs_test.go +++ b/x/ibc/04-channel/types/msgs_test.go @@ -23,15 +23,15 @@ import ( const ( invalidPort = "(invalidport1)" invalidShortPort = "p" - invalidLongPort = "invalidlongportinvalidlongport" + invalidLongPort = "invalidlongportinvalidlongportinvalidlongportidinvalidlongportidinvalid" invalidChannel = "(invalidchannel1)" invalidShortChannel = "invalidch" - invalidLongChannel = "invalidlongchannelinvalidlongchannel" + invalidLongChannel = "invalidlongchannelinvalidlongchannelinvalidlongchannelinvalidlongchannel" invalidConnection = "(invalidconnection1)" invalidShortConnection = "invalidcn" - invalidLongConnection = "invalidlongconnection" + invalidLongConnection = "invalidlongconnectioninvalidlongconnectioninvalidlongconnectioninvalid" ) // define variables used for testing diff --git a/x/ibc/24-host/validate.go b/x/ibc/24-host/validate.go index 0cf552eb7d..0a8db56067 100644 --- a/x/ibc/24-host/validate.go +++ b/x/ibc/24-host/validate.go @@ -7,6 +7,15 @@ import ( sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) +// DefaultMaxCharacterLength defines the default maximum character length used +// in validation of identifiers including the client, connection, port and +// channel identifiers. +// +// NOTE: this restriction is specific to this golang implementation of IBC. If +// your use case demands a higher limit, please open an issue and we will consider +// adjusting this restriction. +const DefaultMaxCharacterLength = 64 + // IsValidID defines regular expression to check if the string consist of // characters in one of the following categories only: // - Alphanumeric @@ -31,7 +40,7 @@ func defaultIdentifierValidator(id string, min, max int) error { //nolint:unpara if strings.Contains(id, "/") { return sdkerrors.Wrapf(ErrInvalidID, "identifier %s cannot contain separator '/'", id) } - // valid id must be between 9 and 20 characters + // valid id must fit the length requirements if len(id) < min || len(id) > max { return sdkerrors.Wrapf(ErrInvalidID, "identifier %s has invalid length: %d, must be between %d-%d characters", id, len(id), min, max) } @@ -50,28 +59,28 @@ func defaultIdentifierValidator(id string, min, max int) error { //nolint:unpara // A valid Identifier must be between 9-20 characters and only contain lowercase // alphabetic characters, func ClientIdentifierValidator(id string) error { - return defaultIdentifierValidator(id, 9, 20) + return defaultIdentifierValidator(id, 9, DefaultMaxCharacterLength) } // ConnectionIdentifierValidator is the default validator function for Connection identifiers. // A valid Identifier must be between 10-20 characters and only contain lowercase // alphabetic characters, func ConnectionIdentifierValidator(id string) error { - return defaultIdentifierValidator(id, 10, 20) + return defaultIdentifierValidator(id, 10, DefaultMaxCharacterLength) } // ChannelIdentifierValidator is the default validator function for Channel identifiers. // A valid Identifier must be between 10-20 characters and only contain lowercase // alphabetic characters, func ChannelIdentifierValidator(id string) error { - return defaultIdentifierValidator(id, 10, 20) + return defaultIdentifierValidator(id, 10, DefaultMaxCharacterLength) } // PortIdentifierValidator is the default validator function for Port identifiers. // A valid Identifier must be between 2-20 characters and only contain lowercase // alphabetic characters, func PortIdentifierValidator(id string) error { - return defaultIdentifierValidator(id, 2, 20) + return defaultIdentifierValidator(id, 2, DefaultMaxCharacterLength) } // NewPathValidator takes in a Identifier Validator function and returns @@ -94,7 +103,7 @@ func NewPathValidator(idValidator ValidateFn) ValidateFn { return err } // Each path element must either be a valid identifier or constant number - if err := defaultIdentifierValidator(p, 1, 20); err != nil { + if err := defaultIdentifierValidator(p, 1, DefaultMaxCharacterLength); err != nil { return sdkerrors.Wrapf(err, "path %s contains an invalid identifier: '%s'", path, p) } } @@ -118,7 +127,7 @@ func PathValidator(path string) error { } // Each path element must be a valid identifier or constant number - if err := defaultIdentifierValidator(p, 1, 20); err != nil { + if err := defaultIdentifierValidator(p, 1, DefaultMaxCharacterLength); err != nil { return sdkerrors.Wrapf(err, "path %s contains an invalid identifier: '%s'", path, p) } } diff --git a/x/ibc/24-host/validate_test.go b/x/ibc/24-host/validate_test.go index 417447fe03..41dd5167d9 100644 --- a/x/ibc/24-host/validate_test.go +++ b/x/ibc/24-host/validate_test.go @@ -24,6 +24,7 @@ func TestDefaultIdentifierValidator(t *testing.T) { {"numeric id", "1234567890", true}, {"blank id", " ", false}, {"id length out of range", "1", false}, + {"id is too long", "this identifier is too long to be used as a valid identifier", false}, {"path-like id", "lower/case/id", false}, {"invalid id", "(clientid)", false}, {"empty string", "", false}, @@ -59,7 +60,7 @@ func TestPathValidator(t *testing.T) { {"uppercase id", "p/NOTLOWERCASE", true}, {"invalid path", "lowercaseid", false}, {"blank id", "p/ ", false}, - {"id length out of range", "p/123456789012345678901", false}, + {"id length out of range", "p/12345678901234567890123456789012345678901234567890123456789012345", false}, {"invalid id", "p/(clientid)", false}, {"empty string", "", false}, {"separators only", "////", false}, @@ -95,7 +96,7 @@ func TestCustomPathValidator(t *testing.T) { {"valid custom path", "id_client/id_one", true}, {"invalid path", "client", false}, {"invalid custom path", "id_one/client", false}, - {"invalid identifier", "id_client/id_123456789012345678901", false}, + {"invalid identifier", "id_client/id_1234567890123456789012345678901234567890123457890123456789012345", false}, {"separators only", "////", false}, {"just separator", "/", false}, {"ends with separator", "id_client/id_one/", false},