Merge "[FAB-16377] Perform validation of channel ID"

common/configtx/util_test.go

@@ -27,7 +27,7 @@ func TestValidConfigID(t *testing.T) {
 	})
 
 	t.Run("LongerThanMaxAllowed", func(t *testing.T) {
-		if err := validateConfigID(randomAlphaString(maxLength + 1)); err == nil {
+		if err := validateConfigID(randomAlphaString(MaxLength + 1)); err == nil {
 			t.Fatal(rejectMsg)
 		}
 	})
@@ -59,37 +59,37 @@ func TestValidChannelID(t *testing.T) {
 	rejectMsg := "Should have rejected invalid channel ID"
 
 	t.Run("ZeroLength", func(t *testing.T) {
-		if err := validateChannelID(""); err == nil {
+		if err := ValidateChannelID(""); err == nil {
 			t.Fatal(rejectMsg)
 		}
 	})
 
 	t.Run("LongerThanMaxAllowed", func(t *testing.T) {
-		if err := validateChannelID(randomLowerAlphaString(maxLength + 1)); err == nil {
+		if err := ValidateChannelID(randomLowerAlphaString(MaxLength + 1)); err == nil {
 			t.Fatal(rejectMsg)
 		}
 	})
 
 	t.Run("ContainsIllegalCharacter", func(t *testing.T) {
-		if err := validateChannelID("foo_bar"); err == nil {
+		if err := ValidateChannelID("foo_bar"); err == nil {
 			t.Fatal(rejectMsg)
 		}
 	})
 
 	t.Run("StartsWithNumber", func(t *testing.T) {
-		if err := validateChannelID("8foo"); err == nil {
+		if err := ValidateChannelID("8foo"); err == nil {
 			t.Fatal(rejectMsg)
 		}
 	})
 
 	t.Run("StartsWithDot", func(t *testing.T) {
-		if err := validateChannelID(".foo"); err == nil {
+		if err := ValidateChannelID(".foo"); err == nil {
 			t.Fatal(rejectMsg)
 		}
 	})
 
 	t.Run("ValidName", func(t *testing.T) {
-		if err := validateChannelID("f-oo.bar"); err != nil {
+		if err := ValidateChannelID("f-oo.bar"); err != nil {
 			t.Fatal(acceptMsg)
 		}
 	})

common/configtx/validator.go

@@ -21,9 +21,9 @@ var logger = flogging.MustGetLogger("common.configtx")
 
 // Constraints for valid channel and config IDs
 var (
-	channelAllowedChars = "[a-z][a-z0-9.-]*"
+	ChannelAllowedChars = "[a-z][a-z0-9.-]*"
 	configAllowedChars  = "[a-zA-Z0-9.-]+"
-	maxLength           = 249
+	MaxLength           = 249
 	illegalNames        = map[string]struct{}{
 		".":  {},
 		"..": {},
@@ -51,8 +51,8 @@ func validateConfigID(configID string) error {
 	if len(configID) <= 0 {
 		return errors.New("config ID illegal, cannot be empty")
 	}
-	if len(configID) > maxLength {
-		return errors.Errorf("config ID illegal, cannot be longer than %d", maxLength)
+	if len(configID) > MaxLength {
+		return errors.Errorf("config ID illegal, cannot be longer than %d", MaxLength)
 	}
 	// Illegal name
 	if _, ok := illegalNames[configID]; ok {
@@ -67,7 +67,7 @@ func validateConfigID(configID string) error {
 	return nil
 }
 
-// validateChannelID makes sure that proposed channel IDs comply with the
+// ValidateChannelID makes sure that proposed channel IDs comply with the
 // following restrictions:
 //      1. Contain only lower case ASCII alphanumerics, dots '.', and dashes '-'
 //      2. Are shorter than 250 characters.
@@ -77,14 +77,17 @@ func validateConfigID(configID string) error {
 // with the following exception: '.' is converted to '_' in the CouchDB naming
 // This is to accommodate existing channel names with '.', especially in the
 // behave tests which rely on the dot notation for their sluggification.
-func validateChannelID(channelID string) error {
-	re, _ := regexp.Compile(channelAllowedChars)
+//
+// note: this function is a copy of the same in core/tx/endorser/parser.go
+//
+func ValidateChannelID(channelID string) error {
+	re, _ := regexp.Compile(ChannelAllowedChars)
 	// Length
 	if len(channelID) <= 0 {
 		return errors.Errorf("channel ID illegal, cannot be empty")
 	}
-	if len(channelID) > maxLength {
-		return errors.Errorf("channel ID illegal, cannot be longer than %d", maxLength)
+	if len(channelID) > MaxLength {
+		return errors.Errorf("channel ID illegal, cannot be longer than %d", MaxLength)
 	}
 
 	// Illegal characters
@@ -106,7 +109,7 @@ func NewValidatorImpl(channelID string, config *cb.Config, namespace string, pm
 		return nil, errors.Errorf("nil channel group")
 	}
 
-	if err := validateChannelID(channelID); err != nil {
+	if err := ValidateChannelID(channelID); err != nil {
 		return nil, errors.Errorf("bad channel ID: %s", err)
 	}
 

core/tx/endorser/endorsertx_suite_test.go

@@ -7,6 +7,7 @@ SPDX-License-Identifier: Apache-2.0
 package endorsertx_test
 
 import (
+	"math/rand"
 	"testing"
 
 	. "github.com/onsi/ginkgo"
@@ -17,6 +18,15 @@ type protomsg struct {
 	msg []byte
 }
 
+func randomLowerAlphaString(size int) string {
+	letters := []rune("abcdefghijklmnopqrstuvwxyz")
+	output := make([]rune, size)
+	for i := range output {
+		output[i] = letters[rand.Intn(len(letters))]
+	}
+	return string(output)
+}
+
 func TestEndorserTx(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "EndorserTx Suite")

core/tx/endorser/parser.go

@@ -7,13 +7,23 @@ SPDX-License-Identifier: Apache-2.0
 package endorsertx
 
 import (
+	"regexp"
+
 	"github.com/hyperledger/fabric-protos-go/common"
 	"github.com/hyperledger/fabric-protos-go/peer"
+
 	"github.com/hyperledger/fabric/pkg/tx"
 	"github.com/hyperledger/fabric/protoutil"
 	"github.com/pkg/errors"
 )
 
+var (
+	// ChannelAllowedChars tracks the permitted characters for a channel name
+	ChannelAllowedChars = "[a-z][a-z0-9.-]*"
+	// MaxLength tracks the maximum length of a channel name
+	MaxLength = 249
+)
+
 // EndorserTx represents a parsed common.Envelope protobuf
 type EndorserTx struct {
 	ComputedTxID string
@@ -43,6 +53,10 @@ func validateHeaders(
 		return errors.Errorf("invalid version in ChannelHeader. Expected 0, got [%d]", cHdr.Version)
 	}
 
+	if err := ValidateChannelID(cHdr.ChannelId); err != nil {
+		return err
+	}
+
 	/********************************************/
 	/*****VALIDATION OF THE SIGNATURE HEADER*****/
 	/********************************************/
@@ -167,3 +181,35 @@ func NewEndorserTx(txenv *tx.Envelope) (*EndorserTx, error) {
 		CcID:         hdrExt.ChaincodeId.Name, // FIXME: we might have to get the ccid from the CIS
 	}, nil
 }
+
+// ValidateChannelID makes sure that proposed channel IDs comply with the
+// following restrictions:
+//      1. Contain only lower case ASCII alphanumerics, dots '.', and dashes '-'
+//      2. Are shorter than 250 characters.
+//      3. Start with a letter
+//
+// This is the intersection of the Kafka restrictions and CouchDB restrictions
+// with the following exception: '.' is converted to '_' in the CouchDB naming
+// This is to accommodate existing channel names with '.', especially in the
+// behave tests which rely on the dot notation for their sluggification.
+//
+// note: this function is a copy of the same in common/configtx/validator.go
+//
+func ValidateChannelID(channelID string) error {
+	re, _ := regexp.Compile(ChannelAllowedChars)
+	// Length
+	if len(channelID) <= 0 {
+		return errors.Errorf("channel ID illegal, cannot be empty")
+	}
+	if len(channelID) > MaxLength {
+		return errors.Errorf("channel ID illegal, cannot be longer than %d", MaxLength)
+	}
+
+	// Illegal characters
+	matched := re.FindString(channelID)
+	if len(matched) != len(channelID) {
+		return errors.Errorf("'%s' contains illegal characters", channelID)
+	}
+
+	return nil
+}

core/tx/endorser/parser_test.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/hyperledger/fabric-protos-go/common"
 	"github.com/hyperledger/fabric-protos-go/peer"
+	"github.com/hyperledger/fabric/common/configtx"
 	endorsertx "github.com/hyperledger/fabric/core/tx/endorser"
 	"github.com/hyperledger/fabric/pkg/tx"
 	txpkg "github.com/hyperledger/fabric/pkg/tx"
@@ -331,6 +332,34 @@ var _ = Describe("Parser", func() {
 			})
 		})
 
+		When("there is an empty channel name", func() {
+			BeforeEach(func() {
+				chHeader = &common.ChannelHeader{
+					ChannelId: "",
+				}
+			})
+
+			It("returns an error", func() {
+				pe, err := endorsertx.NewEndorserTx(txenv)
+				Expect(err).To(MatchError("channel ID illegal, cannot be empty"))
+				Expect(pe).To(BeNil())
+			})
+		})
+
+		When("there is an invalid channel name", func() {
+			BeforeEach(func() {
+				chHeader = &common.ChannelHeader{
+					ChannelId: ".foo",
+				}
+			})
+
+			It("returns an error", func() {
+				pe, err := endorsertx.NewEndorserTx(txenv)
+				Expect(err).To(MatchError("'.foo' contains illegal characters"))
+				Expect(pe).To(BeNil())
+			})
+		})
+
 		When("there is an empty nonce", func() {
 			BeforeEach(func() {
 				sigHeader = &common.SignatureHeader{
@@ -400,4 +429,52 @@ var _ = Describe("Parser", func() {
 			})
 		})
 	})
+
+	Describe("Validation of the channel ID", func() {
+		Context("the used constants", func() {
+			It("ensures that are kept in sync", func() {
+				Expect(endorsertx.ChannelAllowedChars).To(Equal(configtx.ChannelAllowedChars))
+				Expect(endorsertx.MaxLength).To(Equal(configtx.MaxLength))
+			})
+		})
+
+		Context("the validation function", func() {
+			It("behaves as the one in the configtx package", func() {
+				err1 := endorsertx.ValidateChannelID(randomLowerAlphaString(endorsertx.MaxLength + 1))
+				err2 := configtx.ValidateChannelID(randomLowerAlphaString(endorsertx.MaxLength + 1))
+				Expect(err1).To(HaveOccurred())
+				Expect(err2).To(HaveOccurred())
+				Expect(err1.Error()).To(Equal(err2.Error()))
+
+				err1 = endorsertx.ValidateChannelID("foo_bar")
+				err2 = configtx.ValidateChannelID("foo_bar")
+				Expect(err1).To(HaveOccurred())
+				Expect(err2).To(HaveOccurred())
+				Expect(err1.Error()).To(Equal(err2.Error()))
+
+				err1 = endorsertx.ValidateChannelID("8foo")
+				err2 = configtx.ValidateChannelID("8foo")
+				Expect(err1).To(HaveOccurred())
+				Expect(err2).To(HaveOccurred())
+				Expect(err1.Error()).To(Equal(err2.Error()))
+
+				err1 = endorsertx.ValidateChannelID(".foo")
+				err2 = configtx.ValidateChannelID(".foo")
+				Expect(err1).To(HaveOccurred())
+				Expect(err2).To(HaveOccurred())
+				Expect(err1.Error()).To(Equal(err2.Error()))
+
+				err1 = endorsertx.ValidateChannelID("")
+				err2 = configtx.ValidateChannelID("")
+				Expect(err1).To(HaveOccurred())
+				Expect(err2).To(HaveOccurred())
+				Expect(err1.Error()).To(Equal(err2.Error()))
+
+				err1 = endorsertx.ValidateChannelID("f-oo.bar")
+				err2 = configtx.ValidateChannelID("f-oo.bar")
+				Expect(err1).NotTo(HaveOccurred())
+				Expect(err2).NotTo(HaveOccurred())
+			})
+		})
+	})
 })