Skip to content

Commit

Permalink
Fix the issue of Nil/Zero-length-byte-array value (#2312)
Browse files Browse the repository at this point in the history
Signed-off-by: manish <[email protected]>
  • Loading branch information
manish-sethi authored Jan 26, 2021
1 parent e95a134 commit 3813607
Show file tree
Hide file tree
Showing 14 changed files with 495 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ func getKeyModificationFromTran(tranEnvelope *common.Envelope, namespace string,
for _, kvWrite := range nsRWSet.KvRwSet.Writes {
if kvWrite.Key == key {
return &queryresult.KeyModification{TxId: txID, Value: kvWrite.Value,
Timestamp: timestamp, IsDelete: kvWrite.IsDelete}, nil
Timestamp: timestamp, IsDelete: rwsetutil.IsKVWriteDelete(kvWrite)}, nil
}
} // end keys loop
logger.Debugf("key [%s] not found in namespace [%s]'s writeset", key, namespace)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"strconv"
"testing"

"github.com/golang/protobuf/proto"
configtxtest "github.com/hyperledger/fabric/common/configtx/test"
"github.com/hyperledger/fabric/common/flogging"
"github.com/hyperledger/fabric/common/ledger/testutil"
Expand All @@ -23,9 +24,12 @@ import (
"github.com/hyperledger/fabric/core/ledger/util"
"github.com/hyperledger/fabric/protos/common"
"github.com/hyperledger/fabric/protos/ledger/queryresult"
"github.com/hyperledger/fabric/protos/ledger/rwset"
"github.com/hyperledger/fabric/protos/ledger/rwset/kvrwset"
"github.com/hyperledger/fabric/protos/peer"
"github.com/spf13/viper"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestMain(m *testing.M) {
Expand Down Expand Up @@ -526,6 +530,61 @@ func TestName(t *testing.T) {
assert.Equal(t, "history", env.testHistoryDB.Name())
}

// TestHistoryWithKVWriteOfNilValue - See FAB-18386 for details
func TestHistoryWithKVWriteOfNilValue(t *testing.T) {
env := newTestHistoryEnv(t)
defer env.cleanup()
provider := env.testBlockStorageEnv.provider
store, err := provider.OpenBlockStore("ledger1")
require.NoError(t, err)
defer store.Shutdown()

bg, gb := testutil.NewBlockGenerator(t, "ledger1", false)

kvRWSet := &kvrwset.KVRWSet{
Writes: []*kvrwset.KVWrite{
// explicitly set IsDelete to false while the value to nil. As this will never be generated by simulation
{Key: "key1", IsDelete: false, Value: nil},
},
}
kvRWsetBytes, err := proto.Marshal(kvRWSet)
require.NoError(t, err)

txRWSet := &rwset.TxReadWriteSet{
NsRwset: []*rwset.NsReadWriteSet{
{
Namespace: "ns1",
Rwset: kvRWsetBytes,
},
},
}

txRWSetBytes, err := proto.Marshal(txRWSet)
require.NoError(t, err)

block1 := bg.NextBlockWithTxid([][]byte{txRWSetBytes}, []string{"txid1"})

historydb := env.testHistoryDB
require.NoError(t, store.AddBlock(gb))
require.NoError(t, historydb.Commit(gb))
require.NoError(t, store.AddBlock(block1))
require.NoError(t, historydb.Commit(block1))

historydbQE, err := historydb.NewHistoryQueryExecutor(store)
require.NoError(t, err)
itr, err := historydbQE.GetHistoryForKey("ns1", "key1")
require.NoError(t, err)
kmod, err := itr.Next()
require.NoError(t, err)
keyModification := kmod.(*queryresult.KeyModification)
// despite IsDelete set to "false" in the write-set, historydb results should set this to "true"
require.True(t, keyModification.IsDelete)

kmod, err = itr.Next()
require.NoError(t, err)
require.Nil(t, kmod)
}

func testutilVerifyResults(t *testing.T, hqe ledger.HistoryQueryExecutor, ns, key string, expectedVals []string) {
itr, err := hqe.GetHistoryForKey(ns, key)
assert.NoError(t, err, "Error upon GetHistoryForKey()")
Expand Down
4 changes: 4 additions & 0 deletions core/ledger/kvledger/tests/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ func (c *client) simulateDataTx(txid string, simulationLogic func(s *simulator))
return txAndPvtdata
}

func (c *client) submitHandCraftedTx(txAndPvtdata *txAndPvtdata) {
c.simulatedTrans = append(c.simulatedTrans, txAndPvtdata)
}

// simulateDeployTx mimics a transction that deploys a chaincode. This in turn calls the function 'simulateDataTx'
// with supplying the simulation logic that mimics the inoke funciton of 'lscc' for the ledger tests
func (c *client) simulateDeployTx(ccName string, collConfs []*collConf) *txAndPvtdata {
Expand Down
154 changes: 154 additions & 0 deletions core/ledger/kvledger/tests/nilvalue_no_delete_marker_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
/*
Copyright IBM Corp. All Rights Reserved.
SPDX-License-Identifier: Apache-2.0
*/

package tests

import (
"testing"

"github.com/golang/protobuf/proto"
"github.com/hyperledger/fabric/common/util"
"github.com/hyperledger/fabric/protos/ledger/rwset"
"github.com/hyperledger/fabric/protos/ledger/rwset/kvrwset"
"github.com/hyperledger/fabric/protos/peer"
"github.com/stretchr/testify/require"
)

// TestNilValNoDeleteMarker tests for a special writeset which carries a nil value and yet the delete marker is set to false.
// This kind of write-set gets produced in previous versions. See FAB-18386 for more details.
func TestNilValNoDeleteMarker(t *testing.T) {
env := newEnv(
config{
"peer.fileSystemPath": "/tmp/fabric/ledgertests",
"ledger.state.stateDatabase": "goleveldb",
"ledger.history.enableHistoryDatabase": true,
},
t,
)
defer env.cleanup()

testLedger := newTestHelperCreateLgr("test-ledger", t)
testLedger.simulateDeployTx("cc1", []*collConf{
{
name: "coll1",
},
})
testLedger.cutBlockAndCommitWithPvtdata()

testLedger.simulateDataTx("txid1", func(s *simulator) {
s.setState("cc1", "pubKey1", "pubValue1")
s.setPvtdata("cc1", "coll1", "pvtKey1", "pvtValue1")
s.setPvtdata("cc1", "coll1", "pvtKey2", "pvtValue2")
})
testLedger.cutBlockAndCommitWithPvtdata()

testLedger.verifyPubState("cc1", "pubKey1", "pubValue1")
testLedger.verifyPvtdataHashState("cc1", "coll1", "pvtKey1", util.ComputeSHA256([]byte("pvtValue1")))
testLedger.verifyPvtdataHashState("cc1", "coll1", "pvtKey2", util.ComputeSHA256([]byte("pvtValue2")))
testLedger.verifyPvtState("cc1", "coll1", "pvtKey1", "pvtValue1")
testLedger.verifyPvtState("cc1", "coll1", "pvtKey2", "pvtValue2")

// Handcraft writeset that includes a delete for each of the above keys. We handcraft here because the one generated by the simulator
// will always have IsDelete flag true and we want to test when this flag is set to false and the actual value is nil. See FAB-18386 for
// more details.
pubWrites := &kvrwset.KVRWSet{
Writes: []*kvrwset.KVWrite{
{
Key: "pubKey1",
IsDelete: false,
Value: nil,
},
},
}

hashedWrites := &kvrwset.HashedRWSet{
HashedWrites: []*kvrwset.KVWriteHash{
{
KeyHash: util.ComputeSHA256([]byte("pvtKey1")),
IsDelete: false,
ValueHash: nil,
},
{
KeyHash: util.ComputeSHA256([]byte("pvtKey2")),
IsDelete: false,
ValueHash: util.ComputeSHA256([]byte{}),
},
},
}

pvtWrites := &kvrwset.KVRWSet{
Writes: []*kvrwset.KVWrite{
{
Key: "pvtKey1",
IsDelete: false,
},
{
Key: "pvtKey2",
IsDelete: false,
},
},
}

pubWritesBytes, err := proto.Marshal(pubWrites)
require.NoError(t, err)

hashedWritesBytes, err := proto.Marshal(hashedWrites)
require.NoError(t, err)

pvtWritesBytes, err := proto.Marshal(pvtWrites)
require.NoError(t, err)

pubRwset := &rwset.TxReadWriteSet{
DataModel: rwset.TxReadWriteSet_KV,
NsRwset: []*rwset.NsReadWriteSet{
{
Namespace: "cc1",
Rwset: pubWritesBytes,
CollectionHashedRwset: []*rwset.CollectionHashedReadWriteSet{
{
CollectionName: "coll1",
HashedRwset: hashedWritesBytes,
PvtRwsetHash: util.ComputeSHA256(pvtWritesBytes),
},
},
},
},
}
pubRwsetBytes, err := proto.Marshal(pubRwset)
require.NoError(t, err)
envelope, err := constructTransaction("txid2", pubRwsetBytes)
require.NoError(t, err)

txAndPvtdata := &txAndPvtdata{
Txid: "txid2",
Envelope: envelope,
Pvtws: &rwset.TxPvtReadWriteSet{
DataModel: rwset.TxReadWriteSet_KV,
NsPvtRwset: []*rwset.NsPvtReadWriteSet{
{
Namespace: "cc1",
CollectionPvtRwset: []*rwset.CollectionPvtReadWriteSet{
{
CollectionName: "coll1",
Rwset: pvtWritesBytes,
},
},
},
},
},
}

testLedger.submitHandCraftedTx(txAndPvtdata)
testLedger.cutBlockAndCommitWithPvtdata()

testLedger.verifyTxValidationCode("txid2", peer.TxValidationCode_VALID)
testLedger.verifyPubState("cc1", "pubKey1", "")
testLedger.verifyPvtdataHashState("cc1", "coll1", "pvtKey1", nil)
testLedger.verifyPvtdataHashState("cc1", "coll1", "pvtKey2", nil)
testLedger.verifyPvtState("cc1", "coll1", "pvtKey1", "")
testLedger.verifyPvtState("cc1", "coll1", "pvtKey2", "")
testLedger.verifyHistory("cc1", "pubKey1", []string{"pubValue1", ""})
}
27 changes: 27 additions & 0 deletions core/ledger/kvledger/tests/verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/hyperledger/fabric/core/ledger"
lgrutil "github.com/hyperledger/fabric/core/ledger/util"
"github.com/hyperledger/fabric/protos/common"
"github.com/hyperledger/fabric/protos/ledger/queryresult"
"github.com/hyperledger/fabric/protos/ledger/rwset/kvrwset"
protopeer "github.com/hyperledger/fabric/protos/peer"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -52,6 +53,15 @@ func (v *verifier) verifyPubState(ns, key string, expectedVal string) {
v.assert.Equal(expectedValBytes, committedVal)
}

func (v *verifier) verifyPvtdataHashState(ns, coll, key string, expectedValHash []byte) {
qe, err := v.lgr.NewQueryExecutor()
v.assert.NoError(err)
defer qe.Done()
committedValHash, err := qe.GetPrivateDataHash(ns, coll, key)
v.assert.NoError(err)
v.assert.Equal(expectedValHash, committedValHash)
}

func (v *verifier) verifyPvtState(ns, coll, key string, expectedVal string) {
qe, err := v.lgr.NewQueryExecutor()
v.assert.NoError(err)
Expand Down Expand Up @@ -117,6 +127,23 @@ func (v *verifier) verifyTxValidationCode(txid string, expectedCode protopeer.Tx
v.assert.Equal(int32(expectedCode), tran.ValidationCode)
}

func (v *verifier) verifyHistory(ns, key string, expectedVals []string) {
hqe, err := v.lgr.NewHistoryQueryExecutor()
v.assert.NoError(err)
itr, err := hqe.GetHistoryForKey(ns, key)
v.assert.NoError(err)
historyValues := []string{}
for {
result, err := itr.Next()
v.assert.NoError(err)
if result == nil {
break
}
historyValues = append(historyValues, string(result.(*queryresult.KeyModification).GetValue()))
}
v.assert.Equal(expectedVals, historyValues)
}

func (v *verifier) verifyCommitHashExists() {
bcInfo, err := v.lgr.GetBlockchainInfo()
v.assert.NoError(err)
Expand Down
69 changes: 69 additions & 0 deletions core/ledger/kvledger/txmgmt/rwsetutil/rwset_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/hyperledger/fabric/protos/ledger/rwset"
"github.com/hyperledger/fabric/protos/ledger/rwset/kvrwset"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestMain(m *testing.M) {
Expand Down Expand Up @@ -357,3 +358,71 @@ func serializeTestProtoMsg(t *testing.T, protoMsg proto.Message) []byte {
assert.NoError(t, err)
return msgBytes
}

func TestNilOrZeroLengthByteArrayValueConvertedToDelete(t *testing.T) {
t.Run("public_writeset", func(t *testing.T) {
rwsetBuilder := NewRWSetBuilder()
rwsetBuilder.AddToWriteSet("ns", "key1", nil)
rwsetBuilder.AddToWriteSet("ns", "key2", []byte{})

simulationResults, err := rwsetBuilder.GetTxSimulationResults()
require.NoError(t, err)
pubRWSet := &kvrwset.KVRWSet{}
require.NoError(
t,
proto.Unmarshal(simulationResults.PubSimulationResults.NsRwset[0].Rwset, pubRWSet),
)
require.True(t, proto.Equal(
&kvrwset.KVRWSet{
Writes: []*kvrwset.KVWrite{
{Key: "key1", IsDelete: true},
{Key: "key2", IsDelete: true},
},
},
pubRWSet,
))
})

t.Run("pvtdata_and_hashes_writesets", func(t *testing.T) {
rwsetBuilder := NewRWSetBuilder()
rwsetBuilder.AddToPvtAndHashedWriteSet("ns", "coll", "key1", nil)
rwsetBuilder.AddToPvtAndHashedWriteSet("ns", "coll", "key2", []byte{})

simulationResults, err := rwsetBuilder.GetTxSimulationResults()
require.NoError(t, err)

t.Run("hashed_writeset", func(t *testing.T) {
hashedRWSet := &kvrwset.HashedRWSet{}
require.NoError(
t,
proto.Unmarshal(simulationResults.PubSimulationResults.NsRwset[0].CollectionHashedRwset[0].HashedRwset, hashedRWSet),
)
require.True(t, proto.Equal(
&kvrwset.HashedRWSet{
HashedWrites: []*kvrwset.KVWriteHash{
{KeyHash: util.ComputeStringHash("key1"), IsDelete: true},
{KeyHash: util.ComputeStringHash("key2"), IsDelete: true},
},
},
hashedRWSet,
))
})

t.Run("pvtdata_writeset", func(t *testing.T) {
pvtWSet := &kvrwset.KVRWSet{}
require.NoError(
t,
proto.Unmarshal(simulationResults.PvtSimulationResults.NsPvtRwset[0].CollectionPvtRwset[0].Rwset, pvtWSet),
)
require.True(t, proto.Equal(
&kvrwset.KVRWSet{
Writes: []*kvrwset.KVWrite{
{Key: "key1", IsDelete: true},
{Key: "key2", IsDelete: true},
},
},
pvtWSet,
))
})
})
}
Loading

0 comments on commit 3813607

Please sign in to comment.