[FAB-2183] fix RangeQuery key collision

When we do a range query on simple keys, it returns composite
keys too. This CR make sure that only simple keys are returned by
GetStateByRange() API. In order to achieve that, we make the
CreateCompositeKey() chaincode API to add a null character (0x00) as
a first character in composite key. This creates a separate namespace
for composite key. The simpleKey must not be an empty string as it is
treated as 0x00 in levelDB which results in collision between simple
and compositeKey. Hence, we do not allow empty string as key in PutState().

Further, we need to ensure that a simple key does not start with a
null character (0x00). Currently, we cannot impose this constraint on
simpleKey as PutState() is being used for storing both <simpleKey, value>
and <compositeKey, value> where the compositeKey must start with 0x00.
Hence, we have only documented this constraint on simpleKey (note that it
is unusual for a key to start with a null char). In future (post v1), we
may introduce PutCompositeKey() API so that we can explicitly impose this
constraint. Other approaches are listed in FAB-2183

Change-Id: I28aee64d81e07f2a504580b3fe87a182c130d82e
Signed-off-by: senthil <[email protected]>

Author: cendhu

core/chaincode/exectransaction_test.go

@@ -1167,6 +1167,34 @@ func TestQueries(t *testing.T) {
 		return
 	}
 
+	// querying for all simple key. This query should return exactly 101 simple keys (one
+	// call to Next()) no composite keys.
+	//The following open ended range query for "" to "" should return 101 marbles
+	f = "keys"
+	args = util.ToChaincodeArgs(f, "", "")
+
+	spec = &pb.ChaincodeSpec{Type: 1, ChaincodeId: cID, Input: &pb.ChaincodeInput{Args: args}}
+	_, _, retval, err = invoke(ctxt, chainID, spec, nextBlockNumber, nil)
+	nextBlockNumber++
+	if err != nil {
+		t.Fail()
+		t.Logf("Error invoking <%s>: %s", ccID, err)
+		theChaincodeSupport.Stop(ctxt, cccid, &pb.ChaincodeDeploymentSpec{ChaincodeSpec: spec})
+		return
+	}
+
+	//unmarshal the results
+	err = json.Unmarshal(retval, &keys)
+
+	//check to see if there are 101 values
+	//default query limit of 10000 is used, this query is effectively unlimited
+	if len(keys) != 101 {
+		t.Fail()
+		t.Logf("Error detected with the range query, should have returned 101 but returned %v", len(keys))
+		theChaincodeSupport.Stop(ctxt, cccid, &pb.ChaincodeDeploymentSpec{ChaincodeSpec: spec})
+		return
+	}
+
 	// ExecuteQuery supported only for CouchDB and
 	// query limits apply for CouchDB range and rich queries only
 	if ledgerconfig.IsCouchDBEnabled() == true {
@@ -1251,7 +1279,7 @@ func TestQueries(t *testing.T) {
 		//default query limit of 10000 is used, this query is effectively unlimited
 		if len(keys) != 50 {
 			t.Fail()
-			t.Logf("Error detected with the rich query, should have returned 9 but returned %v", len(keys))
+			t.Logf("Error detected with the rich query, should have returned 50 but returned %v", len(keys))
 			theChaincodeSupport.Stop(ctxt, cccid, &pb.ChaincodeDeploymentSpec{ChaincodeSpec: spec})
 			return
 		}

core/chaincode/shim/chaincode.go

@@ -47,8 +47,10 @@ var chaincodeLogger = logging.MustGetLogger("shim")
 var logOutput = os.Stderr
 
 const (
-	minUnicodeRuneValue = 0            //U+0000
-	maxUnicodeRuneValue = utf8.MaxRune //U+10FFFF - maximum (and unallocated) code point
+	minUnicodeRuneValue   = 0            //U+0000
+	maxUnicodeRuneValue   = utf8.MaxRune //U+10FFFF - maximum (and unallocated) code point
+	compositeKeyNamespace = "\x00"
+	emptyKeySubstitute    = "\x01"
 )
 
 // ChaincodeStub is an object passed to chaincode for shim side handling of
@@ -349,7 +351,14 @@ func (stub *ChaincodeStub) GetState(key string) ([]byte, error) {
 }
 
 // PutState writes the specified `value` and `key` into the ledger.
+// Simple keys must not be an empty string and must not start with null
+// character (0x00), in order to avoid range query collisions with
+// composite keys, which internally get prefixed with 0x00 as composite
+// key namespace.
 func (stub *ChaincodeStub) PutState(key string, value []byte) error {
+	if key == "" {
+		return fmt.Errorf("key must not be an empty string")
+	}
 	return stub.handler.handlePutState(key, value, stub.TxID)
 }
 
@@ -389,9 +398,17 @@ const (
 // GetStateByRange function can be invoked by a chaincode to query of a range
 // of keys in the state. Assuming the startKey and endKey are in lexical order,
 // an iterator will be returned that can be used to iterate over all keys
-// between the startKey and endKey, inclusive. The order in which keys are
-// returned by the iterator is random.
+// between the startKey and endKey. The startKey is inclusive whereas the endKey
+// is exclusive. The keys are returned by the iterator in lexical order. Note
+// that startKey and endKey can be empty string, which implies unbounded range
+// query on start or end.
 func (stub *ChaincodeStub) GetStateByRange(startKey, endKey string) (StateQueryIteratorInterface, error) {
+	if startKey == "" {
+		startKey = emptyKeySubstitute
+	}
+	if err := validateSimpleKeys(startKey, endKey); err != nil {
+		return nil, err
+	}
 	response, err := stub.handler.handleGetStateByRange(startKey, endKey, stub.TxID)
 	if err != nil {
 		return nil, err
@@ -436,7 +453,7 @@ func createCompositeKey(objectType string, attributes []string) (string, error)
 	if err := validateCompositeKeyAttribute(objectType); err != nil {
 		return "", err
 	}
-	ck := objectType + string(minUnicodeRuneValue)
+	ck := compositeKeyNamespace + objectType + string(minUnicodeRuneValue)
 	for _, att := range attributes {
 		if err := validateCompositeKeyAttribute(att); err != nil {
 			return "", err
@@ -447,9 +464,9 @@ func createCompositeKey(objectType string, attributes []string) (string, error)
 }
 
 func splitCompositeKey(compositeKey string) (string, []string, error) {
-	componentIndex := 0
+	componentIndex := 1
 	components := []string{}
-	for i := 0; i < len(compositeKey); i++ {
+	for i := 1; i < len(compositeKey); i++ {
 		if compositeKey[i] == minUnicodeRuneValue {
 			components = append(components, compositeKey[componentIndex:i])
 			componentIndex = i + 1
@@ -471,23 +488,35 @@ func validateCompositeKeyAttribute(str string) error {
 	return nil
 }
 
+//To ensure that simple keys do not go into composite key namespace,
+//we validate simplekey to check whether the key starts with 0x00 (which
+//is the namespace for compositeKey). This helps in avoding simple/composite
+//key collisions.
+func validateSimpleKeys(simpleKeys ...string) error {
+	for _, key := range simpleKeys {
+		if len(key) > 0 && key[0] == compositeKeyNamespace[0] {
+			return fmt.Errorf(`First character of the key [%s] contains a null character which is not allowed`, key)
+		}
+	}
+	return nil
+}
+
 //GetStateByPartialCompositeKey function can be invoked by a chaincode to query the
 //state based on a given partial composite key. This function returns an
 //iterator which can be used to iterate over all composite keys whose prefix
 //matches the given partial composite key. This function should be used only for
 //a partial composite key. For a full composite key, an iter with empty response
 //would be returned.
 func (stub *ChaincodeStub) GetStateByPartialCompositeKey(objectType string, attributes []string) (StateQueryIteratorInterface, error) {
-	return getStateByPartialCompositeKey(stub, objectType, attributes)
-}
-
-func getStateByPartialCompositeKey(stub ChaincodeStubInterface, objectType string, attributes []string) (StateQueryIteratorInterface, error) {
-	partialCompositeKey, _ := stub.CreateCompositeKey(objectType, attributes)
-	keysIter, err := stub.GetStateByRange(partialCompositeKey, partialCompositeKey+string(maxUnicodeRuneValue))
+	partialCompositeKey, err := stub.CreateCompositeKey(objectType, attributes)
+	if err != nil {
+		return nil, err
+	}
+	response, err := stub.handler.handleGetStateByRange(partialCompositeKey, partialCompositeKey+string(maxUnicodeRuneValue), stub.TxID)
 	if err != nil {
-		return nil, fmt.Errorf("Error fetching rows: %s", err)
+		return nil, err
 	}
-	return keysIter, nil
+	return &StateQueryIterator{CommonIterator: &CommonIterator{stub.handler, stub.TxID, response, 0}}, nil
 }
 
 func (iter *StateQueryIterator) Next() (*queryresult.KV, error) {

core/chaincode/shim/mockstub.go

@@ -209,6 +209,9 @@ func (stub *MockStub) DelState(key string) error {
 }
 
 func (stub *MockStub) GetStateByRange(startKey, endKey string) (StateQueryIteratorInterface, error) {
+	if err := validateSimpleKeys(startKey, endKey); err != nil {
+		return nil, err
+	}
 	return NewMockStateRangeQueryIterator(stub, startKey, endKey), nil
 }
 
@@ -237,7 +240,11 @@ func (stub *MockStub) GetHistoryForKey(key string) (HistoryQueryIteratorInterfac
 //a partial composite key. For a full composite key, an iter with empty response
 //would be returned.
 func (stub *MockStub) GetStateByPartialCompositeKey(objectType string, attributes []string) (StateQueryIteratorInterface, error) {
-	return getStateByPartialCompositeKey(stub, objectType, attributes)
+	partialCompositeKey, err := stub.CreateCompositeKey(objectType, attributes)
+	if err != nil {
+		return nil, err
+	}
+	return NewMockStateRangeQueryIterator(stub, partialCompositeKey, partialCompositeKey+string(maxUnicodeRuneValue)), nil
 }
 
 // CreateCompositeKey combines the list of attributes

examples/chaincode/go/map/map.go

@@ -55,11 +55,23 @@ func (t *SimpleChaincode) Invoke(stub shim.ChaincodeStubInterface) pb.Response {
 		key := args[0]
 		value := args[1]
 
-		err := stub.PutState(key, []byte(value))
-		if err != nil {
+		if err := stub.PutState(key, []byte(value)); err != nil {
 			fmt.Printf("Error putting state %s", err)
 			return shim.Error(fmt.Sprintf("put operation failed. Error updating state: %s", err))
 		}
+
+		indexName := "compositeKeyTest"
+		compositeKeyTestIndex, err := stub.CreateCompositeKey(indexName, []string{key})
+		if err != nil {
+			return shim.Error(err.Error())
+		}
+
+		valueByte := []byte{0x00}
+		if err := stub.PutState(compositeKeyTestIndex, valueByte); err != nil {
+			fmt.Printf("Error putting state with compositeKey %s", err)
+			return shim.Error(fmt.Sprintf("put operation failed. Error updating state with compositeKey: %s", err))
+		}
+
 		return shim.Success(nil)
 
 	case "remove":