Skip to content

Commit

Permalink
MB-44534: SetWithMeta fixes the datatype for empty value with dt:xattr
Browse files Browse the repository at this point in the history 
Datatype Xattr is invalid for empty payloads. The current behaviour that
the document is successfully stored and persisted as it is, while we
want to sanitize it.

Change-Id: Ib6181b822790db0865b51446a2fee831b1aa6c72
Reviewed-on: http://review.couchbase.org/c/kv_engine/+/146827
Reviewed-by: Trond Norbye <trond.norbye@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
Well-Formed: Build Bot <build@couchbase.com>
  • Loading branch information
@paolococchi
paolococchi committed Feb 26, 2021
1 parent bca68e4 commit 0f1701d
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 43 deletions.
94 changes: 51 additions & 43 deletions engines/ep/src/ep_engine.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5415,57 +5415,65 @@ ENGINE_ERROR_CODE EventuallyPersistentEngine::setWithMeta(
cb::const_byte_buffer inflatedValue = value;
protocol_binary_datatype_t inflatedDatatype = datatype;

if (mcbp::datatype::is_snappy(datatype)) {
if (!cb::compression::inflate(cb::compression::Algorithm::Snappy,
payload, uncompressedValue)) {
setErrorContext(cookie, "Failed to inflate document");
return ENGINE_EINVAL;
}
if (value.empty()) {
finalDatatype = PROTOCOL_BINARY_RAW_BYTES;
} else {
if (mcbp::datatype::is_snappy(datatype)) {
if (!cb::compression::inflate(cb::compression::Algorithm::Snappy,
payload,
uncompressedValue)) {
setErrorContext(cookie, "Failed to inflate document");
return ENGINE_EINVAL;
}

inflatedValue = uncompressedValue;
inflatedDatatype &= ~PROTOCOL_BINARY_DATATYPE_SNAPPY;
inflatedValue = uncompressedValue;
inflatedDatatype &= ~PROTOCOL_BINARY_DATATYPE_SNAPPY;

if (compressionMode == BucketCompressionMode::Off ||
uncompressedValue.size() < value.size()) {
// If the inflated version version is smaller than the compressed
// version we should keep it inflated
finalValue = uncompressedValue;
finalDatatype &= ~PROTOCOL_BINARY_DATATYPE_SNAPPY;
if (compressionMode == BucketCompressionMode::Off ||
uncompressedValue.size() < value.size()) {
// If the inflated version version is smaller than the
// compressed version we should keep it inflated
finalValue = uncompressedValue;
finalDatatype &= ~PROTOCOL_BINARY_DATATYPE_SNAPPY;
}
}
}

size_t system_xattr_size = 0;
if (mcbp::datatype::is_xattr(datatype)) {
// the validator ensured that the xattr was valid
cb::const_char_buffer xattr;
xattr = {reinterpret_cast<const char*>(inflatedValue.data()),
inflatedValue.size()};
system_xattr_size =
cb::xattr::get_system_xattr_size(inflatedDatatype, xattr);
if (system_xattr_size > cb::limits::PrivilegedBytes) {
setErrorContext(
cookie,
"System XATTR (" + std::to_string(system_xattr_size) +
") exceeds the max limit for system xattrs: " +
std::to_string(cb::limits::PrivilegedBytes));
return ENGINE_EINVAL;
size_t system_xattr_size = 0;
if (mcbp::datatype::is_xattr(datatype)) {
// the validator ensured that the xattr was valid
cb::const_char_buffer xattr;
xattr = {reinterpret_cast<const char*>(inflatedValue.data()),
inflatedValue.size()};
system_xattr_size =
cb::xattr::get_system_xattr_size(inflatedDatatype, xattr);
if (system_xattr_size > cb::limits::PrivilegedBytes) {
setErrorContext(
cookie,
"System XATTR (" + std::to_string(system_xattr_size) +
") exceeds the max limit for system "
"xattrs: " +
std::to_string(cb::limits::PrivilegedBytes));
return ENGINE_EINVAL;
}
}
}

const auto valuesize = inflatedValue.size();
if ((valuesize - system_xattr_size) > maxItemSize) {
EP_LOG_WARN(
"Item value size {} for setWithMeta is bigger than the max "
"size {} allowed!!!",
inflatedValue.size(),
maxItemSize);
const auto valuesize = inflatedValue.size();
if ((valuesize - system_xattr_size) > maxItemSize) {
EP_LOG_WARN(
"Item value size {} for setWithMeta is bigger than the max "
"size {} allowed!!!",
inflatedValue.size(),
maxItemSize);

return ENGINE_E2BIG;
}
return ENGINE_E2BIG;
}

finalDatatype = checkForDatatypeJson(cookie, finalDatatype,
mcbp::datatype::is_snappy(datatype) ?
uncompressedValue : payload);
finalDatatype = checkForDatatypeJson(cookie,
finalDatatype,
mcbp::datatype::is_snappy(datatype)
? uncompressedValue
: payload);
}

auto item = std::make_unique<Item>(key,
itemMeta.flags,
Expand Down
53 changes: 53 additions & 0 deletions engines/ep/tests/module_tests/evp_store_with_meta.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,59 @@ TEST_F(WithMetaTest, basicAdd) {
ENGINE_SUCCESS); // can still get the key
}

/**
* The test verifies that SetWithMeta sanitizes invalid payloads like
* {datatype:xattr, value-size=0} by resetting datatype:raw
*/
TEST_F(WithMetaTest, SetWithMetaXattrWithEmptyPayload) {
mock_set_datatype_support(cookie, PROTOCOL_BINARY_DATATYPE_JSON);

const auto& vb = store->getVBucket(vbid);
ASSERT_EQ(0, vb->getHighSeqno());

const std::string key = "key";
const std::string value; // Empty value
const auto datatype = PROTOCOL_BINARY_DATATYPE_XATTR;
ItemMetaData meta{1 /*cas*/, 1 /*revSeqno*/, 0 /*flags*/, 0 /*expiry*/};
const std::vector<char> extMeta = {};
const auto packet = buildWithMetaPacket(cb::mcbp::ClientOpcode::SetWithMeta,
datatype,
vbid,
0 /*opaque*/,
0 /*cas*/,
meta,
key,
value,
extMeta,
0 /*options*/);
const auto* req = reinterpret_cast<const cb::mcbp::Request*>(packet.data());
EXPECT_EQ(ENGINE_SUCCESS, engine->setWithMeta(cookie, *req, addResponse));

// Check in memory
const auto res = store->get({key, DocKeyEncodesCollectionId::No},
vbid,
cookie,
get_options_t::NONE);
EXPECT_EQ(ENGINE_SUCCESS, res.getStatus());
EXPECT_EQ(1, res.item->getBySeqno());
EXPECT_FALSE(res.item->isDeleted());
EXPECT_EQ(PROTOCOL_BINARY_RAW_BYTES, res.item->getDataType());
EXPECT_NE(nullptr, res.item->getData());
EXPECT_EQ(0, res.item->getNBytes());

// Check on disk
flush_vbucket_to_disk(vbid, 1 /*expectedNumFlushed*/);
auto* kvstore = store->getRWUnderlyingByShard(
store->getVBucket(vbid)->getShard()->getId());
auto gv = kvstore->get(makeDiskDocKey(key), vbid);
EXPECT_EQ(ENGINE_SUCCESS, gv.getStatus());
EXPECT_EQ(1, gv.item->getBySeqno());
EXPECT_FALSE(gv.item->isDeleted());
EXPECT_EQ(PROTOCOL_BINARY_RAW_BYTES, gv.item->getDataType());
EXPECT_TRUE(gv.item->getValue());
EXPECT_EQ(0, gv.item->getNBytes());
}

TEST_P(DelWithMetaTest, basic) {
ItemMetaData itemMeta{0xdeadbeef, 0xf00dcafe, 0xfacefeed, expiry};
// A delete_w_meta against an empty bucket queues a BGFetch (get = ewblock)
Expand Down

0 comments on commit 0f1701d

Please sign in to comment.