[FAB-9476]: WithTargets() should check for nil targets

Change-Id: I3daf638c0c4705664325819ddce4b0c28c4ee226
Signed-off-by: Sandra Vrtikapa <sandra.vrtikapa@securekey.com>

pkg/client/channel/api.go

@@ -51,6 +51,14 @@ type Response struct {
 //WithTargets encapsulates ProposalProcessors to Option
 func WithTargets(targets ...fab.Peer) RequestOption {
 	return func(ctx context.Client, o *requestOptions) error {
+
+		// Validate targets
+		for _, t := range targets {
+			if t == nil {
+				return errors.New("target is nil")
+			}
+		}
+
 		o.Targets = targets
 		return nil
 	}

pkg/client/channel/chclient_test.go

@@ -8,6 +8,7 @@ package channel
 
 import (
 	"fmt"
+	"strings"
 	"testing"
 	"time"
 
@@ -175,6 +176,23 @@ func TestQueryWithOptTarget(t *testing.T) {
 	}
 }
 
+func TestQueryWithNilTargets(t *testing.T) {
+	chClient := setupChannelClient(nil, t)
+
+	_, err := chClient.Query(Request{ChaincodeID: "testCC", Fcn: "invoke",
+		Args: [][]byte{[]byte("query"), []byte("b")}}, WithTargets(nil, nil))
+	if err == nil || !strings.Contains(err.Error(), "target is nil") {
+		t.Fatalf("Should have failed to invoke test cc due to nil target")
+	}
+
+	peers := []fab.Peer{fcmocks.NewMockPeer("Peer1", "http://peer1.com"), nil}
+	_, err = chClient.Query(Request{ChaincodeID: "testCC", Fcn: "invoke",
+		Args: [][]byte{[]byte("query"), []byte("b")}}, WithTargets(peers...))
+	if err == nil || !strings.Contains(err.Error(), "target is nil") {
+		t.Fatalf("Should have failed to invoke test cc due to nil target")
+	}
+}
+
 func TestExecuteTx(t *testing.T) {
 	chClient := setupChannelClient(nil, t)
 

pkg/client/ledger/ledger_test.go

@@ -65,6 +65,17 @@ func TestQueryBlock(t *testing.T) {
 
 }
 
+func TestQueryBlockWithNilTargets(t *testing.T) {
+
+	peer1 := &mocks.MockPeer{MockName: "Peer1", MockURL: "http://peer1.com", MockRoles: []string{}, MockCert: nil, Status: 200, MockMSP: "test"}
+	lc := setupLedgerClient([]fab.Peer{peer1}, t)
+
+	_, err := lc.QueryBlock(1, WithTargets(peer1, nil))
+	if err == nil || !strings.Contains(err.Error(), "target is nil") {
+		t.Fatalf("Should have failed due to nil target")
+	}
+}
+
 func TestQueryBlockDiscoveryError(t *testing.T) {
 	peer1 := mocks.MockPeer{MockName: "Peer1", MockURL: "http://peer1.com", MockRoles: []string{}, MockCert: nil, Status: 200, MockMSP: "test"}
 	peer2 := mocks.MockPeer{MockName: "Peer2", MockURL: "http://peer2.com", MockRoles: []string{}, MockCert: nil, Status: 200, MockMSP: "test"}

pkg/client/ledger/opts.go

@@ -48,6 +48,14 @@ type requestOptions struct {
 //WithTargets encapsulates fab.Peer targets to ledger RequestOption
 func WithTargets(targets ...fab.Peer) RequestOption {
 	return func(ctx context.Client, opts *requestOptions) error {
+
+		// Validate targets
+		for _, t := range targets {
+			if t == nil {
+				return errors.New("target is nil")
+			}
+		}
+
 		opts.Targets = targets
 		return nil
 	}

pkg/client/resmgmt/opts.go

@@ -20,6 +20,14 @@ import (
 // WithTargets allows overriding of the target peers for the request.
 func WithTargets(targets ...fab.Peer) RequestOption {
 	return func(ctx context.Client, opts *requestOptions) error {
+
+		// Validate targets
+		for _, t := range targets {
+			if t == nil {
+				return errors.New("target is nil")
+			}
+		}
+
 		opts.Targets = targets
 		return nil
 	}

pkg/client/resmgmt/resmgmt_test.go

@@ -67,12 +67,18 @@ func TestJoinChannelFail(t *testing.T) {
 
 	rc := setupResMgmtClient(ctx, nil, t)
 
+	// Test nil target
+	err := rc.JoinChannel("mychannel", WithTargets(nil))
+	if err == nil || !strings.Contains(err.Error(), "target is nil") {
+		t.Fatalf("Should have failed due to nil target")
+	}
+
 	// Setup target peers
 	peer1, _ := peer.New(fcmocks.NewMockEndpointConfig(), peer.WithURL("grpc://"+addr))
 
 	// Test fail with send proposal error
 	endorserServer.ProposalError = errors.New("Test Error")
-	err := rc.JoinChannel("mychannel", WithTargets(peer1))
+	err = rc.JoinChannel("mychannel", WithTargets(peer1))
 
 	if err == nil {
 		t.Fatal("Should have failed to get genesis block")

pkg/fab/txn/proposal.go

@@ -88,6 +88,12 @@ func SendProposal(reqCtx reqContext.Context, proposal *fab.TransactionProposal,
 		return nil, errors.New("targets is required")
 	}
 
+	for _, p := range targets {
+		if p == nil {
+			return nil, errors.New("target is nil")
+		}
+	}
+
 	ctx, ok := context.RequestClientContext(reqCtx)
 	if !ok {
 		return nil, errors.New("failed get client context from reqContext for signProposal")

pkg/fab/txn/proposal_test.go

@@ -8,6 +8,7 @@ package txn
 import (
 	"fmt"
 	"reflect"
+	"strings"
 	"testing"
 
 	"github.com/golang/mock/gomock"
@@ -88,7 +89,12 @@ func TestSendTransactionProposal(t *testing.T) {
 	reqCtx, cancel := context.NewRequest(ctx, context.WithTimeout(10*time.Second))
 	defer cancel()
 
-	tpr, err := SendProposal(reqCtx, tp, []fab.ProposalProcessor{&peer})
+	tpr, err := SendProposal(reqCtx, tp, []fab.ProposalProcessor{nil})
+	if err == nil || !strings.Contains(err.Error(), "target is nil") {
+		t.Fatalf("Should have failed due to nil target")
+	}
+
+	tpr, err = SendProposal(reqCtx, tp, []fab.ProposalProcessor{&peer})
 	if err != nil {
 		t.Fatalf("send transaction proposal failed: %s", err)
 	}

test/integration/orgs/multiple_orgs_test.go

@@ -70,6 +70,9 @@ func TestOrgsEndToEnd(t *testing.T) {
 	integration.CleanupUserData(t, sdk)
 	defer integration.CleanupUserData(t, sdk)
 
+	// Load specific targets for move funds test
+	loadOrgPeers(t, sdk.Context(fabsdk.WithUser(org1AdminUser), fabsdk.WithOrg(org1)))
+
 	expectedValue := testWithOrg1(t, sdk)
 	expectedValue = testWithOrg2(t, expectedValue)
 	verifyWithOrg1(t, sdk, expectedValue)
@@ -159,9 +162,6 @@ func testWithOrg1(t *testing.T, sdk *fabsdk.FabricSDK) int {
 	assert.Nil(t, err, "error should be nil")
 	assert.NotEmpty(t, instantiateResp, "transaction response should be populated")
 
-	// Load specific targets for move funds test
-	loadOrgPeers(t, org1AdminClientContext)
-
 	// Verify that example CC is instantiated on Org1 peer
 	chaincodeQueryResponse, err := org1ResMgmt.QueryInstantiatedChaincodes("orgchannel", resmgmt.WithRetry(retry.DefaultResMgmtOpts))
 	if err != nil {