fix null character in object name triggering segfault

Parsing \n in  lfn_parse_object_name is implemented with

  out->append('\0');

which segfaults when using libstdc++ and g++ version 4.6.3 on Debian
GNU/Linux. It is replaced with

  (*out) += '\0';

to avoid the bugous implicit conversion. There is no append(charT)
method in C++98 or C++11, which means it relies on an implicit
conversion that is bugous. It would be better to rely on the
basic_string& operator+=(charT c); method as defined in ISO 14882-1998
(page 385) thru ISO 14882-2012 (page 640)

A set of tests is added to generate and parse object names. They need
access to the private function lfn_parse_object_name because there is
no convenient protected method to exercise it. The tests contain a
LFNIndex derived class, TestWrapLFNIndex which is made a friend of
LFNIndex to gain access to the private methods.

http://tracker.ceph.com/issues/4594 refs #4594

Signed-off-by: Loic Dachary <loic@dachary.org>

diff --git a/src/os/LFNIndex.cc b/src/os/LFNIndex.cc
index f1ba8c158f0b0b799adf47680b6db49cda2e61c3..62b8f7085e16bc56c168cdac12713e80c6c4376b 100644 (file)
--- a/src/os/LFNIndex.cc
+++ b/src/os/LFNIndex.cc
@@ -912,7 +912,7 @@ static bool append_unescaped(string::const_iterator begin,
       else if (*i == 's')
        out->append("/");
       else if (*i == 'n')
-       out->append('\0');
+       (*out) += '\0';
       else if (*i == 'u')
        out->append("_");
       else

diff --git a/src/os/LFNIndex.h b/src/os/LFNIndex.h
index b3c053588222cca76486145bb388a2bd9f852c3a..b73ff4db268ffcedbf9565dd8d13b78db9ba1007 100644 (file)
--- a/src/os/LFNIndex.h
+++ b/src/os/LFNIndex.h
@@ -577,6 +577,8 @@ private:
     char *hash,                  ///< [out] Hash of filename.
     int len              ///< [in] Size of hash buffer.
     ); ///< @return Error Code, 0 on success.
+
+  friend class TestWrapLFNIndex;
 };
 typedef LFNIndex::IndexedPath IndexedPath;
 

diff --git a/src/test/os/TestLFNIndex.cc b/src/test/os/TestLFNIndex.cc
index 864c88c03b524361e1ef866ce7ddbec37068d925..3666de0ede153baf7fa86d954b75d6c8cc6eb3e7 100644 (file)
--- a/src/test/os/TestLFNIndex.cc
+++ b/src/test/os/TestLFNIndex.cc
@@ -27,11 +27,11 @@
 #include "global/global_init.h"
 #include <gtest/gtest.h>
 
-class TestIndex : public LFNIndex {
+class TestWrapLFNIndex : public LFNIndex {
 public:
-  TestIndex(coll_t collection,
-           const char *base_path,
-           uint32_t index_version) : LFNIndex(collection, base_path, index_version) {}
+  TestWrapLFNIndex(coll_t collection,
+                  const char *base_path,
+                  uint32_t index_version) : LFNIndex(collection, base_path, index_version) {}
 
   virtual uint32_t collection_version() {
     return index_version;
@@ -45,6 +45,14 @@ public:
                     std::tr1::shared_ptr<CollectionIndex> dest
                     ) { return 0; }
 
+  void test_generate_and_parse(const hobject_t &hoid, const std::string &mangled_expected) {
+    const std::string mangled_name = lfn_generate_object_name(hoid);
+    EXPECT_EQ(mangled_expected, mangled_name);
+    hobject_t hoid_parsed;
+    EXPECT_TRUE(lfn_parse_object_name(mangled_name, &hoid_parsed));
+    EXPECT_EQ(hoid, hoid_parsed);
+  }
+
 protected:
   virtual int _init() { return 0; }
 
@@ -81,9 +89,51 @@ protected:
                                       ) { return 0; }
 };
 
