From 0e7ffae0b1900d5ac9cb9c368397e90bb11a08f9 Mon Sep 17 00:00:00 2001 From: YACOVM Date: Wed, 24 May 2017 20:01:30 +0300 Subject: [PATCH] [FAB-4146] chaincode install panics if GOPATH empty If the GOPATH isn't set, the command peer chaincode install panics because of golang/platform.go I added a test that simulates such a condition. Also added one test which uses an invalid URL path in order to improve the UT coverage for ValidateSpec(). Change-Id: I1c9ed3406078bddf48e866bd0ca9a856a1c0848e Signed-off-by: Yacov Manevich Signed-off-by: Will Lahti --- core/chaincode/platforms/golang/platform.go | 17 ++++++---- .../platforms/golang/platform_test.go | 34 ++++++++++++++----- 2 files changed, 36 insertions(+), 15 deletions(-) diff --git a/core/chaincode/platforms/golang/platform.go b/core/chaincode/platforms/golang/platform.go index 81c2f1ca0f0..2a2e00a3de0 100644 --- a/core/chaincode/platforms/golang/platform.go +++ b/core/chaincode/platforms/golang/platform.go @@ -25,9 +25,8 @@ import ( "net/url" "os" "path/filepath" - "strings" - "regexp" + "strings" "github.com/hyperledger/fabric/core/chaincode/platforms/util" cutil "github.com/hyperledger/fabric/core/container/util" @@ -84,14 +83,18 @@ func (goPlatform *Platform) ValidateSpec(spec *pb.ChaincodeSpec) error { if path.Scheme == "" { gopath := os.Getenv("GOPATH") // Only take the first element of GOPATH - gopath = filepath.SplitList(gopath)[0] + splitGoPath := filepath.SplitList(gopath) + if len(splitGoPath) == 0 { + return fmt.Errorf("invalid GOPATH environment variable value:[%s]", gopath) + } + gopath = splitGoPath[0] pathToCheck := filepath.Join(gopath, "src", spec.ChaincodeId.Path) exists, err := pathExists(pathToCheck) if err != nil { - return fmt.Errorf("Error validating chaincode path: %s", err) + return fmt.Errorf("error validating chaincode path: %s", err) } if !exists { - return fmt.Errorf("Path to chaincode does not exist: %s", spec.ChaincodeId.Path) + return fmt.Errorf("path to chaincode does not exist: %s", spec.ChaincodeId.Path) } } return nil @@ -133,7 +136,7 @@ func (goPlatform *Platform) ValidateDeploymentSpec(cds *pb.ChaincodeDeploymentSp // Check name for conforming path // -------------------------------------------------------------------------------------- if !re.MatchString(header.Name) { - return fmt.Errorf("Illegal file detected in payload: \"%s\"", header.Name) + return fmt.Errorf("illegal file detected in payload: \"%s\"", header.Name) } // -------------------------------------------------------------------------------------- @@ -146,7 +149,7 @@ func (goPlatform *Platform) ValidateDeploymentSpec(cds *pb.ChaincodeDeploymentSp // Anything else is suspect in this context and will be rejected // -------------------------------------------------------------------------------------- if header.Mode&^0100666 != 0 { - return fmt.Errorf("Illegal file mode detected for file %s: %o", header.Name, header.Mode) + return fmt.Errorf("illegal file mode detected for file %s: %o", header.Name, header.Mode) } } diff --git a/core/chaincode/platforms/golang/platform_test.go b/core/chaincode/platforms/golang/platform_test.go index 126b085aa39..cdbe254063c 100644 --- a/core/chaincode/platforms/golang/platform_test.go +++ b/core/chaincode/platforms/golang/platform_test.go @@ -17,20 +17,19 @@ limitations under the License. package golang import ( + "archive/tar" + "bytes" + "compress/gzip" "fmt" "os" "strings" "testing" - - "archive/tar" - "bytes" - "compress/gzip" "time" - "github.com/spf13/viper" - "github.com/hyperledger/fabric/core/config" pb "github.com/hyperledger/fabric/protos/peer" + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" ) func testerr(err error, succ bool) error { @@ -108,6 +107,24 @@ func TestValidateCDS(t *testing.T) { } } +func TestPlatform_GoPathNotSet(t *testing.T) { + p := &Platform{} + spec := &pb.ChaincodeSpec{ + ChaincodeId: &pb.ChaincodeID{ + Path: "/opt/gopath/src/github.com/hyperledger/fabric", + }, + } + gopath := os.Getenv("GOPATH") + defer os.Setenv("GOPATH", gopath) + os.Setenv("GOPATH", "") + + f := func() { + p.ValidateSpec(spec) + } + assert.NotPanics(t, f) + assert.Contains(t, p.ValidateSpec(spec).Error(), "invalid GOPATH environment variable value") +} + func Test_writeGopathSrc(t *testing.T) { inputbuf := bytes.NewBuffer(nil) @@ -157,7 +174,7 @@ func Test_decodeUrl(t *testing.T) { } } -func TestValidChaincodeSpec(t *testing.T) { +func TestValidateSpec(t *testing.T) { platform := &Platform{} var tests = []struct { @@ -168,12 +185,13 @@ func TestValidChaincodeSpec(t *testing.T) { {spec: &pb.ChaincodeSpec{ChaincodeId: &pb.ChaincodeID{Name: "Test Chaincode", Path: "https://github.com/hyperledger/fabric/examples/chaincode/go/map"}}, succ: true}, {spec: &pb.ChaincodeSpec{ChaincodeId: &pb.ChaincodeID{Name: "Test Chaincode", Path: "github.com/hyperledger/fabric/examples/chaincode/go/map"}}, succ: true}, {spec: &pb.ChaincodeSpec{ChaincodeId: &pb.ChaincodeID{Name: "Test Chaincode", Path: "github.com/hyperledger/fabric/bad/chaincode/go/map"}}, succ: false}, + {spec: &pb.ChaincodeSpec{ChaincodeId: &pb.ChaincodeID{Name: "Test Chaincode", Path: ":github.com/hyperledger/fabric/examples/chaincode/go/map"}}, succ: false}, } for _, tst := range tests { err := platform.ValidateSpec(tst.spec) if err = testerr(err, tst.succ); err != nil { - t.Errorf("Error to validating chaincode spec: %s, %s", tst.spec.ChaincodeId.Path, err) + t.Errorf("Error validating chaincode spec: %s, %s", tst.spec.ChaincodeId.Path, err) } } }