SERVER-23297 consolidate namespace validation

8 files changed, 40 insertions, 23 deletions

diff --git a/src/mongo/client/dbclient.cpp b/src/mongo/client/dbclient.cpp
index c7fafa1eec8..8500868a46f 100644
--- a/src/mongo/client/dbclient.cpp
+++ b/src/mongo/client/dbclient.cpp
@@ -760,7 +760,7 @@ list<BSONObj> DBClientWithCommands::getCollectionInfos(const string& db, const B
     while (c->more()) {
         BSONObj obj = c->nextSafe();
         string ns = obj["name"].valuestr();
-        if (ns.find("$") != string::npos)
+        if (NamespaceString::virtualized(ns))
             continue;
         BSONObjBuilder b;
         b.append("name", ns.substr(db.size() + 1));
diff --git a/src/mongo/db/catalog/collection.cpp b/src/mongo/db/catalog/collection.cpp
index 6e5f5a5306b..c3bbffa8603 100644
--- a/src/mongo/db/catalog/collection.cpp
+++ b/src/mongo/db/catalog/collection.cpp
@@ -205,8 +205,8 @@ Collection::~Collection() {
 }
 
 bool Collection::requiresIdIndex() const {
-    if (_ns.ns().find('$') != string::npos) {
-        // no indexes on indexes
+    if (_ns.isVirtualized() || _ns.isOplog()) {
+        // No indexes on virtual collections or the oplog.
         return false;
     }
 
@@ -217,17 +217,6 @@ bool Collection::requiresIdIndex() const {
         }
     }
 
-    if (_ns.db() == "local") {
-        if (_ns.coll().startsWith("oplog."))
-            return false;
-    }
-
-    if (!_ns.isSystem()) {
-        // non system collections definitely have an _id index
-        return true;
-    }
-
-
     return true;
 }
 
diff --git a/src/mongo/db/dbcommands.cpp b/src/mongo/db/dbcommands.cpp
index 8dc17ca1972..1516a27989a 100644
--- a/src/mongo/db/dbcommands.cpp
+++ b/src/mongo/db/dbcommands.cpp
@@ -470,8 +470,8 @@ public:
                      BSONObjBuilder& result) {
         const std::string nsToDrop = parseNsCollectionRequired(dbname, cmdObj);
 
-        if (nsToDrop.find('$') != string::npos) {
-            errmsg = "can't drop collection with reserved $ character in name";
+        if (NamespaceString::virtualized(nsToDrop)) {
+            errmsg = "can't drop a virtual collection";
             return false;
         }
 
diff --git a/src/mongo/db/namespace_string-inl.h b/src/mongo/db/namespace_string-inl.h
index 7bb98f81086..05387e4791a 100644
--- a/src/mongo/db/namespace_string-inl.h
+++ b/src/mongo/db/namespace_string-inl.h
@@ -41,9 +41,7 @@ inline StringData NamespaceString::coll() const {
 }
 
 inline bool NamespaceString::normal(StringData ns) {
-    if (ns.find('$') == std::string::npos)
-        return true;
-    return oplog(ns);
+    return !virtualized(ns);
 }
 
 inline bool NamespaceString::oplog(StringData ns) {
@@ -54,6 +52,10 @@ inline bool NamespaceString::special(StringData ns) {
     return !normal(ns) || ns.substr(ns.find('.')).startsWith(".system.");
 }
 
+inline bool NamespaceString::virtualized(StringData ns) {
+    return ns.find('$') != std::string::npos && ns != "local.oplog.$main";
+}
+
 inline bool NamespaceString::validDBName(StringData db, DollarInDbNameBehavior behavior) {
     if (db.size() == 0 || db.size() > 64)
         return false;
diff --git a/src/mongo/db/namespace_string.h b/src/mongo/db/namespace_string.h
index 65d23736dee..651c5fc830e 100644
--- a/src/mongo/db/namespace_string.h
+++ b/src/mongo/db/namespace_string.h
@@ -161,6 +161,12 @@ public:
     bool isNormal() const {
         return normal(_ns);
     }
+
+    // Check if the NamespaceString references a special collection that cannot
+    // be used for generic data storage.
+    bool isVirtualized() const {
+        return virtualized(_ns);
+    }
     bool isListCollectionsCursorNS() const;
     bool isListIndexesCursorNS() const;
 
@@ -227,6 +233,10 @@ public:
 
     static bool special(StringData ns);
 
+    // Check if `ns` references a special collection that cannot be used for
+    // generic data storage.
+    static bool virtualized(StringData ns);
+
     /**
      * Returns true for DBs with special meaning to mongodb.
      */
diff --git a/src/mongo/db/namespace_string_test.cpp b/src/mongo/db/namespace_string_test.cpp
index 03e2837159c..5f3c326aeec 100644
--- a/src/mongo/db/namespace_string_test.cpp
+++ b/src/mongo/db/namespace_string_test.cpp
@@ -42,6 +42,8 @@ TEST(NamespaceStringTest, Normal) {
 
     ASSERT(!NamespaceString::normal("a.b.$c"));
     ASSERT(!NamespaceString::normal("a.b.$.c"));
+    ASSERT(!NamespaceString::normal("a.b$.c"));
+    ASSERT(!NamespaceString::normal("a$.b.c"));
 
     ASSERT(NamespaceString::normal("local.oplog.$main"));
     ASSERT(NamespaceString::normal("local.oplog.rs"));
@@ -65,6 +67,20 @@ TEST(NamespaceStringTest, Special) {
     ASSERT(!NamespaceString::special("a.systemfoo"));
 }
 
+TEST(NamespaceStringTest, Virtualized) {
+    ASSERT(!NamespaceString::virtualized("a"));
+    ASSERT(!NamespaceString::virtualized("a.b"));
+    ASSERT(!NamespaceString::virtualized("a.b.c"));
+
+    ASSERT(NamespaceString::virtualized("a.b.$c"));
+    ASSERT(NamespaceString::virtualized("a.b.$.c"));
+    ASSERT(NamespaceString::virtualized("a.b$.c"));
+    ASSERT(NamespaceString::virtualized("a$.b.c"));
+
+    ASSERT(!NamespaceString::virtualized("local.oplog.$main"));
+    ASSERT(!NamespaceString::virtualized("local.oplog.rs"));
+}
+
 TEST(NamespaceStringTest, DatabaseValidNames) {
     ASSERT(NamespaceString::validDBName("foo", NamespaceString::DollarInDbNameBehavior::Allow));
     ASSERT(NamespaceString::validDBName("foo$bar", NamespaceString::DollarInDbNameBehavior::Allow));
diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp
index dbbd6e8176a..d2c70e1d673 100644
--- a/src/mongo/db/query/get_executor.cpp
+++ b/src/mongo/db/query/get_executor.cpp
@@ -647,9 +647,9 @@ StatusWith<unique_ptr<PlanExecutor>> getExecutorDelete(OperationContext* txn,
             uassert(
                 12050, "cannot delete from system namespace", legalClientSystemNS(nss.ns(), true));
         }
-        if (nss.ns().find('$') != string::npos) {
-            log() << "cannot delete from collection with reserved $ in name: " << nss;
-            uasserted(10100, "cannot delete from collection with reserved $ in name");
+        if (nss.isVirtualized()) {
+            log() << "cannot delete from a virtual collection: " << nss;
+            uasserted(10100, "cannot delete from a virtual collection");
         }
     }
 
diff --git a/src/mongo/db/storage/mmap_v1/record_store_v1_base.cpp b/src/mongo/db/storage/mmap_v1/record_store_v1_base.cpp
index 5f5e0438d73..0105d10c4df 100644
--- a/src/mongo/db/storage/mmap_v1/record_store_v1_base.cpp
+++ b/src/mongo/db/storage/mmap_v1/record_store_v1_base.cpp
@@ -256,7 +256,7 @@ DiskLoc RecordStoreV1Base::_findFirstSpot(OperationContext* txn,
     DiskLoc emptyLoc = extDiskLoc;
     emptyLoc.inc(Extent::HeaderSize());
     int delRecLength = e->length - Extent::HeaderSize();
-    if (delRecLength >= 32 * 1024 && _ns.find('$') != string::npos && !isCapped()) {
+    if (delRecLength >= 32 * 1024 && NamespaceString::virtualized(_ns) && !isCapped()) {
         // probably an index. so skip forward to keep its records page aligned
         int& ofs = emptyLoc.GETOFS();
         int newOfs = (ofs + 0xfff) & ~0xfff;