Merge pull request #54 from HDFGroup/chogan/name_limits

Limit Bucket names to 256B and Blob names to 64B.
HDFGroup · Dec 15, 2020 · 8e170a6 · 8e170a6
2 parents 232a1a5 + b1ade64
commit 8e170a6
Show file tree

Hide file tree

Showing 6 changed files with 35 additions and 16 deletions.
diff --git a/src/api/bucket.cc b/src/api/bucket.cc
@@ -16,7 +16,7 @@ Bucket::Bucket(const std::string &initial_name,
     : name_(initial_name), hermes_(h) {
   (void)ctx;
 
-  if (IsNameTooLong(name_)) {
+  if (IsBucketNameTooLong(name_)) {
     id_.as_int = 0;
   } else {
     id_ = GetOrCreateBucketId(&hermes_->context_, &hermes_->rpc_, name_);
@@ -33,7 +33,7 @@ Status Bucket::Put(const std::string &name, const u8 *data, size_t size,
                    Context &ctx) {
   Status ret = 0;
 
-  if (IsNameTooLong(name)) {
+  if (IsBlobNameTooLong(name)) {
     // TODO(chogan): @errorhandling
     ret = 1;
   }
@@ -164,7 +164,7 @@ Status Bucket::RenameBlob(const std::string &old_name,
   (void)ctx;
   Status ret = 0;
 
-  if (IsNameTooLong(new_name)) {
+  if (IsBlobNameTooLong(new_name)) {
     ret = 1;
     // TODO(chogan): @errorhandling
   } else {
@@ -213,7 +213,7 @@ Status Bucket::Rename(const std::string &new_name, Context &ctx) {
   (void)ctx;
   Status ret = 0;
 
-  if (IsNameTooLong(new_name)) {
+  if (IsBucketNameTooLong(new_name)) {
     ret = 1;
     // TODO(chogan): @errorhandling
   } else {

diff --git a/src/api/bucket.h b/src/api/bucket.h
@@ -166,7 +166,7 @@ Status Bucket::Put(std::vector<std::string> &names,
   Status ret = 0;
 
   for (auto &name : names) {
-    if (IsNameTooLong(name)) {
+    if (IsBlobNameTooLong(name)) {
       // TODO(chogan): @errorhandling
       ret = 1;
       break;

diff --git a/src/hermes_types.h b/src/hermes_types.h
@@ -39,7 +39,8 @@ static constexpr int kMaxBufferPoolSlabs = 8;
 constexpr int kMaxPathLength = 256;
 constexpr int kMaxBufferPoolShmemNameLength = 64;
 constexpr int kMaxDevices = 8;
-constexpr int kMaxNameSize = 128;
+constexpr int kMaxBucketNameSize = 256;
+constexpr int kMaxBlobNameSize = 64;
 
 constexpr char kPlaceInHierarchy[] = "PlaceInHierarchy";
 

diff --git a/src/metadata_management.cc b/src/metadata_management.cc
@@ -12,17 +12,29 @@
 
 namespace hermes {
 
-bool IsNameTooLong(const std::string &name) {
+bool IsNameTooLong(const std::string &name, size_t max) {
   bool result = false;
-  if (name.size() + 1 >= kMaxNameSize) {
+  if (name.size() + 1 >= max) {
     LOG(WARNING) << "Name '" << name << "' exceeds the maximum name size of "
-                 << kMaxNameSize << " bytes." << std::endl;
+                 << max << " bytes." << std::endl;
     result = true;
   }
 
   return result;
 }
 
+bool IsBlobNameTooLong(const std::string &name) {
+  bool result = IsNameTooLong(name, kMaxBlobNameSize);
+
+  return result;
+}
+
+bool IsBucketNameTooLong(const std::string &name) {
+  bool result = IsNameTooLong(name, kMaxBucketNameSize);
+
+  return result;
+}
+
 bool IsNullBucketId(BucketID id) {
   bool result = id.as_int == 0;
 

diff --git a/src/metadata_management.h b/src/metadata_management.h
@@ -231,7 +231,12 @@ SwapBlob IdArrayToSwapBlob(BufferIdArray ids);
 /**
  *
  */
-bool IsNameTooLong(const std::string &name);
+bool IsBlobNameTooLong(const std::string &name);
+
+/**
+ *
+ */
+bool IsBucketNameTooLong(const std::string &name);
 
 }  // namespace hermes
 

diff --git a/test/mdm_test.cc b/test/mdm_test.cc
@@ -149,27 +149,28 @@ static void TestBucketRefCounting(HermesPtr hermes) {
 static void TestMaxNameLength(HermesPtr hermes) {
   // Bucket with a name that's too large is invalid.
   hapi::Context ctx;
-  std::string long_name(kMaxNameSize + 1, 'x');
-  hapi::Bucket invalid_bucket(long_name, hermes, ctx);
+  std::string long_bucket_name(kMaxBucketNameSize + 1, 'x');
+  hapi::Bucket invalid_bucket(long_bucket_name, hermes, ctx);
   Assert(!invalid_bucket.IsValid());
 
   // Put fails when a blob name is too long
   std::string name = "b1";
+  std::string long_blob_name(kMaxBlobNameSize + 1, 'x');
   hapi::Bucket bucket(name, hermes, ctx);
   hapi::Blob blob('x');
-  Status status = bucket.Put(long_name, blob, ctx);
+  Status status = bucket.Put(long_blob_name, blob, ctx);
   Assert(status != 0);
-  Assert(!bucket.ContainsBlob(long_name));
+  Assert(!bucket.ContainsBlob(long_blob_name));
 
   // Vector Put fails if one name is too long
   std::string a = "a";
   std::string b = "b";
   std::string c = "c";
-  std::vector<std::string> blob_names = {a, b, long_name, c};
+  std::vector<std::string> blob_names = {a, b, long_blob_name, c};
   std::vector<hapi::Blob> blobs = {blob, blob, blob, blob};
   status = bucket.Put(blob_names, blobs, ctx);
   Assert(status != 0);
-  Assert(!bucket.ContainsBlob(long_name));
+  Assert(!bucket.ContainsBlob(long_blob_name));
   Assert(!bucket.ContainsBlob(a));
   Assert(!bucket.ContainsBlob(b));
   Assert(!bucket.ContainsBlob(c));