-class TestLFNIndex : public TestIndex, public ::testing::Test {
+class TestHASH_INDEX_TAG : public TestWrapLFNIndex, public ::testing::Test {
+public:
+  TestHASH_INDEX_TAG() : TestWrapLFNIndex(coll_t("ABC"), "PATH", CollectionIndex::HASH_INDEX_TAG) {
+  }
+};
+
+TEST_F(TestHASH_INDEX_TAG, generate_and_parse_name) {
+  const vector<string> path;
+  const std::string key;
+  uint64_t hash = 0xABABABAB;
+  uint64_t pool = -1;
+
+  test_generate_and_parse(hobject_t(object_t(".A/B_\\C.D"), key, CEPH_NOSNAP, hash, pool),
+                         "\\.A\\sB_\\\\C.D_head_ABABABAB");
+  test_generate_and_parse(hobject_t(object_t("DIR_A"), key, CEPH_NOSNAP, hash, pool),
+                         "\\dA_head_ABABABAB");
+}
+
+class TestHASH_INDEX_TAG_2 : public TestWrapLFNIndex, public ::testing::Test {
+public:
+  TestHASH_INDEX_TAG_2() : TestWrapLFNIndex(coll_t("ABC"), "PATH", CollectionIndex::HASH_INDEX_TAG_2) {
+  }
+};
+
+TEST_F(TestHASH_INDEX_TAG_2, generate_and_parse_name) {
+  const vector<string> path;
+  std::string mangled_name;
+  const std::string key("KEY");
+  uint64_t hash = 0xABABABAB;
+  uint64_t pool = -1;
+
+  {
+    std::string name(".XA/B_\\C.D");
+    name[1] = '\0';
+    hobject_t hoid(object_t(name), key, CEPH_NOSNAP, hash, pool);
+
+    test_generate_and_parse(hoid, "\\.\\nA\\sB\\u\\\\C.D_KEY_head_ABABABAB");
+  }
+  test_generate_and_parse(hobject_t(object_t("DIR_A"), key, CEPH_NOSNAP, hash, pool),
+                         "\\dA_KEY_head_ABABABAB");
+}
+
+class TestLFNIndex : public TestWrapLFNIndex, public ::testing::Test {
 public:
-  TestLFNIndex() : TestIndex(coll_t("ABC"), "PATH", 1) {
+  TestLFNIndex() : TestWrapLFNIndex(coll_t("ABC"), "PATH", CollectionIndex::HASH_INDEX_TAG) {
   }
 
   virtual void SetUp() {
@@ -233,29 +283,6 @@ TEST_F(TestLFNIndex, remove_object) {
 TEST_F(TestLFNIndex, get_mangled_name) {
   const vector<string> path;
 
-  //
-  // object name escape logic
-  //
-  {
-    std::string mangled_name;
-    int exists = 666;
-
-    hobject_t hoid(sobject_t(".A/B_\\C.D", CEPH_NOSNAP));
-
-    EXPECT_EQ(0, get_mangled_name(path, hoid, &mangled_name, &exists));
-    EXPECT_NE(std::string::npos, mangled_name.find("\\.A\\sB_\\\\C.D_head"));
-    EXPECT_EQ(0, exists);
-  }
-  {
-    std::string mangled_name;
-    int exists = 666;
-
-    hobject_t hoid(sobject_t("DIR_A", CEPH_NOSNAP));
-
-    EXPECT_EQ(0, get_mangled_name(path, hoid, &mangled_name, &exists));
-    EXPECT_NE(std::string::npos, mangled_name.find("\\dA_head"));
-    EXPECT_EQ(0, exists);
-  }
   //
   // small object name
   //
@@ -376,5 +403,5 @@ int main(int argc, char **argv) {
 }
 
 // Local Variables:
-// compile-command: "cd ../.. ; make unittest_lfnindex ; ./unittest_lfnindex # --gtest_filter=TestLFNIndex.* --log-to-stderr=true --debug-filestore=20"
+// compile-command: "cd ../.. ; make unittest_lfnindex ; valgrind --tool=memcheck ./unittest_lfnindex # --gtest_filter=TestLFNIndex.* --log-to-stderr=true --debug-filestore=20"
 // End: