Skip to content

Commit

Permalink
Chaincode persistence uses period separator (#492)
Browse files Browse the repository at this point in the history 
- Use a period separator between the chaincode label and hash
- Update chaincode store file matcher to explicitly look for files with
  a suffix of exactly 64 hex characters + ".tar.gz" to reduce depenency
  on special characters in the label.

FAB-17315

Change-Id: I9c14ac0a5b7adaa4c5ca50a487e1fba1d87f8b4b
Signed-off-by: Matthew Sykes <sykesmat@us.ibm.com>
  • Loading branch information
@sykesm
sykesm authored and Jason Yellick committed Jan 15, 2020
1 parent 143d2e9 commit b578a96
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 31 deletions.
8 changes: 3 additions & 5 deletions core/chaincode/persistence/chaincode_package.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,11 +240,9 @@ type ChaincodePackageParser struct {
MetadataProvider MetadataProvider
}

var (
// LabelRegexp is the regular expression controlling
// the allowed characters for the package label
LabelRegexp = regexp.MustCompile(`^[[:alnum:]][[:alnum:]_.+-]*$`)
)
// LabelRegexp is the regular expression controlling the allowed characters
// for the package label.
var LabelRegexp = regexp.MustCompile(`^[[:alnum:]][[:alnum:]_.+-]*$`)

func validateLabel(label string) error {
if !LabelRegexp.MatchString(label) {
Expand Down
4 changes: 3 additions & 1 deletion core/chaincode/persistence/label_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,21 @@ func TestLabels(t *testing.T) {
{label: "a#", success: false},
{label: "a$", success: false},
{label: "a%", success: false},
{label: "a-", success: true},
{label: "a++b", success: true},
{label: "a+b", success: true},
{label: "a+bb", success: true},
{label: "a-", success: true},
{label: "a--b", success: true},
{label: "a-b", success: true},
{label: "a-bb", success: true},
{label: "a.b", success: true},
{label: "a::b", success: false},
{label: "a:b", success: false},
{label: "a__b", success: true},
{label: "a_b", success: true},
{label: "a_bb", success: true},
{label: "aa", success: true},
{label: "v1.0.0", success: true},
}

for _, tt := range tests {
Expand Down
7 changes: 4 additions & 3 deletions core/chaincode/persistence/persistence.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"os"
"path/filepath"
"regexp"
"strings"

"github.com/hyperledger/fabric/common/chaincode"
"github.com/hyperledger/fabric/common/flogging"
Expand Down Expand Up @@ -230,14 +231,14 @@ func (s *Store) GetChaincodeInstallPath() string {
}

func packageID(label string, hash []byte) string {
return fmt.Sprintf("%s~%x", label, hash)
return fmt.Sprintf("%s:%x", label, hash)
}

func CCFileName(packageID string) string {
return packageID + ".tar.gz"
return strings.Replace(packageID, ":", ".", 1) + ".tar.gz"
}

var packageFileMatcher = regexp.MustCompile("^(.+)~([0-9abcdef]+)[.]tar[.]gz$")
var packageFileMatcher = regexp.MustCompile("^(.+)[.]([0-9a-f]{64})[.]tar[.]gz$")

func installedChaincodeFromFilename(fileName string) (chaincode.InstalledChaincode, bool) {
matches := packageFileMatcher.FindStringSubmatch(fileName)
Expand Down
59 changes: 38 additions & 21 deletions core/chaincode/persistence/persistence_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ import (
"path/filepath"

"github.com/hyperledger/fabric/common/chaincode"
"github.com/hyperledger/fabric/common/util"
"github.com/hyperledger/fabric/core/chaincode/persistence"
"github.com/hyperledger/fabric/core/chaincode/persistence/mock"
. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/extensions/table"
. "github.com/onsi/gomega"
"github.com/pkg/errors"
)
Expand Down Expand Up @@ -227,11 +229,11 @@ var _ = Describe("Persistence", func() {
It("saves a new code package successfully", func() {
packageID, err := store.Save("testcc", pkgBytes)
Expect(err).NotTo(HaveOccurred())
Expect(packageID).To(Equal("testcc~3fec0187440286d404241e871b44725310b11aaf43d100b053eae712fcabc66d"))
Expect(packageID).To(Equal("testcc:3fec0187440286d404241e871b44725310b11aaf43d100b053eae712fcabc66d"))
Expect(mockReadWriter.WriteFileCallCount()).To(Equal(1))
pkgDataFilePath, pkgDataFileName, pkgData := mockReadWriter.WriteFileArgsForCall(0)
Expect(pkgDataFilePath).To(Equal(""))
Expect(pkgDataFileName).To(Equal("testcc~3fec0187440286d404241e871b44725310b11aaf43d100b053eae712fcabc66d.tar.gz"))
Expect(pkgDataFileName).To(Equal("testcc.3fec0187440286d404241e871b44725310b11aaf43d100b053eae712fcabc66d.tar.gz"))
Expect(pkgData).To(Equal([]byte("testpkg")))
})

Expand All @@ -243,7 +245,7 @@ var _ = Describe("Persistence", func() {
It("does nothing and returns the packageID", func() {
packageID, err := store.Save("testcc", pkgBytes)
Expect(err).NotTo(HaveOccurred())
Expect(packageID).To(Equal("testcc~3fec0187440286d404241e871b44725310b11aaf43d100b053eae712fcabc66d"))
Expect(packageID).To(Equal("testcc:3fec0187440286d404241e871b44725310b11aaf43d100b053eae712fcabc66d"))
Expect(mockReadWriter.WriteFileCallCount()).To(Equal(0))
})
})
Expand All @@ -256,7 +258,7 @@ var _ = Describe("Persistence", func() {
It("returns an error", func() {
packageID, err := store.Save("testcc", pkgBytes)
Expect(packageID).To(Equal(""))
Expect(err).To(MatchError(ContainSubstring("error writing chaincode install package to testcc~3fec0187440286d404241e871b44725310b11aaf43d100b053eae712fcabc66d.tar.gz: soccer")))
Expect(err).To(MatchError(ContainSubstring("error writing chaincode install package to testcc.3fec0187440286d404241e871b44725310b11aaf43d100b053eae712fcabc66d.tar.gz: soccer")))
})
})
})
Expand Down Expand Up @@ -358,14 +360,17 @@ var _ = Describe("Persistence", func() {
var (
mockReadWriter *mock.IOReadWriter
store *persistence.Store
hash1, hash2 []byte
)

BeforeEach(func() {
hash1 = util.ComputeSHA256([]byte("hash1"))
hash2 = util.ComputeSHA256([]byte("hash2"))
mockReadWriter = &mock.IOReadWriter{}
mockFileInfo := &mock.OSFileInfo{}
mockFileInfo.NameReturns(fmt.Sprintf("%s~%x.tar.gz", "label1", []byte("hash1")))
mockFileInfo.NameReturns(fmt.Sprintf("%s.%x.tar.gz", "label1", hash1))
mockFileInfo2 := &mock.OSFileInfo{}
mockFileInfo2.NameReturns(fmt.Sprintf("%s~%x.tar.gz", "label2", []byte("hash2")))
mockFileInfo2.NameReturns(fmt.Sprintf("%s.%x.tar.gz", "label2", hash2))
mockReadWriter.ReadDirReturns([]os.FileInfo{mockFileInfo, mockFileInfo2}, nil)
store = &persistence.Store{
ReadWriter: mockReadWriter,
Expand All @@ -377,27 +382,31 @@ var _ = Describe("Persistence", func() {
Expect(err).NotTo(HaveOccurred())
Expect(installedChaincodes).To(HaveLen(2))
Expect(installedChaincodes[0]).To(Equal(chaincode.InstalledChaincode{
Hash: []byte("hash1"),
Hash: hash1,
Label: "label1",
PackageID: "label1~6861736831",
PackageID: fmt.Sprintf("label1:%x", hash1),
}))
Expect(installedChaincodes[1]).To(Equal(chaincode.InstalledChaincode{
Hash: []byte("hash2"),
Hash: hash2,
Label: "label2",
PackageID: "label2~6861736832",
PackageID: fmt.Sprintf("label2:%x", hash2),
}))
})

Context("when extraneous files are present", func() {
var hash1, hash2 []byte

BeforeEach(func() {
hash1 = util.ComputeSHA256([]byte("hash1"))
hash2 = util.ComputeSHA256([]byte("hash2"))
mockFileInfo := &mock.OSFileInfo{}
mockFileInfo.NameReturns(fmt.Sprintf("%s~%x.tar.gz", "label1", []byte("hash1")))
mockFileInfo.NameReturns(fmt.Sprintf("%s.%x.tar.gz", "label1", hash1))
mockFileInfo2 := &mock.OSFileInfo{}
mockFileInfo2.NameReturns(fmt.Sprintf("%s~%x.tar.gz", "label2", []byte("hash2")))
mockFileInfo2.NameReturns(fmt.Sprintf("%s.%x.tar.gz", "label2", hash2))
mockFileInfo3 := &mock.OSFileInfo{}
mockFileInfo3.NameReturns(fmt.Sprintf("%s~%x.tar.gz", "", "Musha rain dum a doo, dum a da"))
mockFileInfo3.NameReturns(fmt.Sprintf("%s.%x.tar.gz", "", "Musha rain dum a doo, dum a da"))
mockFileInfo4 := &mock.OSFileInfo{}
mockFileInfo4.NameReturns(fmt.Sprintf("%s~%x.tar.gz", "", "barfity~barf.tar.gz"))
mockFileInfo4.NameReturns(fmt.Sprintf("%s.%x.tar.gz", "", "barfity:barf.tar.gz"))
mockReadWriter.ReadDirReturns([]os.FileInfo{mockFileInfo, mockFileInfo2, mockFileInfo3}, nil)
})

Expand All @@ -406,14 +415,14 @@ var _ = Describe("Persistence", func() {
Expect(err).NotTo(HaveOccurred())
Expect(installedChaincodes).To(HaveLen(2))
Expect(installedChaincodes[0]).To(Equal(chaincode.InstalledChaincode{
Hash: []byte("hash1"),
Hash: hash1,
Label: "label1",
PackageID: "label1~6861736831",
PackageID: fmt.Sprintf("label1:%x", hash1),
}))
Expect(installedChaincodes[1]).To(Equal(chaincode.InstalledChaincode{
Hash: []byte("hash2"),
Hash: hash2,
Label: "label2",
PackageID: "label2~6861736832",
PackageID: fmt.Sprintf("label2:%x", hash2),
}))
})
})
Expand All @@ -432,9 +441,7 @@ var _ = Describe("Persistence", func() {
})

Describe("GetChaincodeInstallPath", func() {
var (
store *persistence.Store
)
var store *persistence.Store

BeforeEach(func() {
store = &persistence.Store{
Expand All @@ -447,4 +454,14 @@ var _ = Describe("Persistence", func() {
Expect(path).To(Equal("testPath"))
})
})

DescribeTable("CCFileName",
func(packageID, expectedName string) {
Expect(persistence.CCFileName(packageID)).To(Equal(expectedName))
},
Entry("label with dot and without hash", "aaa.bbb", "aaa.bbb.tar.gz"),
Entry("label and hash with colon delimeter", "aaa:bbb", "aaa.bbb.tar.gz"),
Entry("missing label with colon delimeter", ":bbb", ".bbb.tar.gz"),
Entry("missing hash with colon delimeter", "aaa:", "aaa..tar.gz"),
)
})
2 changes: 1 addition & 1 deletion integration/nwo/deploy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (c *Chaincode) SetPackageIDFromPackageFile() {
fileBytes, err := ioutil.ReadFile(c.PackageFile)
Expect(err).NotTo(HaveOccurred())
hashStr := fmt.Sprintf("%x", util.ComputeSHA256(fileBytes))
c.PackageID = c.Label + "~" + hashStr
c.PackageID = c.Label + ":" + hashStr
}

// DeployChaincode is a helper that will install chaincode to all peers that
Expand Down

0 comments on commit b578a96

Please sign in to comment.