MB-23712: Check for invalid cas before setting value for deleted body 52/76252/7
authorDave Rigby <daver@couchbase.com>
Tue, 4 Apr 2017 13:31:52 +0000 (14:31 +0100)
committerSriram Ganesan <sriram@couchbase.com>
Sat, 8 Apr 2017 01:24:22 +0000 (01:24 +0000)
Before setting the value for a deleted item, check to see if the
incoming cas matches the existing cas

Change-Id: If61e47f0c29ede41778f0f3d817082a83ec8f851
Reviewed-on: http://review.couchbase.org/76252
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Sriram Ganesan <sriram@couchbase.com>
src/vbucket.cc
tests/ep_testsuite_basic.cc

index 27942e9..ee14850 100644 (file)
@@ -1080,19 +1080,28 @@ ENGINE_ERROR_CODE VBucket::deleteItem(const DocKey& key,
          * delete.
          */
         if (v) {
+            if (cas != 0 && cas != v->getCas()) {
+                return ENGINE_KEY_EEXISTS;
+            }
             itm->setRevSeqno(v->getRevSeqno());
             v->setValue(*itm, ht);
         } else {
-            AddStatus rv = addTempStoredValue(hbl, key);
-            if (rv == AddStatus::NoMem) {
-                return ENGINE_ENOMEM;
+            /* retrieve the item, if it is present in disk, for CAS comparison */
+            if (maybeKeyExistsInFilter(key)) {
+                return addTempItemAndBGFetch(
+                                hbl, key, cookie, engine, bgFetchDelay, true);
+            } else {
+                AddStatus rv = addTempStoredValue(hbl, key);
+                if (rv == AddStatus::NoMem) {
+                    return ENGINE_ENOMEM;
+                }
+                v = ht.unlocked_find(key,
+                                     hbl.getBucketNum(),
+                                     WantsDeleted::Yes,
+                                     TrackReference::No);
+                v->setValue(*itm, ht);
+                /* Due to the above setValue() v is no longer a temp stored value*/
             }
-            v = ht.unlocked_find(key,
-                                 hbl.getBucketNum(),
-                                 WantsDeleted::Yes,
-                                 TrackReference::No);
-            v->setValue(*itm, ht);
-            /* Due to the above setValue() v is no longer a temp stored value*/
         }
     }
 
index 8ea9e90..4b399e3 100644 (file)
@@ -1511,6 +1511,15 @@ static enum test_result test_delete_with_value_cas(ENGINE_HANDLE *h,
 
     curr_revseqno = last_meta.revSeqno;
 
+    // Attempt to Delete-with-value using incorrect CAS (should fail)
+    const uint64_t incorrect_CAS = info.cas + 1;
+    checkeq(ENGINE_KEY_EEXISTS,
+            store(h, h1, nullptr, OPERATION_SET, "key2",
+                  "newdeletevaluewithcas", nullptr, incorrect_CAS, 0, 3600,
+                  0x00, DocumentState::Deleted),
+            "Expected KEY_EEXISTS with incorrect CAS");
+
+    // Attempt with correct CAS.
     checkeq(ENGINE_SUCCESS,
             store(h, h1, nullptr, OPERATION_SET, "key2",
                   "newdeletevaluewithcas", nullptr, info.cas, 0, 3600, 0x00,