increase port identifier limit to 128 characters (#344)

* increase port identifier limit to 128 characters increase port limit and add tests for port validation * add changelog * fix tests
cosmos · Aug 26, 2021 · 358bfa5 · 358bfa5
1 parent 950b92d
commit 358bfa5
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -36,8 +36,14 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## [Unreleased]
 
+### API Breaking
+
 * (core) [\#227](https://github.com/cosmos/ibc-go/pull/227) Remove sdk.Result from application callbacks
 
+### State Machine Breaking
+
+* (24-host) [#\344](https://github.com/cosmos/ibc-go/pull/344) Increase port identifier limit to 128 characters. 
+
 
 ## [v1.0.0](https://github.com/cosmos/ibc-go/releases/tag/v1.0.0) - 2021-08-10
 

diff --git a/modules/apps/transfer/types/msgs_test.go b/modules/apps/transfer/types/msgs_test.go
@@ -16,7 +16,8 @@ const (
 	validPort        = "testportid"
 	invalidPort      = "(invalidport1)"
 	invalidShortPort = "p"
-	invalidLongPort  = "invalidlongportinvalidlongportinvalidlongportinvalidlongportinvalid"
+	// 195 characters
+	invalidLongPort = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Duis eros neque, ultricies vel ligula ac, convallis porttitor elit. Maecenas tincidunt turpis elit, vel faucibus nisl pellentesque sodales"
 
 	validChannel        = "testchannel"
 	invalidChannel      = "(invalidchannel1)"

diff --git a/modules/core/04-channel/types/msgs_test.go b/modules/core/04-channel/types/msgs_test.go
@@ -32,7 +32,8 @@ const (
 	// invalid constants used for testing
 	invalidPort      = "(invalidport1)"
 	invalidShortPort = "p"
-	invalidLongPort  = "invalidlongportinvalidlongportinvalidlongportidinvalidlongportidinvalid"
+	// 195 characters
+	invalidLongPort = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Duis eros neque, ultricies vel ligula ac, convallis porttitor elit. Maecenas tincidunt turpis elit, vel faucibus nisl pellentesque sodales"
 
 	invalidChannel      = "(invalidchannel1)"
 	invalidShortChannel = "invalid"

diff --git a/modules/core/24-host/validate.go b/modules/core/24-host/validate.go
@@ -16,6 +16,10 @@ import (
 // adjusting this restriction.
 const DefaultMaxCharacterLength = 64
 
+// DefaultMaxPortCharacterLength defines the default maximum character length used
+// in validation of port identifiers.
+var DefaultMaxPortCharacterLength = 128
+
 // IsValidID defines regular expression to check if the string consist of
 // characters in one of the following categories only:
 // - Alphanumeric
@@ -80,7 +84,7 @@ func ChannelIdentifierValidator(id string) error {
 // A valid Identifier must be between 2-64 characters and only contain alphanumeric and some allowed
 // special characters (see IsValidID).
 func PortIdentifierValidator(id string) error {
-	return defaultIdentifierValidator(id, 2, DefaultMaxCharacterLength)
+	return defaultIdentifierValidator(id, 2, DefaultMaxPortCharacterLength)
 }
 
 // NewPathValidator takes in a Identifier Validator function and returns

diff --git a/modules/core/24-host/validate_test.go b/modules/core/24-host/validate_test.go
@@ -8,6 +8,9 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// 195 characters
+var longId = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Duis eros neque, ultricies vel ligula ac, convallis porttitor elit. Maecenas tincidunt turpis elit, vel faucibus nisl pellentesque sodales"
+
 type testCase struct {
 	msg     string
 	id      string
@@ -50,6 +53,33 @@ func TestDefaultIdentifierValidator(t *testing.T) {
 	}
 }
 
+func TestPortIdentifierValidator(t *testing.T) {
+	testCases := []testCase{
+		{"valid lowercase", "transfer", true},
+		{"valid id special chars", "._+-#[]<>._+-#[]<>", true},
+		{"valid id lower and special chars", "lower._+-#[]<>", true},
+		{"numeric id", "1234567890", true},
+		{"uppercase id", "NOTLOWERCASE", true},
+		{"numeric id", "1234567890", true},
+		{"blank id", "               ", false},
+		{"id length out of range", "1", false},
+		{"id is too long", longId, false},
+		{"path-like id", "lower/case/id", false},
+		{"invalid id", "(clientid)", false},
+		{"empty string", "", false},
+	}
+
+	for _, tc := range testCases {
+
+		err := PortIdentifierValidator(tc.id)
+		if tc.expPass {
+			require.NoError(t, err, tc.msg)
+		} else {
+			require.Error(t, err, tc.msg)
+		}
+	}
+}
+
 func TestPathValidator(t *testing.T) {
 	testCases := []testCase{
 		{"valid lowercase", "p/lowercaseid", true},