From d61a449364d9230e2e7a93a2c9330194d590daba Mon Sep 17 00:00:00 2001 From: Baptiste Lemaire Date: Thu, 10 Jun 2021 12:54:13 -0700 Subject: [PATCH] Fixed manifest_dump issues when printing keys and values containing null characters (#8378) Summary: Changed fprintf function to fputc in ApplyVersionEdit, and replaced null characters with whitespaces. Added unit test in ldb_test.py - verifies that manifest_dump --verbose output is correct when keys and values containing null characters are inserted. Pull Request resolved: https://github.com/facebook/rocksdb/pull/8378 Reviewed By: pdillinger Differential Revision: D29034584 Pulled By: bjlemaire fbshipit-source-id: 50833687a8a5f726e247c38457eadc3e6dbab862 --- db/version_edit_handler.cc | 5 ++++- db/version_edit_handler.h | 8 ++++++-- tools/ldb_test.py | 25 +++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/db/version_edit_handler.cc b/db/version_edit_handler.cc index 807cac9d0..7a2996a59 100644 --- a/db/version_edit_handler.cc +++ b/db/version_edit_handler.cc @@ -909,7 +909,10 @@ void DumpManifestHandler::CheckIterationResult(const log::Reader& reader, fprintf(stdout, "comparator: %s\n", cfd->user_comparator()->Name()); } assert(cfd->current()); - fprintf(stdout, "%s \n", cfd->current()->DebugString(hex_).c_str()); + + // Print out DebugStrings. Can include non-terminating null characters. + fwrite(cfd->current()->DebugString(hex_).data(), sizeof(char), + cfd->current()->DebugString(hex_).size(), stdout); } fprintf(stdout, "next_file_number %" PRIu64 " last_sequence %" PRIu64 diff --git a/db/version_edit_handler.h b/db/version_edit_handler.h index 3fd16f413..665e0f0d4 100644 --- a/db/version_edit_handler.h +++ b/db/version_edit_handler.h @@ -285,9 +285,13 @@ class DumpManifestHandler : public VersionEditHandler { Status ApplyVersionEdit(VersionEdit& edit, ColumnFamilyData** cfd) override { // Write out each individual edit if (verbose_ && !json_) { - fprintf(stdout, "%s\n", edit.DebugString(hex_).c_str()); + // Print out DebugStrings. Can include non-terminating null characters. + fwrite(edit.DebugString(hex_).data(), sizeof(char), + edit.DebugString(hex_).size(), stdout); } else if (json_) { - fprintf(stdout, "%s\n", edit.DebugJSON(count_, hex_).c_str()); + // Print out DebugStrings. Can include non-terminating null characters. + fwrite(edit.DebugString(hex_).data(), sizeof(char), + edit.DebugString(hex_).size(), stdout); } ++count_; return VersionEditHandler::ApplyVersionEdit(edit, cfd); diff --git a/tools/ldb_test.py b/tools/ldb_test.py index 6587ad6c3..f329699a2 100644 --- a/tools/ldb_test.py +++ b/tools/ldb_test.py @@ -473,6 +473,31 @@ class LDBTestCase(unittest.TestCase): expected_pattern, unexpected=False, isPattern=True) + # Check if null characters doesn't infer with output format. + self.assertRunOK("put a1 b1", "OK") + self.assertRunOK("put a2 b2", "OK") + self.assertRunOK("put --hex 0x12000DA0 0x80C0000B", "OK") + self.assertRunOK("put --hex 0x7200004f 0x80000004", "OK") + self.assertRunOK("put --hex 0xa000000a 0xf000000f", "OK") + self.assertRunOK("put a3 b3", "OK") + self.assertRunOK("put a4 b4", "OK") + + # Verifies that all "levels" are printed out. + # There should be 66 mentions of levels. + expected_verbose_output = re.compile("matched") + # Test manifest_dump verbose and verify that key 0x7200004f + # is present. Note that we are forced to use grep here because + # an output with a non-terminating null character in it isn't piped + # correctly through the Python subprocess object. + # Also note that 0x72=r and 0x4f=O, hence the regex \'r.{2}O\' + # (we cannot use null character in the subprocess input either, + # so we have to use '.{2}') + cmd_verbose = "manifest_dump --verbose --db=%s | grep -aq $'\'r.{2}O\'' && echo 'matched' || echo 'not matched'" %dbPath + + self.assertRunOKFull(cmd_verbose , expected_verbose_output, + unexpected=False, isPattern=True) + + def testGetProperty(self): print("Running testGetProperty...") dbPath = os.path.join(self.TMP_DIR, self.DB_NAME)