Merge pull request ceph#39322 from rzarzynski/wip-librados-nul-objname

librados, pybind, tests: allow to list objects with the NUL character in names Reviewed-by: Josh Durgin <[email protected]> Reviewed-by: J. Eric Ivancich <[email protected]>
joskid · Feb 24, 2021 · c4d04b3 · c4d04b3
2 parents 8dae450 + da5d4c8
commit c4d04b3
Show file tree

Hide file tree

Showing 9 changed files with 126 additions and 16 deletions.
diff --git a/src/include/rados/librados.h b/src/include/rados/librados.h
@@ -222,7 +222,7 @@ typedef void *rados_ioctx_t;
  *
  * An iterator for listing the objects in a pool.
  * Used with rados_nobjects_list_open(),
- * rados_nobjects_list_next(), and
+ * rados_nobjects_list_next(), rados_nobjects_list_next2(), and
  * rados_nobjects_list_close().
  */
 typedef void *rados_list_ctx_t;
@@ -1137,6 +1137,32 @@ CEPH_RADOS_API int rados_nobjects_list_next(rados_list_ctx_t ctx,
 	                                    const char **key,
                                             const char **nspace);
 
+/**
+ * Get the next object name, locator and their sizes in the pool
+ *
+ * The sizes allow to list objects with \0 (the NUL character)
+ * in .e.g *entry. Is is unusual see such object names but a bug
+ * in a client has risen the need to handle them as well.
+ * *entry and *key are valid until next call to rados_nobjects_list_*
+ *
+ * @param ctx iterator marking where you are in the listing
+ * @param entry where to store the name of the entry
+ * @param key where to store the object locator (set to NULL to ignore)
+ * @param nspace where to store the object namespace (set to NULL to ignore)
+ * @param entry_size where to store the size of name of the entry
+ * @param key_size where to store the size of object locator (set to NULL to ignore)
+ * @param nspace_size where to store the size of object namespace (set to NULL to ignore)
+ * @returns 0 on success, negative error code on failure
+ * @returns -ENOENT when there are no more objects to list
+ */
+CEPH_RADOS_API int rados_nobjects_list_next2(rados_list_ctx_t ctx,
+                                             const char **entry,
+                                             const char **key,
+                                             const char **nspace,
+                                             size_t *entry_size,
+                                             size_t *key_size,
+                                             size_t *nspace_size);
+
 /**
  * Close the object listing handle.
  *

diff --git a/src/librados/librados_c.cc b/src/librados/librados_c.cc
@@ -2410,6 +2410,22 @@ extern "C" int LIBRADOS_C_API_DEFAULT_F(rados_nobjects_list_next)(
   const char **nspace)
 {
   tracepoint(librados, rados_nobjects_list_next_enter, listctx);
+  uint32_t retval = rados_nobjects_list_next2(listctx, entry, key, nspace, NULL, NULL, NULL);
+  tracepoint(librados, rados_nobjects_list_next_exit, 0, *entry, key, nspace);
+  return retval;
+}
+LIBRADOS_C_API_BASE_DEFAULT(rados_nobjects_list_next);
+
+extern "C" int LIBRADOS_C_API_DEFAULT_F(rados_nobjects_list_next2)(
+  rados_list_ctx_t listctx,
+  const char **entry,
+  const char **key,
+  const char **nspace,
+  size_t *entry_size,
+  size_t *key_size,
+  size_t *nspace_size)
+{
+  tracepoint(librados, rados_nobjects_list_next2_enter, listctx);
   librados::ObjListCtx *lh = (librados::ObjListCtx *)listctx;
   Objecter::NListContext *h = lh->nlc;
 
@@ -2421,11 +2437,11 @@ extern "C" int LIBRADOS_C_API_DEFAULT_F(rados_nobjects_list_next)(
   if (h->list.empty()) {
     int ret = lh->ctx->nlist(lh->nlc, RADOS_LIST_MAX_ENTRIES);
     if (ret < 0) {
-      tracepoint(librados, rados_nobjects_list_next_exit, ret, NULL, NULL, NULL);
+      tracepoint(librados, rados_nobjects_list_next2_exit, ret, NULL, NULL, NULL, NULL, NULL, NULL);
       return ret;
     }
     if (h->list.empty()) {
-      tracepoint(librados, rados_nobjects_list_next_exit, -ENOENT, NULL, NULL, NULL);
+      tracepoint(librados, rados_nobjects_list_next2_exit, -ENOENT, NULL, NULL, NULL, NULL, NULL, NULL);
       return -ENOENT;
     }
   }
@@ -2440,10 +2456,19 @@ extern "C" int LIBRADOS_C_API_DEFAULT_F(rados_nobjects_list_next)(
   }
   if (nspace)
     *nspace = h->list.front().nspace.c_str();
-  tracepoint(librados, rados_nobjects_list_next_exit, 0, *entry, key, nspace);
+
+  if (entry_size)
+    *entry_size = h->list.front().oid.size();
+  if (key_size)
+    *key_size = h->list.front().locator.size();
+  if (nspace_size)
+    *nspace_size = h->list.front().nspace.size();
+
+  tracepoint(librados, rados_nobjects_list_next2_exit, 0, entry, key, nspace,
+             entry_size, key_size, nspace_size);
   return 0;
 }
-LIBRADOS_C_API_BASE_DEFAULT(rados_nobjects_list_next);
+LIBRADOS_C_API_BASE_DEFAULT(rados_nobjects_list_next2);
 
 
 /*

diff --git a/src/librados/librados_cxx.cc b/src/librados/librados_cxx.cc
@@ -850,22 +850,24 @@ void librados::NObjectIteratorImpl::set_filter(const bufferlist &bl)
 void librados::NObjectIteratorImpl::get_next()
 {
   const char *entry, *key, *nspace;
+  size_t entry_size, key_size, nspace_size;
   if (ctx->nlc->at_end())
     return;
-  int ret = rados_nobjects_list_next(ctx.get(), &entry, &key, &nspace);
+  int ret = rados_nobjects_list_next2(ctx.get(), &entry, &key, &nspace,
+                                      &entry_size, &key_size, &nspace_size);
   if (ret == -ENOENT) {
     return;
   }
   else if (ret) {
     throw std::system_error(-ret, std::system_category(),
-                            "rados_nobjects_list_next");
+                            "rados_nobjects_list_next2");
   }
 
   if (cur_obj.impl == NULL)
     cur_obj.impl = new ListObjectImpl();
-  cur_obj.impl->nspace = nspace;
-  cur_obj.impl->oid = entry;
-  cur_obj.impl->locator = key ? key : string();
+  cur_obj.impl->nspace = string{nspace, nspace_size};
+  cur_obj.impl->oid = string{entry, entry_size};
+  cur_obj.impl->locator = key ? string(key, key_size) : string();
 }
 
 uint32_t librados::NObjectIteratorImpl::get_pg_hash_position() const

diff --git a/src/pybind/rados/c_rados.pxd b/src/pybind/rados/c_rados.pxd
@@ -191,6 +191,13 @@ cdef extern from "rados/librados.h" nogil:
 
     int rados_nobjects_list_open(rados_ioctx_t io, rados_list_ctx_t *ctx)
     int rados_nobjects_list_next(rados_list_ctx_t ctx, const char **entry, const char **key, const char **nspace)
+    int rados_nobjects_list_next2(rados_list_ctx_t ctx,
+                                  const char **entry,
+                                  const char **key,
+                                  const char **nspace,
+                                  size_t *entry_size,
+                                  size_t *key_size,
+                                  size_t *nspace)
     void rados_nobjects_list_close(rados_list_ctx_t ctx)
 
     int rados_ioctx_pool_requires_alignment2(rados_ioctx_t io, int * requires)

diff --git a/src/pybind/rados/mock_rados.pxi b/src/pybind/rados/mock_rados.pxi
@@ -267,6 +267,9 @@ cdef nogil:
         pass
     int rados_nobjects_list_next(rados_list_ctx_t ctx, const char **entry, const char **key, const char **nspace):
         pass
+    int rados_nobjects_list_next2(rados_list_ctx_t ctx, const char **entry, const char **key, const char **nspace,
+				  size_t *entry_size, size_t *key_size, size_t *nspace_size):
+        pass
     void rados_nobjects_list_close(rados_list_ctx_t ctx):
         pass
 

diff --git a/src/pybind/rados/rados.pyx b/src/pybind/rados/rados.pyx
@@ -1420,16 +1420,20 @@ cdef class ObjectIterator(object):
             const char *key_ = NULL
             const char *locator_ = NULL
             const char *nspace_ = NULL
+            size_t key_size_ = 0
+            size_t locator_size_ = 0
+            size_t nspace_size_ = 0
 
         with nogil:
-            ret = rados_nobjects_list_next(self.ctx, &key_, &locator_, &nspace_)
+            ret = rados_nobjects_list_next2(self.ctx, &key_, &locator_, &nspace_,
+                                            &key_size_, &locator_size_, &nspace_size_)
 
         if ret < 0:
             raise StopIteration()
 
-        key = decode_cstr(key_)
-        locator = decode_cstr(locator_) if locator_ != NULL else None
-        nspace = decode_cstr(nspace_) if nspace_ != NULL else None
+        key = decode_cstr(key_[:key_size_])
+        locator = decode_cstr(locator_[:locator_size_]) if locator_ != NULL else None
+        nspace = decode_cstr(nspace_[:nspace_size_]) if nspace_ != NULL else None
         return Object(self.ioctx, key, locator, nspace)
 
     def __dealloc__(self):

diff --git a/src/test/librados/list.cc b/src/test/librados/list.cc
@@ -38,6 +38,23 @@ TEST_F(LibRadosList, ListObjects) {
   rados_nobjects_list_close(ctx);
 }
 
+TEST_F(LibRadosList, ListObjectsZeroInName) {
+  char buf[128];
+  memset(buf, 0xcc, sizeof(buf));
+  ASSERT_EQ(0, rados_write(ioctx, "foo\0bar", buf, sizeof(buf), 0));
+  rados_list_ctx_t ctx;
+  ASSERT_EQ(0, rados_nobjects_list_open(ioctx, &ctx));
+  const char *entry;
+  size_t entry_size;
+  bool foundit = false;
+  while (rados_nobjects_list_next2(ctx, &entry, NULL, NULL,
+				   &entry_size, NULL, NULL) != -ENOENT) {
+    foundit = true;
+    ASSERT_EQ(std::string(entry, entry_size), "foo\0bar");
+  }
+  ASSERT_TRUE(foundit);
+  rados_nobjects_list_close(ctx);
+}
 
 static void check_list(
   std::set<std::string>& myset,

diff --git a/src/tools/scratchtool.c b/src/tools/scratchtool.c
@@ -291,8 +291,8 @@ static int testrados(void)
 	r = rados_nobjects_list_open(io_ctx, &h);
 	printf("rados_nobjects_list_open = %d, h = %p\n", r, h);
 	const char *poolname;
-	while (rados_nobjects_list_next(h, &poolname, NULL, NULL) == 0)
-		printf("rados_nobjects_list_next got object '%s'\n", poolname);
+	while (rados_nobjects_list_next2(h, &poolname, NULL, NULL, NULL, NULL, NULL) == 0)
+		printf("rados_nobjects_list_next2 got object '%s'\n", poolname);
 	rados_nobjects_list_close(h);
 
 	/* stat */

