SERVER-19033 find command now validates the type of its first parameter

11 files changed, 196 insertions, 77 deletions

diff --git a/src/mongo/client/remote_command_runner_impl.cpp b/src/mongo/client/remote_command_runner_impl.cpp
index 72d57cdfdb2..51dc43108d3 100644
--- a/src/mongo/client/remote_command_runner_impl.cpp
+++ b/src/mongo/client/remote_command_runner_impl.cpp
@@ -95,12 +95,16 @@ namespace {
                                        BSONObj* output) {
 
         const NamespaceString nss(dbname, cmdObj.firstElement().String());
+        if (!nss.isValid()) {
+            return {ErrorCodes::InvalidNamespace,
+                    str::stream() << "Invalid collection name: " << nss.ns()};
+        }
         const std::string& ns = nss.ns();
 
         // It is a little heavy handed to use LiteParsedQuery to convert the command object to
         // query() arguments but we get validation and consistent behavior with the find
         // command implementation on the remote server.
-        auto lpqStatus = LiteParsedQuery::fromFindCommand(ns, cmdObj, false);
+        auto lpqStatus = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, false);
         if (!lpqStatus.isOK()) {
             *output = getCommandResultFromStatus(lpqStatus.getStatus());
             return lpqStatus.getStatus();
diff --git a/src/mongo/db/commands/find_cmd.cpp b/src/mongo/db/commands/find_cmd.cpp
index 2f92a4dcdd7..0bc9589bbef 100644
--- a/src/mongo/db/commands/find_cmd.cpp
+++ b/src/mongo/db/commands/find_cmd.cpp
@@ -103,10 +103,14 @@ namespace mongo {
                        BSONObjBuilder* out) const override {
             const std::string fullns = parseNs(dbname, cmdObj);
             const NamespaceString nss(fullns);
+            if (!nss.isValid()) {
+                return {ErrorCodes::InvalidNamespace,
+                        str::stream() << "Invalid collection name: " << nss.ns()};
+            }
 
             // Parse the command BSON to a LiteParsedQuery.
             const bool isExplain = true;
-            auto lpqStatus = LiteParsedQuery::fromFindCommand(fullns, cmdObj, isExplain);
+            auto lpqStatus = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
             if (!lpqStatus.isOK()) {
                 return lpqStatus.getStatus();
             }
@@ -173,6 +177,11 @@ namespace mongo {
                  BSONObjBuilder& result) override {
             const std::string fullns = parseNs(dbname, cmdObj);
             const NamespaceString nss(fullns);
+            if (!nss.isValid()) {
+                return appendCommandStatus(result, {ErrorCodes::InvalidNamespace,
+                                                    str::stream() << "Invalid collection name: "
+                                                                  << nss.ns()});
+            }
 
             // Although it is a command, a find command gets counted as a query.
             globalOpCounters.gotQuery();
@@ -185,7 +194,7 @@ namespace mongo {
 
             // 1a) Parse the command BSON to a LiteParsedQuery.
             const bool isExplain = false;
-            auto lpqStatus = LiteParsedQuery::fromFindCommand(fullns, cmdObj, isExplain);
+            auto lpqStatus = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
             if (!lpqStatus.isOK()) {
                 return appendCommandStatus(result, lpqStatus.getStatus());
             }
diff --git a/src/mongo/db/exec/multi_plan.cpp b/src/mongo/db/exec/multi_plan.cpp
index 8bc98d109bd..8919ada64e7 100644
--- a/src/mongo/db/exec/multi_plan.cpp
+++ b/src/mongo/db/exec/multi_plan.cpp
@@ -213,7 +213,7 @@ namespace mongo {
             numResults = std::min(static_cast<size_t>(*query.getParsed().getLimit()),
                                   numResults);
         }
-        else if (!query.getParsed().fromFindCommand() && query.getParsed().getBatchSize()) {
+        else if (!query.getParsed().isFromFindCommand() && query.getParsed().getBatchSize()) {
             numResults = std::min(static_cast<size_t>(*query.getParsed().getBatchSize()),
                                   numResults);
         }
diff --git a/src/mongo/db/query/find.cpp b/src/mongo/db/query/find.cpp
index dbf656723ba..d266518a961 100644
--- a/src/mongo/db/query/find.cpp
+++ b/src/mongo/db/query/find.cpp
@@ -149,7 +149,7 @@ namespace mongo {
             return false;
         }
 
-        if (!pq.fromFindCommand() && pq.getBatchSize() && *pq.getBatchSize() == 1) {
+        if (!pq.isFromFindCommand() && pq.getBatchSize() && *pq.getBatchSize() == 1) {
             return false;
         }
 
diff --git a/src/mongo/db/query/lite_parsed_query.cpp b/src/mongo/db/query/lite_parsed_query.cpp
index c508d7d6565..f0c15c7b4f4 100644
--- a/src/mongo/db/query/lite_parsed_query.cpp
+++ b/src/mongo/db/query/lite_parsed_query.cpp
@@ -95,13 +95,13 @@ namespace {
 } // namespace
 
     // static
-    StatusWith<unique_ptr<LiteParsedQuery>> LiteParsedQuery::fromFindCommand(
-            const std::string& fullns,
+    StatusWith<unique_ptr<LiteParsedQuery>> LiteParsedQuery::makeFromFindCommand(
+            const NamespaceString& nss,
             const BSONObj& cmdObj,
             bool isExplain) {
 
         unique_ptr<LiteParsedQuery> pq(new LiteParsedQuery());
-        pq->_ns = fullns;
+        pq->_ns = nss.ns();
         pq->_fromCommand = true;
         pq->_explain = isExplain;
 
@@ -111,9 +111,10 @@ namespace {
             BSONElement el = it.next();
             const char* fieldName = el.fieldName();
             if (str::equals(fieldName, kCmdName)) {
-                // We've already parsed the namespace information contained in the 'find'
-                // field, so just move on.
-                continue;
+                Status status = checkFieldType(el, String);
+                if (!status.isOK()) {
+                    return status;
+                }
             }
             else if (str::equals(fieldName, kFilterField)) {
                 Status status = checkFieldType(el, Object);
diff --git a/src/mongo/db/query/lite_parsed_query.h b/src/mongo/db/query/lite_parsed_query.h
index 9623805d631..0607eabcf8b 100644
--- a/src/mongo/db/query/lite_parsed_query.h
+++ b/src/mongo/db/query/lite_parsed_query.h
@@ -35,6 +35,7 @@
 
 namespace mongo {
 
+    class NamespaceString;
     class QueryMessage;
     class Status;
     template<typename T> class StatusWith;
@@ -52,9 +53,8 @@ namespace mongo {
          * Returns a heap allocated LiteParsedQuery on success or an error if 'cmdObj' is not well
          * formed.
          */
-        static StatusWith<std::unique_ptr<LiteParsedQuery>> fromFindCommand(const std::string& ns,
-                                                                            const BSONObj& cmdObj,
-                                                                            bool isExplain);
+        static StatusWith<std::unique_ptr<LiteParsedQuery>>
+            makeFromFindCommand(const NamespaceString& nss, const BSONObj& cmdObj, bool isExplain);
 
         /**
          * Short and long forms for constructing a new LiteParsedQuery.
@@ -146,7 +146,7 @@ namespace mongo {
         boost::optional<int> getBatchSize() const { return _batchSize; }
         bool wantMore() const { return _wantMore; }
 
-        bool fromFindCommand() const { return _fromCommand; }
+        bool isFromFindCommand() const { return _fromCommand; }
         bool isExplain() const { return _explain; }
 
         const std::string& getComment() const { return _comment; }
diff --git a/src/mongo/db/query/lite_parsed_query_test.cpp b/src/mongo/db/query/lite_parsed_query_test.cpp
index d31d0501b7e..5c2613eb41f 100644
--- a/src/mongo/db/query/lite_parsed_query_test.cpp
+++ b/src/mongo/db/query/lite_parsed_query_test.cpp
@@ -32,6 +32,7 @@
 #include <boost/optional/optional_io.hpp>
 
 #include "mongo/db/json.h"
+#include "mongo/db/namespace_string.h"
 #include "mongo/db/query/lite_parsed_query.h"
 #include "mongo/unittest/unittest.h"
 
@@ -333,8 +334,10 @@ namespace {
                                    "filter: {a: 3},"
                                    "sort: {a: 1},"
                                    "projection: {_id: 0, a: 1}}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandWithOptions) {
@@ -344,9 +347,10 @@ namespace {
                                    "projection: {_id: 0, a: 1},"
                                    "showRecordId: true,"
                                    "maxScan: 1000}}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
         unique_ptr<LiteParsedQuery> lpq(
-            assertGet(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain)));
+            assertGet(LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain)));
 
         // Make sure the values from the command BSON are reflected in the LPQ.
         ASSERT(lpq->showRecordId());
@@ -357,9 +361,10 @@ namespace {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "filter:  {a: 1},"
                                    "hint: 'foo_1'}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
         unique_ptr<LiteParsedQuery> lpq(
-            assertGet(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain)));
+            assertGet(LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain)));
 
         BSONObj hintObj = lpq->getHint();
         ASSERT_EQUALS(BSON("$hint" << "foo_1"), hintObj);
@@ -369,16 +374,18 @@ namespace {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "projection: {a: 1},"
                                    "sort: {a: 1}}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        ASSERT_OK(LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain).getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandValidSortProjMeta) {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "projection: {a: {$meta: 'textScore'}},"
                                    "sort: {a: {$meta: 'textScore'}}}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        ASSERT_OK(LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain).getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandAllFlagsTrue) {
@@ -389,9 +396,10 @@ namespace {
                                    "noCursorTimeout: true,"
                                    "awaitData: true,"
                                    "partial: true}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
         unique_ptr<LiteParsedQuery> lpq(
-            assertGet(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain)));
+            assertGet(LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain)));
 
         // Test that all the flags got set to true.
         ASSERT(lpq->isTailable());
@@ -407,9 +415,10 @@ namespace {
                                    "comment: 'the comment',"
                                    "min: {a: 1},"
                                    "max: {a: 2}}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
         unique_ptr<LiteParsedQuery> lpq(
-            assertGet(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain)));
+            assertGet(LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain)));
 
         ASSERT_EQUALS("the comment", lpq->getComment());
         BSONObj expectedMin = BSON("a" << 1);
@@ -428,9 +437,10 @@ namespace {
                                    "skip: 5,"
                                    "batchSize: 90,"
                                    "singleBatch: false}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
         unique_ptr<LiteParsedQuery> lpq(
-            assertGet(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain)));
+            assertGet(LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain)));
 
         // Check the values inside the LPQ.
         BSONObj expectedQuery = BSON("a" << 1);
@@ -454,24 +464,30 @@ namespace {
     TEST(LiteParsedQueryTest, ParseFromCommandQueryWrongType) {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "filter: 3}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandSortWrongType) {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "filter:  {a: 1},"
                                    "sort: 3}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandProjWrongType) {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "filter:  {a: 1},"
                                    "projection: 'foo'}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandSkipWrongType) {
@@ -479,8 +495,10 @@ namespace {
                                    "filter:  {a: 1},"
                                    "skip: '5',"
                                    "projection: {a: 1}}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandLimitWrongType) {
@@ -488,8 +506,10 @@ namespace {
                                    "filter:  {a: 1},"
                                    "limit: '5',"
                                    "projection: {a: 1}}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandSingleBatchWrongType) {
@@ -497,16 +517,20 @@ namespace {
                                    "filter:  {a: 1},"
                                    "singleBatch: 'false',"
                                    "projection: {a: 1}}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandCommentWrongType) {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "filter:  {a: 1},"
                                    "comment: 1}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandMaxScanWrongType) {
@@ -514,40 +538,50 @@ namespace {
                                    "filter:  {a: 1},"
                                    "maxScan: true,"
                                    "comment: 'foo'}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandMaxTimeMSWrongType) {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "filter:  {a: 1},"
                                    "maxTimeMS: true}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandMaxWrongType) {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "filter:  {a: 1},"
                                    "max: 3}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandMinWrongType) {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "filter:  {a: 1},"
                                    "min: 3}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandReturnKeyWrongType) {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "filter:  {a: 1},"
                                    "returnKey: 3}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
 
@@ -555,48 +589,60 @@ namespace {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "filter:  {a: 1},"
                                    "showRecordId: 3}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandSnapshotWrongType) {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "filter:  {a: 1},"
                                    "snapshot: 3}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandTailableWrongType) {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "filter:  {a: 1},"
                                    "tailable: 3}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandSlaveOkWrongType) {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "filter:  {a: 1},"
                                    "slaveOk: 3}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandOplogReplayWrongType) {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "filter:  {a: 1},"
                                    "oplogReplay: 3}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandNoCursorTimeoutWrongType) {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "filter:  {a: 1},"
                                    "noCursorTimeout: 3}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandAwaitDataWrongType) {
@@ -604,24 +650,30 @@ namespace {
                                    "filter:  {a: 1},"
                                    "tailable: true,"
                                    "awaitData: 3}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandExhaustWrongType) {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "filter:  {a: 1},"
                                    "exhaust: 3}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandPartialWrongType) {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "filter:  {a: 1},"
                                    "exhaust: 3}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     //
@@ -632,31 +684,38 @@ namespace {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "skip: -3,"
                                    "filter: {a: 3}}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandNegativeLimitError) {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "limit: -3,"
                                    "filter: {a: 3}}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandNegativeBatchSizeError) {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "batchSize: -10,"
                                    "filter: {a: 3}}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandBatchSizeZero) {
         BSONObj cmdObj = fromjson("{find: 'testns', batchSize: 0}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
         unique_ptr<LiteParsedQuery> lpq(
-            assertGet(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain)));
+            assertGet(LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain)));
 
         ASSERT(lpq->getBatchSize());
         ASSERT_EQ(0, lpq->getBatchSize());
@@ -666,9 +725,10 @@ namespace {
 
     TEST(LiteParsedQueryTest, ParseFromCommandDefaultBatchSize) {
         BSONObj cmdObj = fromjson("{find: 'testns'}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
         unique_ptr<LiteParsedQuery> lpq(
-            assertGet(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain)));
+            assertGet(LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain)));
 
         ASSERT(!lpq->getBatchSize());
         ASSERT(!lpq->getLimit());
@@ -682,24 +742,30 @@ namespace {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "min: {a: 3},"
                                    "max: {b: 4}}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandSnapshotPlusSortError) {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "sort: {a: 3},"
                                    "snapshot: true}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandSnapshotPlusHintError) {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "snapshot: true,"
                                    "hint: {a: 1}}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseCommandForbidNonMetaSortOnFieldWithMetaProject) {
@@ -708,13 +774,15 @@ namespace {
         cmdObj = fromjson("{find: 'testns',"
                            "projection: {a: {$meta: 'textScore'}},"
                            "sort: {a: 1}}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
 
         cmdObj = fromjson("{find: 'testns',"
                            "projection: {a: {$meta: 'textScore'}},"
                            "sort: {b: 1}}");
-        ASSERT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        ASSERT_OK(LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain).getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseCommandForbidMetaSortOnFieldWithoutMetaProject) {
@@ -723,28 +791,34 @@ namespace {
         cmdObj = fromjson("{find: 'testns',"
                            "projection: {a: 1},"
                            "sort: {a: {$meta: 'textScore'}}}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
 
         cmdObj = fromjson("{find: 'testns',"
                            "projection: {b: 1},"
                            "sort: {a: {$meta: 'textScore'}}}");
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseCommandForbidExhaust) {
         BSONObj cmdObj = fromjson("{find: 'testns', exhaust: true}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseCommandIsFromFindCommand) {
         BSONObj cmdObj = fromjson("{find: 'testns'}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
         unique_ptr<LiteParsedQuery> lpq(
-            assertGet(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain)));
+            assertGet(LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain)));
 
-        ASSERT(lpq->fromFindCommand());
+        ASSERT(lpq->isFromFindCommand());
     }
 
     TEST(LiteParsedQueryTest, ParseCommandNotFromFindCommand) {
@@ -761,24 +835,35 @@ namespace {
                                             BSONObj(),
                                             false,      // snapshot
                                             false)));   // explain
-        ASSERT(!lpq->fromFindCommand());
+        ASSERT(!lpq->isFromFindCommand());
     }
 
     TEST(LiteParsedQueryTest, ParseCommandAwaitDataButNotTailable) {
+        const NamespaceString nss("test.testns");
         BSONObj cmdObj = fromjson("{find: 'testns', awaitData: true}");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
+    }
+
+    TEST(LiteParsedQueryTest, ParseCommandFirstFieldNotString) {
+        BSONObj cmdObj = fromjson("{find: 1}");
+        const NamespaceString nss("test.testns");
+        bool isExplain = false;
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, DefaultQueryParametersCorrect) {
         BSONObj cmdObj = fromjson("{find: 'testns'}");
 
+        const NamespaceString nss("test.testns");
         std::unique_ptr<LiteParsedQuery> lpq(
-            assertGet(LiteParsedQuery::fromFindCommand("testns", cmdObj, false)));
+            assertGet(LiteParsedQuery::makeFromFindCommand(nss, cmdObj, false)));
 
         ASSERT_EQUALS(0, lpq->getSkip());
         ASSERT_EQUALS(true, lpq->wantMore());
-        ASSERT_EQUALS(true, lpq->fromFindCommand());
+        ASSERT_EQUALS(true, lpq->isFromFindCommand());
         ASSERT_EQUALS(false, lpq->isExplain());
         ASSERT_EQUALS(0, lpq->getMaxScan());
         ASSERT_EQUALS(0, lpq->getMaxTimeMS());
@@ -803,16 +888,20 @@ namespace {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "snapshot: true,"
                                    "foo: {a: 1}}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
     TEST(LiteParsedQueryTest, ParseFromCommandForbidExtraOption) {
         BSONObj cmdObj = fromjson("{find: 'testns',"
                                    "snapshot: true,"
                                    "foo: true}");
+        const NamespaceString nss("test.testns");
         bool isExplain = false;
-        ASSERT_NOT_OK(LiteParsedQuery::fromFindCommand("testns", cmdObj, isExplain).getStatus());
+        auto result = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
+        ASSERT_NOT_OK(result.getStatus());
     }
 
 } // namespace mongo
diff --git a/src/mongo/db/query/planner_analysis.cpp b/src/mongo/db/query/planner_analysis.cpp
index f8d1fb3ce3a..eee3093b98a 100644
--- a/src/mongo/db/query/planner_analysis.cpp
+++ b/src/mongo/db/query/planner_analysis.cpp
@@ -475,7 +475,7 @@ namespace mongo {
             // We have a true limit. The limit can be combined with the SORT stage.
             sort->limit = static_cast<size_t>(*lpq.getLimit()) + static_cast<size_t>(lpq.getSkip());
         }
-        else if (!lpq.fromFindCommand() && lpq.getBatchSize()) {
+        else if (!lpq.isFromFindCommand() && lpq.getBatchSize()) {
             // We have an ntoreturn specified by an OP_QUERY style find. This is used
             // by clients to mean both batchSize and limit.
             //
@@ -754,7 +754,7 @@ namespace mongo {
                 limit->children.push_back(solnRoot);
                 solnRoot = limit;
             }
-            else if (!lpq.fromFindCommand() && lpq.getBatchSize() && !lpq.wantMore()) {
+            else if (!lpq.isFromFindCommand() && lpq.getBatchSize() && !lpq.wantMore()) {
                 // We have a "legacy limit", i.e. a negative ntoreturn value from an OP_QUERY style
                 // find.
                 LimitNode* limit = new LimitNode();
diff --git a/src/mongo/db/query/query_planner_test_fixture.cpp b/src/mongo/db/query/query_planner_test_fixture.cpp
index 0027454f630..c6824169ae7 100644
--- a/src/mongo/db/query/query_planner_test_fixture.cpp
+++ b/src/mongo/db/query/query_planner_test_fixture.cpp
@@ -32,6 +32,7 @@
 
 #include "mongo/db/query/query_planner_test_fixture.h"
 
+#include "mongo/db/namespace_string.h"
 #include "mongo/db/matcher/expression_parser.h"
 #include "mongo/db/query/query_knobs.h"
 #include "mongo/db/query/query_planner.h"
@@ -42,7 +43,7 @@ namespace mongo {
 
     using unittest::assertGet;
 
-    const char* QueryPlannerTest::ns = "somebogusns";
+    const char* QueryPlannerTest::ns = "somebogus.ns";
 
     void QueryPlannerTest::setUp() {
         internalQueryPlannerEnableHashIntersection = true;
@@ -236,9 +237,12 @@ namespace mongo {
     void QueryPlannerTest::runQueryAsCommand(const BSONObj& cmdObj) {
         solns.clear();
 
+        const NamespaceString nss(ns);
+        invariant(nss.isValid());
+
         const bool isExplain = false;
         std::unique_ptr<LiteParsedQuery> lpq(
-            assertGet(LiteParsedQuery::fromFindCommand(ns, cmdObj, isExplain)));
+            assertGet(LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain)));
 
         CanonicalQuery* rawCq;
         WhereCallbackNoop whereCallback;
diff --git a/src/mongo/s/catalog/replset/catalog_manager_replica_set_test.cpp b/src/mongo/s/catalog/replset/catalog_manager_replica_set_test.cpp
index 04cdfcd940b..956f6248551 100644
--- a/src/mongo/s/catalog/replset/catalog_manager_replica_set_test.cpp
+++ b/src/mongo/s/catalog/replset/catalog_manager_replica_set_test.cpp
@@ -84,7 +84,8 @@ namespace {
             const NamespaceString nss(request.dbname, request.cmdObj.firstElement().String());
             ASSERT_EQ(nss.toString(), CollectionType::ConfigNS);
 
-            auto query = assertGet(LiteParsedQuery::fromFindCommand(nss, request.cmdObj, false));
+            auto query = assertGet(
+                LiteParsedQuery::makeFromFindCommand(nss, request.cmdObj, false));
 
             // Ensure the query is correct
             ASSERT_EQ(query->ns(), CollectionType::ConfigNS);
@@ -134,7 +135,8 @@ namespace {
             const NamespaceString nss(request.dbname, request.cmdObj.firstElement().String());
             ASSERT_EQ(nss.toString(), DatabaseType::ConfigNS);
 
-            auto query = assertGet(LiteParsedQuery::fromFindCommand(nss, request.cmdObj, false));
+            auto query = assertGet(
+                LiteParsedQuery::makeFromFindCommand(nss, request.cmdObj, false));
 
             ASSERT_EQ(query->ns(), DatabaseType::ConfigNS);
             ASSERT_EQ(query->getFilter(), BSON(DatabaseType::name(expectedDb.getName())));
@@ -337,7 +339,8 @@ namespace {
             const NamespaceString nss(request.dbname, request.cmdObj.firstElement().String());
             ASSERT_EQ(nss.toString(), ShardType::ConfigNS);
 
-            auto query = assertGet(LiteParsedQuery::fromFindCommand(nss, request.cmdObj, false));
+            auto query = assertGet(
+                LiteParsedQuery::makeFromFindCommand(nss, request.cmdObj, false));
 
             ASSERT_EQ(query->ns(), ShardType::ConfigNS);
             ASSERT_EQ(query->getFilter(), BSONObj());
@@ -370,7 +373,8 @@ namespace {
             const NamespaceString nss(request.dbname, request.cmdObj.firstElement().String());
             ASSERT_EQ(nss.toString(), ShardType::ConfigNS);
 
-            auto query = assertGet(LiteParsedQuery::fromFindCommand(nss, request.cmdObj, false));
+            auto query = assertGet(
+                LiteParsedQuery::makeFromFindCommand(nss, request.cmdObj, false));
 
             ASSERT_EQ(query->ns(), ShardType::ConfigNS);
             ASSERT_EQ(query->getFilter(), BSONObj());
@@ -432,7 +436,8 @@ namespace {
             const NamespaceString nss(request.dbname, request.cmdObj.firstElement().String());
             ASSERT_EQ(nss.toString(), ChunkType::ConfigNS);
 
-            auto query = assertGet(LiteParsedQuery::fromFindCommand(nss, request.cmdObj, false));
+            auto query = assertGet(
+                LiteParsedQuery::makeFromFindCommand(nss, request.cmdObj, false));
 
             ASSERT_EQ(query->ns(), ChunkType::ConfigNS);
             ASSERT_EQ(query->getFilter(), chunksQuery.getFilter());
@@ -468,7 +473,8 @@ namespace {
             const NamespaceString nss(request.dbname, request.cmdObj.firstElement().String());
             ASSERT_EQ(nss.toString(), ChunkType::ConfigNS);
 
-            auto query = assertGet(LiteParsedQuery::fromFindCommand(nss, request.cmdObj, false));
+            auto query = assertGet(
+                LiteParsedQuery::makeFromFindCommand(nss, request.cmdObj, false));
 
             ASSERT_EQ(query->ns(), ChunkType::ConfigNS);
             ASSERT_EQ(query->getFilter(), chunksQuery.getFilter());
@@ -503,7 +509,8 @@ namespace {
             const NamespaceString nss(request.dbname, request.cmdObj.firstElement().String());
             ASSERT_EQ(nss.toString(), ChunkType::ConfigNS);
 
-            auto query = assertGet(LiteParsedQuery::fromFindCommand(nss, request.cmdObj, false));
+            auto query = assertGet(
+                LiteParsedQuery::makeFromFindCommand(nss, request.cmdObj, false));
 
             ASSERT_EQ(query->ns(), ChunkType::ConfigNS);
             ASSERT_EQ(query->getFilter(), chunksQuery.getFilter());
diff --git a/src/mongo/s/commands/cluster_find_cmd.cpp b/src/mongo/s/commands/cluster_find_cmd.cpp
index 850d2612477..f85b65fe39e 100644
--- a/src/mongo/s/commands/cluster_find_cmd.cpp
+++ b/src/mongo/s/commands/cluster_find_cmd.cpp
@@ -90,10 +90,15 @@ namespace {
                                BSONObjBuilder* out) const {
 
             const string fullns = parseNs(dbname, cmdObj);
+            const NamespaceString nss(fullns);
+            if (!nss.isValid()) {
+                return {ErrorCodes::InvalidNamespace,
+                        str::stream() << "Invalid collection name: " << nss.ns()};
+            }
 
             // Parse the command BSON to a LiteParsedQuery.
             bool isExplain = true;
-            auto lpqStatus = LiteParsedQuery::fromFindCommand(fullns, cmdObj, isExplain);
+            auto lpqStatus = LiteParsedQuery::makeFromFindCommand(nss, cmdObj, isExplain);
             if (!lpqStatus.isOK()) {
                 return lpqStatus.getStatus();
             }