SERVER-17544 getMore request parsing now fails if there is an unrecognized field name

2 files changed, 63 insertions, 30 deletions

diff --git a/src/mongo/db/query/getmore_request.cpp b/src/mongo/db/query/getmore_request.cpp
index e1e6d4d3d24..8f58164623f 100644
--- a/src/mongo/db/query/getmore_request.cpp
+++ b/src/mongo/db/query/getmore_request.cpp
@@ -32,6 +32,10 @@
 
 #include "mongo/db/query/getmore_request.h"
 
+#include <boost/optional.hpp>
+
+#include "mongo/util/stringutils.h"
+
 namespace mongo {
 
     const int GetMoreRequest::kDefaultBatchSize = 101;
@@ -76,42 +80,64 @@ namespace mongo {
     // static
     StatusWith<GetMoreRequest> GetMoreRequest::parseFromBSON(const std::string& dbname,
                                                              const BSONObj& cmdObj) {
-        if (!str::equals(cmdObj.firstElementFieldName(), "getMore")) {
-            return StatusWith<GetMoreRequest>(ErrorCodes::FailedToParse, str::stream()
-                << "First field name must be 'getMore' in: " << cmdObj);
-        }
+        // Required fields.
+        boost::optional<CursorId> cursorid;
+        boost::optional<std::string> fullns;
 
-        BSONElement cursorIdElt = cmdObj.firstElement();
-        if (cursorIdElt.type() != BSONType::NumberLong) {
-            return StatusWith<GetMoreRequest>(ErrorCodes::TypeMismatch, str::stream()
-                << "Field 'getMore' must be of type long in: " << cmdObj);
-        }
-        const CursorId cursorid = cursorIdElt.Long();
+        // Optional field, set to its default.
+        int batchSize = kDefaultBatchSize;
 
-        BSONElement collElt = cmdObj["collection"];
-        if (collElt.type() != BSONType::String) {
-            return StatusWith<GetMoreRequest>(ErrorCodes::TypeMismatch, str::stream()
-                << "Field 'collection' must be of type string in: " << cmdObj);
+        for (BSONElement el : cmdObj) {
+            const char* fieldName = el.fieldName();
+            if (str::equals(fieldName, "getMore")) {
+                if (el.type() != BSONType::NumberLong) {
+                    return {ErrorCodes::TypeMismatch,
+                            str::stream() << "Field 'getMore' must be of type long in: " << cmdObj};
+                }
+
+                cursorid = el.Long();
+            }
+            else if (str::equals(fieldName, "collection")) {
+                if (el.type() != BSONType::String) {
+                    return {ErrorCodes::TypeMismatch,
+                            str::stream() << "Field 'collection' must be of type string in: "
+                                          << cmdObj};
+                }
+
+                fullns = parseNs(dbname, cmdObj);
+            }
+            else if (str::equals(fieldName, "batchSize")) {
+                if (!el.isNumber()) {
+                    return {ErrorCodes::TypeMismatch,
+                            str::stream() << "Field 'batchSize' must be a number in: " << cmdObj};
+                }
+
+                batchSize = el.numberInt();
+            }
+            else if (!str::startsWith(fieldName, "$")) {
+                return {ErrorCodes::FailedToParse,
+                        str::stream() << "Failed to parse: " << cmdObj << ". "
+                                      << "Unrecognized field '" << fieldName << "'."};
+            }
         }
-        const std::string fullns = parseNs(dbname, cmdObj);
 
-        int batchSize = kDefaultBatchSize;
-        BSONElement batchSizeElt = cmdObj["batchSize"];
-        if (batchSizeElt.type() != BSONType::NumberInt && !batchSizeElt.eoo()) {
-            return StatusWith<GetMoreRequest>(ErrorCodes::TypeMismatch, str::stream()
-                << "Field 'batchSize' must be of type int in: " << cmdObj);
+        if (!cursorid) {
+            return {ErrorCodes::FailedToParse,
+                    str::stream() << "Field 'getMore' missing in: " << cmdObj};
         }
-        else if (!batchSizeElt.eoo()) {
-            batchSize = batchSizeElt.Int();
+
+        if (!fullns) {
+            return {ErrorCodes::FailedToParse,
+                    str::stream() << "Field 'collection' missing in: " << cmdObj};
         }
 
-        GetMoreRequest request(fullns, cursorid, batchSize);
+        GetMoreRequest request(*fullns, *cursorid, batchSize);
         Status validStatus = request.isValid();
         if (!validStatus.isOK()) {
-            return StatusWith<GetMoreRequest>(validStatus);
+            return validStatus;
         }
 
-        return StatusWith<GetMoreRequest>(request);
+        return request;
     }
 
 } // namespace mongo
diff --git a/src/mongo/db/query/getmore_request_test.cpp b/src/mongo/db/query/getmore_request_test.cpp
index 757d4e86193..86d65e5cf33 100644
--- a/src/mongo/db/query/getmore_request_test.cpp
+++ b/src/mongo/db/query/getmore_request_test.cpp
@@ -68,7 +68,7 @@ namespace {
             "db",
             BSON("getMore" << CursorId(123)));
         ASSERT_NOT_OK(result.getStatus());
-        ASSERT_EQUALS(ErrorCodes::TypeMismatch, result.getStatus().code());
+        ASSERT_EQUALS(ErrorCodes::FailedToParse, result.getStatus().code());
     }
 
     TEST(GetMoreRequestTest, parseFromBSONCollectionNotString) {
@@ -121,10 +121,8 @@ namespace {
             BSON("getMore" << CursorId(123) <<
                  "collection" << "coll" <<
                  "unknown_field" << 1));
-        ASSERT_OK(result.getStatus());
-        ASSERT_EQUALS("db.coll", result.getValue().nss.toString());
-        ASSERT_EQUALS(CursorId(123), result.getValue().cursorid);
-        ASSERT_EQUALS(GetMoreRequest::kDefaultBatchSize, result.getValue().batchSize);
+        ASSERT_NOT_OK(result.getStatus());
+        ASSERT_EQUALS(ErrorCodes::FailedToParse, result.getStatus().code());
     }
 
     TEST(GetMoreRequestTest, parseFromBSONInvalidBatchSize) {
@@ -154,4 +152,13 @@ namespace {
         ASSERT_EQUALS(200, result.getValue().batchSize);
     }
 
+    TEST(GetMoreRequestTest, parseFromBSONIgnoreDollarPrefixedFields) {
+        StatusWith<GetMoreRequest> result = GetMoreRequest::parseFromBSON(
+            "db",
+            BSON("getMore" << CursorId(123) << "collection" << "coll" << "$foo" << "bar"));
+        ASSERT_OK(result.getStatus());
+        ASSERT_EQUALS("db.coll", result.getValue().nss.toString());
+        ASSERT_EQUALS(CursorId(123), result.getValue().cursorid);
+    }
+
 } // namespace