diff --git a/src/tracing/librados.tp b/src/tracing/librados.tp
@@ -1884,6 +1884,32 @@ TRACEPOINT_EVENT(librados, rados_nobjects_list_next_exit,
     )
 )
 
+TRACEPOINT_EVENT(librados, rados_nobjects_list_next2_enter,
+    TP_ARGS(
+        rados_list_ctx_t, listctx),
+    TP_FIELDS(
+        ctf_integer_hex(rados_list_ctx_t, listctx, listctx)
+    )
+)
+
+TRACEPOINT_EVENT(librados, rados_nobjects_list_next2_exit,
+    TP_ARGS(
+        int, retval,
+        const char* const*, entry,
+        const char* const*, key,
+        const char* const*, nspace,
+        const size_t*, entry_size,
+        const size_t*, key_size,
+        const size_t*, nspace_size),
+    TP_FIELDS(
+        ctf_integer(int, retval, retval)
+        ceph_ctf_sequencep(char, entry, entry, size_t, entry_size)
+        ceph_ctf_sequencep(char, key, key, size_t, key_size)
+        ceph_ctf_sequencep(char, nspace, nspace, size_t, nspace_size)
+    )
+)
+
+
 TRACEPOINT_EVENT(librados, rados_objects_list_open_enter,
     TP_ARGS(
         rados_ioctx_t, ioctx),