CBG-4459 validate cv when passed in

- validate that sourceID is base64, requires some test changes
- pass back better error messages if hlv was not found

db/blip_handler.go

@@ -1076,14 +1076,21 @@ func (bh *blipHandler) processRev(rq *blip.Message, stats *processRevStats) (err
 			history = append(history, strings.Split(historyStr, ",")...)
 		}
 	} else {
-		versionVectorStr := rev
-		if historyStr != "" {
-			versionVectorStr += ";" + historyStr
+		cv, err := ParseVersion(rev)
+		if err != nil {
+			base.InfofCtx(bh.loggingCtx, base.KeySync, "Error parsing hlv while processing rev for doc %v. Rev:%v Error: %v", base.UD(docID), rev, err)
+			return base.HTTPErrorf(http.StatusUnprocessableEntity, "error extracting hlv from blip message, rev=%q error: %v", rev, err)
+		}
+		incomingHLV = NewHybridLogicalVector()
+		err = incomingHLV.AddVersion(cv)
+		if err != nil {
+			return err
 		}
-		incomingHLV, legacyRevList, err = ExtractHLVFromBlipMessage(versionVectorStr)
+
+		legacyRevList, err = addPVMVToHLV(strings.Split(historyStr, ";"), incomingHLV)
 		if err != nil {
-			base.InfofCtx(bh.loggingCtx, base.KeySync, "Error parsing hlv while processing rev for doc %v.  HLV:%v Error: %v", base.UD(docID), versionVectorStr, err)
-			return base.HTTPErrorf(http.StatusUnprocessableEntity, "error extracting hlv from blip message")
+			base.InfofCtx(bh.loggingCtx, base.KeySync, "Error parsing hlv while processing rev message for doc %v. Rev:%v History:%q Error: %v", base.UD(docID), rev, history, err)
+			return base.HTTPErrorf(http.StatusUnprocessableEntity, "error extracting hlv from blip message history=%q, error: %v", historyStr, err)
 		}
 		newDoc.HLV = incomingHLV
 	}

db/database_test.go

@@ -363,7 +363,7 @@ func TestCheckProposedVersion(t *testing.T) {
 			// proposed version is from a source not present in server HLV, and previousVersion matches server cv
 			//  Not a conflict, due to previousVersion match
 			name:            "new version,new source,prev matches",
-			newVersion:      Version{"other", incrementCas(cvValue, 100)},
+			newVersion:      Version{"otherq", incrementCas(cvValue, 100)},
 			previousVersion: &currentVersion,
 			expectedStatus:  ProposedRev_OK,
 			expectedRev:     "",
@@ -374,7 +374,7 @@ func TestCheckProposedVersion(t *testing.T) {
 			//  version and cv
 			name:            "new version,prev mismatch,new matches cv",
 			newVersion:      Version{cvSource, incrementCas(cvValue, 100)},
-			previousVersion: &Version{"other", incrementCas(cvValue, 50)},
+			previousVersion: &Version{"otherq", incrementCas(cvValue, 50)},
 			expectedStatus:  ProposedRev_OK,
 			expectedRev:     "",
 		},
@@ -388,16 +388,16 @@ func TestCheckProposedVersion(t *testing.T) {
 		{
 			// conflict - previous version is older than CV
 			name:            "conflict,same source,server updated",
-			newVersion:      Version{"other", incrementCas(cvValue, -100)},
+			newVersion:      Version{"otherq", incrementCas(cvValue, -100)},
 			previousVersion: &Version{cvSource, incrementCas(cvValue, -50)},
 			expectedStatus:  ProposedRev_Conflict,
 			expectedRev:     Version{cvSource, cvValue}.String(),
 		},
 		{
 			// conflict - previous version is older than CV
 			name:            "conflict,new source,server updated",
-			newVersion:      Version{"other", incrementCas(cvValue, 100)},
-			previousVersion: &Version{"other", incrementCas(cvValue, -50)},
+			newVersion:      Version{"otherq", incrementCas(cvValue, 100)},
+			previousVersion: &Version{"otherq", incrementCas(cvValue, -50)},
 			expectedStatus:  ProposedRev_Conflict,
 			expectedRev:     Version{cvSource, cvValue}.String(),
 		},
@@ -423,15 +423,15 @@ func TestCheckProposedVersion(t *testing.T) {
 
 	// New doc cases - standard insert
 	t.Run("new doc", func(t *testing.T) {
-		newVersion := Version{"other", 100}.String()
+		newVersion := Version{"otherq", 100}.String()
 		status, _ := collection.CheckProposedVersion(ctx, "doc2", newVersion, "")
 		assert.Equal(t, ProposedRev_OK_IsNew, status)
 	})
 
 	// New doc cases - insert with prev version (previous version purged from SGW)
 	t.Run("new doc with prev version", func(t *testing.T) {
-		newVersion := Version{"other", 100}.String()
-		prevVersion := Version{"another other", 50}.String()
+		newVersion := Version{"otherq", 100}.String()
+		prevVersion := Version{"2otherq", 50}.String()
 		status, _ := collection.CheckProposedVersion(ctx, "doc2", newVersion, prevVersion)
 		assert.Equal(t, ProposedRev_OK_IsNew, status)
 	})

db/hybrid_logical_vector.go

@@ -127,6 +127,10 @@ func ParseVersion(versionString string) (version Version, err error) {
 	if !found {
 		return version, fmt.Errorf("Malformed version string %s, delimiter not found", versionString)
 	}
+	_, err = base64.RawStdEncoding.DecodeString(sourceBase64)
+	if err != nil {
+		return version, fmt.Errorf("Malformed version string %s, sourceID=%q is not base64 encoded", versionString, sourceBase64)
+	}
 	version.SourceID = sourceBase64
 	// remove any leading whitespace, this should be addressed in CBG-3662
 	if len(timestampString) > 0 && timestampString[0] == ' ' {
@@ -447,19 +451,22 @@ func appendRevocationMacroExpansions(currentSpec []sgbucket.MacroExpansionSpec,
 // Function will return list of revIDs if legacy rev ID was found in the HLV history section (PV)
 // TODO: CBG-3662 - Optimise once we've settled on and tested the format with CBL
 func ExtractHLVFromBlipMessage(versionVectorStr string) (*HybridLogicalVector, []string, error) {
-	hlv := &HybridLogicalVector{}
+	hlv := NewHybridLogicalVector()
 
 	vectorFields := strings.Split(versionVectorStr, ";")
 	vectorLength := len(vectorFields)
-	if (vectorLength == 1 && vectorFields[0] == "") || vectorLength > 3 {
-		return &HybridLogicalVector{}, nil, fmt.Errorf("invalid hlv in changes message received")
+	if vectorLength == 1 && vectorFields[0] == "" {
+		return &HybridLogicalVector{}, nil, fmt.Errorf("Empty HLV of length 1: %v", versionVectorStr)
+	}
+	if vectorLength > 3 {
+		return &HybridLogicalVector{}, nil, fmt.Errorf("hlv contains more than 3 parts for cv;mv;pv: %v", versionVectorStr)
 	}
 
 	// add current version (should always be present)
 	cvStr := vectorFields[0]
 	version := strings.Split(cvStr, "@")
 	if len(version) < 2 {
-		return &HybridLogicalVector{}, nil, fmt.Errorf("invalid version in changes message received")
+		return &HybridLogicalVector{}, nil, fmt.Errorf("cv does not contain @: cv:%q hlv:%q", cvStr, versionVectorStr)
 	}
 
 	vrs, err := strconv.ParseUint(version[0], 16, 64)
@@ -470,44 +477,58 @@ func ExtractHLVFromBlipMessage(versionVectorStr string) (*HybridLogicalVector, [
 	if err != nil {
 		return &HybridLogicalVector{}, nil, err
 	}
+	if len(vectorFields) == 1 {
+		return hlv, nil, nil
+	}
+	legacyRev, err := addPVMVToHLV(vectorFields[1:len(vectorFields)], hlv)
+	if err != nil {
+		return &HybridLogicalVector{}, nil, err
+	}
+	return hlv, legacyRev, nil
+}
 
-	switch vectorLength {
+// addPVMVToHLV will take a HLV and add the previous and merge versions to it from the input string. This is a partial parsing from ExtractHLVFromBlipMessage
+//  1. empty string
+//  2. pv:  		pv
+//  2. pv, and mv: 	mv;pv
+func addPVMVToHLV(vectorFields []string, hlv *HybridLogicalVector) (legacyRev []string, err error) {
+	switch len(vectorFields) {
+	case 0:
+		return []string{}, nil
 	case 1:
-		// cv only
-		return hlv, nil, nil
-	case 2:
-		// only cv and pv present
-		sourceVersionListPV, legacyRev, err := parseVectorValues(vectorFields[1])
+		// if strings.Split("", ";") was passed in as vectorFields, this will result in a one element array with an empty string
+		if vectorFields[0] == "" {
+			return nil, nil
+		}
+		// pv present
+		sourceVersionListPV, legacyRev, err := parseVectorValues(vectorFields[0])
 		if err != nil {
-			return &HybridLogicalVector{}, nil, err
+			return nil, err
 		}
-		hlv.PreviousVersions = make(HLVVersions)
 		for _, v := range sourceVersionListPV {
 			hlv.PreviousVersions[v.SourceID] = v.Value
 		}
-		return hlv, legacyRev, nil
-	case 3:
+		return legacyRev, nil
+	case 2:
 		// cv, mv and pv present
-		sourceVersionListPV, legacyRev, err := parseVectorValues(vectorFields[2])
-		hlv.PreviousVersions = make(HLVVersions)
+		sourceVersionListPV, legacyRev, err := parseVectorValues(vectorFields[1])
 		if err != nil {
-			return &HybridLogicalVector{}, nil, err
+			return nil, err
 		}
 		for _, pv := range sourceVersionListPV {
 			hlv.PreviousVersions[pv.SourceID] = pv.Value
 		}
 
-		sourceVersionListMV, _, err := parseVectorValues(vectorFields[1])
-		hlv.MergeVersions = make(HLVVersions)
+		sourceVersionListMV, _, err := parseVectorValues(vectorFields[0])
 		if err != nil {
-			return &HybridLogicalVector{}, nil, err
+			return nil, err
 		}
 		for _, mv := range sourceVersionListMV {
 			hlv.MergeVersions[mv.SourceID] = mv.Value
 		}
-		return hlv, legacyRev, nil
+		return legacyRev, nil
 	default:
-		return &HybridLogicalVector{}, nil, fmt.Errorf("invalid hlv in changes message received")
+		return nil, fmt.Errorf("invalid mv/pv, contains more than 2 extries %v", vectorFields)
 	}
 }
 
@@ -561,7 +582,7 @@ func isLegacyRev(rev string) bool {
 
 // Helper functions for version source and value encoding
 func EncodeSource(source string) string {
-	return base64.StdEncoding.EncodeToString([]byte(source))
+	return base64.RawStdEncoding.EncodeToString([]byte(source))
 }
 
 // EncodeValueStr converts a simplified number ("1") to a hex-encoded string

db/hybrid_logical_vector_test.go

@@ -164,7 +164,7 @@ func createHLVForTest(tb *testing.T, inputList []string) *HybridLogicalVector {
 
 	// first element will be current version and source pair
 	currentVersionPair := strings.Split(inputList[0], "@")
-	hlvOutput.SourceID = base64.StdEncoding.EncodeToString([]byte(currentVersionPair[0]))
+	hlvOutput.SourceID = base64.RawStdEncoding.EncodeToString([]byte(currentVersionPair[0]))
 	value, err := strconv.ParseUint(currentVersionPair[1], 10, 64)
 	require.NoError(tb, err)
 	hlvOutput.Version = value
@@ -494,18 +494,18 @@ func TestHLVMapToCBLString(t *testing.T) {
 			name: "Both PV and mv",
 			inputHLV: []string{"cb06dc003846116d9b66d2ab23887a96@123456", "YZvBpEaztom9z5V/hDoeIw@1628620455135215600", "m_NqiIe0LekFPLeX4JvTO6Iw@1628620455139868700",
 				"m_LhRPsa7CpjEvP5zeXTXEBA@1628620455147864000"},
-			expectedStr: "169a05acd68c001c@TnFpSWUwTGVrRlBMZVg0SnZUTzZJdw==,169a05acd705ffc0@TGhSUHNhN0NwakV2UDV6ZVhUWEVCQQ==;169a05acd644fff0@WVp2QnBFYXp0b205ejVWL2hEb2VJdw==",
+			expectedStr: "169a05acd68c001c@TnFpSWUwTGVrRlBMZVg0SnZUTzZJdw,169a05acd705ffc0@TGhSUHNhN0NwakV2UDV6ZVhUWEVCQQ==;169a05acd644fff0@WVp2QnBFYXp0b205ejVWL2hEb2VJdw",
 			both:        true,
 		},
 		{
 			name:        "Just PV",
 			inputHLV:    []string{"cb06dc003846116d9b66d2ab23887a96@123456", "YZvBpEaztom9z5V/hDoeIw@1628620455135215600"},
-			expectedStr: "169a05acd644fff0@WVp2QnBFYXp0b205ejVWL2hEb2VJdw==",
+			expectedStr: "169a05acd644fff0@WVp2QnBFYXp0b205ejVWL2hEb2VJdw",
 		},
 		{
 			name:        "Just MV",
 			inputHLV:    []string{"cb06dc003846116d9b66d2ab23887a96@123456", "m_NqiIe0LekFPLeX4JvTO6Iw@1628620455139868700"},
-			expectedStr: "169a05acd68c001c@TnFpSWUwTGVrRlBMZVg0SnZUTzZJdw==",
+			expectedStr: "169a05acd68c001c@TnFpSWUwTGVrRlBMZVg0SnZUTzZJdw",
 		},
 	}
 	for _, test := range testCases {
@@ -532,18 +532,31 @@ func TestHLVMapToCBLString(t *testing.T) {
 //   - Test hlv string that is empty
 //   - Assert that ExtractHLVFromBlipMessage will return error in both cases
 func TestInvalidHLVInBlipMessageForm(t *testing.T) {
-	hlvStr := "25@def; 22@def,21@eff; 20@abc,18@hij; 222@hiowdwdew, 5555@dhsajidfgd"
-
-	hlv, _, err := ExtractHLVFromBlipMessage(hlvStr)
-	require.Error(t, err)
-	assert.ErrorContains(t, err, "invalid hlv in changes message received")
-	assert.Equal(t, &HybridLogicalVector{}, hlv)
-
-	hlvStr = ""
-	hlv, _, err = ExtractHLVFromBlipMessage(hlvStr)
-	require.Error(t, err)
-	assert.ErrorContains(t, err, "invalid hlv in changes message received")
-	assert.Equal(t, &HybridLogicalVector{}, hlv)
+	testCases := []struct {
+		name   string
+		hlv    string
+		errMsg string
+	}{
+		{
+			name:   "Too many parts",
+			hlv:    "25@def; 22@def,21@eff; 20@abc,18@hij; 222@hiowdwdew, 5555@dhsajidfgd",
+			errMsg: "hlv contains more than 3 parts",
+		},
+		{
+			name:   "Empty HLV",
+			hlv:    "",
+			errMsg: "Empty HLV",
+		},
+	}
+	for _, test := range testCases {
+		t.Run(test.name, func(t *testing.T) {
+
+			hlv, _, err := ExtractHLVFromBlipMessage(test.hlv)
+			require.Error(t, err)
+			assert.ErrorContains(t, err, test.errMsg)
+			assert.Equal(t, &HybridLogicalVector{}, hlv)
+		})
+	}
 }
 
 var extractHLVFromBlipMsgBMarkCases = []struct {
@@ -616,7 +629,7 @@ func TestExtractHLVFromChangesMessage(t *testing.T) {
 			//       that may represent a base64 encoding
 			base64EncodedHlvString := EncodeTestHistory(test.hlvString)
 			hlv, _, err := ExtractHLVFromBlipMessage(base64EncodedHlvString)
-			require.NoError(t, err)
+			require.NoError(t, err, "hlv=%q", base64EncodedHlvString)
 
 			assert.Equal(t, expectedVector.SourceID, hlv.SourceID)
 			assert.Equal(t, expectedVector.Version, hlv.Version)

rest/utilities_testing_blip_client.go

@@ -1362,8 +1362,9 @@ func (btcc *BlipTesterCollectionClient) StartPushWithOpts(opts BlipTesterPushOpt
 								// conflict on write of rev - OK to ignore and let pull replication resolve
 								btcc.TB().Logf("conflict on write of rev %s / %v", change.docID, change.version)
 							} else {
-								btcc.TB().Errorf("error response from rev: %s %s", revResp.Properties["Error-Domain"], revResp.Properties["Error-Code"])
-								return
+								body, err := revResp.Body()
+								require.NoError(btcc.TB(), err)
+								require.FailNow(btcc.TB(), fmt.Sprintf("error response from rev: %s %s: %s, inputProperties: %+v", revResp.Properties["Error-Domain"], revResp.Properties["Error-Code"], body, revRequest.Properties))
 							}
 						}
 						base.DebugfCtx(ctx, base.KeySGTest, "peer acked rev %s / %v", change.docID, change.version)
@@ -1560,7 +1561,8 @@ func (btc *BlipTesterCollectionClient) upsertDoc(docID string, parentVersion *Do
 	var docVersion DocVersion
 	if btc.UseHLV() {
 		// TODO: CBG-4440 Construct a HLC for Value - UnixNano is not accurate enough on Windows to generate unique values, and seq is not comparable across clients.
-		newVersion := db.Version{SourceID: fmt.Sprintf("btc-%d", btc.parent.id), Value: uint64(time.Now().UnixNano())}
+		sourceID := base64.RawStdEncoding.EncodeToString([]byte(fmt.Sprintf("btc-%d", btc.parent.id)))
+		newVersion := db.Version{SourceID: sourceID, Value: uint64(time.Now().UnixNano())}
 		require.NoError(btc.TB(), hlv.AddVersion(newVersion))
 		docVersion = DocVersion{CV: *hlv.ExtractCurrentVersionFromHLV()}
 	} else {

topologytest/couchbase_lite_mock_peer_test.go

@@ -10,6 +10,7 @@ package topologytest
 
 import (
 	"context"
+	"encoding/base64"
 	"fmt"
 	"testing"
 
@@ -209,7 +210,7 @@ func (p *CouchbaseLiteMockPeer) CreateReplication(peer Peer, config PeerReplicat
 
 // SourceID returns the source ID for the peer used in <val>@<sourceID>.
 func (p *CouchbaseLiteMockPeer) SourceID() string {
-	return p.name
+	return base64.RawStdEncoding.EncodeToString([]byte(p.name))
 }
 
 // Context returns the context for the peer.