Commit 0669cd92 authored by Krishna Kondaka's avatar Krishna Kondaka Committed by Facebook Github Bot

Split validateRecord to allow validating header and data separately

Summary:
Split validateRecord to allow validating header and data separately. This is
useful when the header is used while storing records on storage devices.
Validating header alone before validating the record helps avoiding unnecessary
device reads. It also helps avoiding unnecessary memory allocations while trying to read invalid dataLength bytes present in an invalid header.

Reviewed By: yfeldblum

Differential Revision: D15184763

fbshipit-source-id: e1f008df82cf2d040322b8d62135ea2b5e0dcc0e
parent baa4c665
......@@ -167,19 +167,29 @@ size_t prependHeader(std::unique_ptr<IOBuf>& buf, uint32_t fileId) {
return lengthAndHash.first + headerSize();
}
RecordInfo validateRecord(ByteRange range, uint32_t fileId) {
if (range.size() <= headerSize()) { // records may not be empty
return {0, {}};
bool validateRecordHeader(ByteRange range, uint32_t fileId) {
if (range.size() < headerSize()) { // records may not be empty
return false;
}
const Header* header = reinterpret_cast<const Header*>(range.begin());
range.advance(sizeof(Header));
if (header->magic != Header::kMagic || header->version != 0 ||
header->hashFunction != 0 || header->flags != 0 ||
(fileId != 0 && header->fileId != fileId) ||
header->dataLength > range.size()) {
return {0, {}};
(fileId != 0 && header->fileId != fileId)) {
return false;
}
if (headerHash(*header) != header->headerHash) {
return false;
}
return true;
}
RecordInfo validateRecordData(ByteRange range) {
if (range.size() <= headerSize()) { // records may not be empty
return {0, {}};
}
const Header* header = reinterpret_cast<const Header*>(range.begin());
range.advance(sizeof(Header));
if (header->dataLength > range.size()) {
return {0, {}};
}
range.reset(range.begin(), header->dataLength);
......@@ -189,6 +199,13 @@ RecordInfo validateRecord(ByteRange range, uint32_t fileId) {
return {header->fileId, range};
}
RecordInfo validateRecord(ByteRange range, uint32_t fileId) {
if (!validateRecordHeader(range, fileId)) {
return {0, {}};
}
return validateRecordData(range);
}
RecordInfo
findRecord(ByteRange searchRange, ByteRange wholeRange, uint32_t fileId) {
static const uint32_t magic = Header::kMagic;
......
......@@ -168,7 +168,26 @@ findRecord(ByteRange searchRange, ByteRange wholeRange, uint32_t fileId);
RecordInfo findRecord(ByteRange range, uint32_t fileId);
/**
* Check if there is a valid record at the beginning of range. Returns the
* Check if the Record Header is valid at the beginning of range.
* Useful to check the validity of the header before building the entire record
* in IOBuf. If the record is from storage device (e.g. flash) then, it
* is better to make sure that the header is valid before reading the data
* from the storage device.
* Returns true if valid, false otherwise.
*/
bool validateRecordHeader(ByteRange range, uint32_t fileId);
/**
* Check if there Record Data is valid (to be used after validating the header
* separately)
* Returns the record data (not the header) if the record data is valid,
* ByteRange() otherwise.
*/
RecordInfo validateRecordData(ByteRange range);
/**
* Check if there is a valid record at the beginning of range. This validates
* both record header and data and Returns the
* record data (not the header) if the record is valid, ByteRange() otherwise.
*/
RecordInfo validateRecord(ByteRange range, uint32_t fileId);
......
......@@ -262,6 +262,72 @@ TEST(RecordIOTest, Randomized) {
EXPECT_EQ(records.size(), i);
}
}
TEST(RecordIOTest, validateRecordAPI) {
uint32_t hdrSize = recordio_helpers::headerSize();
std::vector<uint32_t> testSizes = {
0, 1, hdrSize - 1, hdrSize, hdrSize + 1, 2 * hdrSize, 1024};
constexpr uint32_t kTestFileId = 100;
std::mt19937 rnd(FLAGS_random_seed);
for (auto& testSize : testSizes) {
char testChar = testSize % 26 + 'A';
// create a IOBuf of size = testSize
auto buf = folly::IOBuf::create(testSize);
buf->append(testSize);
EXPECT_NE(buf, nullptr);
auto wData = buf->writableData();
if (testSize > hdrSize) {
// if the testSize is more than header size, populate databytes with
// testChar
buf->trimStart(hdrSize);
memset(wData, testChar, buf->length());
recordio_helpers::prependHeader(buf, kTestFileId);
buf->unshare();
buf->coalesce();
wData = buf->writableData();
}
// validate header
auto res = recordio_helpers::validateRecordHeader(
folly::Range<unsigned char*>(wData, buf->length()), kTestFileId);
if (testSize > hdrSize) {
// validation should succeed for buffers larger than header size
EXPECT_TRUE(res);
auto range = folly::Range<unsigned char*>(wData, buf->length());
// make sure data validation also succeeds
auto dataRes = recordio_helpers::validateRecordData(range);
EXPECT_NE(dataRes.fileId, 0);
// do the entire record validation again
auto recRes = recordio_helpers::validateRecord(range, kTestFileId);
EXPECT_NE(recRes.fileId, 0);
std::uniform_int_distribution<uint32_t> dataDist(
0, testSize - hdrSize - 1);
size_t idx = dataDist(rnd);
/* Now corrupt a random byte and expect data validation to fail */
wData[hdrSize + idx] = testChar + 1;
auto newDataRes = recordio_helpers::validateRecordData(range);
EXPECT_EQ(newDataRes.fileId, 0);
/* header validation should still pass */
auto newRes = recordio_helpers::validateRecordHeader(range, kTestFileId);
EXPECT_TRUE(newRes);
/* corrupt a random header byte */
std::uniform_int_distribution<uint32_t> hdrDist(0, hdrSize - 1);
idx = hdrDist(rnd);
wData[idx] = testChar;
/* header validation should fail now */
newRes = recordio_helpers::validateRecordHeader(range, kTestFileId);
EXPECT_FALSE(newRes);
} else {
EXPECT_FALSE(res);
}
}
}
} // namespace test
} // namespace folly
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment