MB-16181: Make server deny creation of reserved collections 74/78074/8
authorJim Walker <jim@couchbase.com>
Fri, 12 May 2017 09:12:20 +0000 (10:12 +0100)
committerDave Rigby <daver@couchbase.com>
Wed, 17 May 2017 10:10:54 +0000 (10:10 +0000)
The design has always stated that _ and $ would be reserved for
system use. Make sure the server enforces this by checking the
names on incoming manifests.

1. Any _ prefixed collection will cause the Manifest construction to
 throw invalid_argument.

2. Any $ prefixed collection which is not $default will cause the
 Manifest construction to throw invalid_argument.

Change-Id: I1e5daf2ae87cba2a8dbcdda4c9bb0be66e40ffae
Reviewed-on: http://review.couchbase.org/78074
Reviewed-by: Trond Norbye <trond.norbye@gmail.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
src/collections/manifest.cc
src/collections/manifest.h
tests/module_tests/collections/manifest_test.cc

index 5004db0..1b38dbe 100644 (file)
@@ -84,8 +84,12 @@ Manifest::Manifest(const std::string& json) {
                         std::to_string(ii) +
                         (!collection ? " nullptr"
                                      : std::to_string(collection->type)));
-            } else {
+            } else if (validCollection(collection->valuestring)) {
                 collections.push_back(collection->valuestring);
+            } else {
+                throw std::invalid_argument(
+                        "Manifest::Manifest invalid collection name:" +
+                        std::string(collection->valuestring));
             }
         }
     }
@@ -95,4 +99,15 @@ bool Manifest::validSeparator(const char* separator) {
     size_t size = std::strlen(separator);
     return size > 0 && size <= 250;
 }
+
+bool Manifest::validCollection(const char* collection) {
+    // Current validation is to just check the prefix to ensure
+    // 1. $default is the only $ prefixed collection.
+    // 2. _ is not allowed as the first character.
+
+    if (collection[0] == '$' && !(DefaultCollectionIdentifier == collection)) {
+        return false;
+    }
+    return collection[0] != '_';
+}
 }
index 6d1ac7d..79d2f59 100644 (file)
@@ -77,6 +77,14 @@ private:
      */
     static bool validSeparator(const char* separator);
 
+    /**
+     * Check if the C-string represents a legal collection name.
+     * Current validation is to ensure we block creation of _ prefixed
+     * collections and only accept $default for $ prefixed names.
+     * @param collection a C-string representing a collection name.
+     */
+    static bool validCollection(const char* collection);
+
     int revision;
     std::string separator;
     std::vector<std::string> collections;
index 8dd9e6f..fba6a67 100644 (file)
@@ -62,7 +62,13 @@ TEST(ManifestTest, validation) {
             "------------------------------------------------------------------"
             "------------------------------------------------------------------"
             "---\","
-            "\"collections\":[\"beer\",\"brewery\"]}"};
+            "\"collections\":[\"beer\",\"brewery\"]}",
+            "{\"revision\":0"
+            "\"separator\":\":\"," // illegal $ prefixed  name
+            "\"collections\":[\"$magic\",\"beer\",\"brewery\"]}",
+            "{\"revision\":0"
+            "\"separator\":\":\"," // illegal _ prefixed  name
+            "\"collections\":[\"beer\",\"_brewery\"]}"};
 
     std::vector<std::string> validManifests = {
             "{\"revision\":0,"