SERVER-27570: Enforce stricter checks on top level command BSON objects

(cherry picked from commit 957549cd11a34e0c92c9996eb564a6e2d6ada58a)

7 files changed, 173 insertions, 33 deletions

diff --git a/buildscripts/resmokeconfig/suites/integration_tests_sharded.yml b/buildscripts/resmokeconfig/suites/integration_tests_sharded.yml
new file mode 100644
index 00000000000..db7bc2b0c15
--- /dev/null
+++ b/buildscripts/resmokeconfig/suites/integration_tests_sharded.yml
@@ -0,0 +1,22 @@
+selector:
+  cpp_integration_test:
+    root: build/integration_tests.txt
+    exclude_files:
+        - build/integration_tests/network_interface_asio_integration_test*
+
+executor:
+  cpp_integration_test:
+    config: {}
+    hooks:
+    - class: ValidateCollections
+    fixture:
+      class: ShardedClusterFixture
+      mongod_options:
+        set_parameters:
+          enableTestCommands: 1
+          numInitialSyncAttempts: 1
+      mongos_options:
+        set_parameters:
+          enableTestCommands: 1
+      enable_sharding:
+      - test
diff --git a/buildscripts/resmokelib/testing/fixtures/shardedcluster.py b/buildscripts/resmokelib/testing/fixtures/shardedcluster.py
index 67f0bbb9f86..3420a4c354a 100644
--- a/buildscripts/resmokelib/testing/fixtures/shardedcluster.py
+++ b/buildscripts/resmokelib/testing/fixtures/shardedcluster.py
@@ -170,6 +170,12 @@ class ShardedClusterFixture(interface.Fixture):
                 all(shard.is_running() for shard in self.shards) and
                 self.mongos is not None and self.mongos.is_running())
 
+    def get_connection_string(self):
+        if self.mongos is None:
+            raise ValueError("Must call setup() before calling get_connection_string()")
+
+        return "%s:%d" % (socket.gethostname(), self.mongos.port)
+
     def _new_configsvr(self):
         """
         Returns a replicaset.ReplicaSetFixture configured to be used as
diff --git a/etc/evergreen.yml b/etc/evergreen.yml
index df4300ce973..17e8d3cd07b 100644
--- a/etc/evergreen.yml
+++ b/etc/evergreen.yml
@@ -1824,6 +1824,15 @@ tasks:
       run_multiple_jobs: true
 
 - <<: *task_template
+  name: integration_tests_sharded
+  commands:
+  - func: "do setup"
+  - func: "run tests"
+    vars:
+      resmoke_args: --suites=integration_tests_sharded
+      run_multiple_jobs: true
+
+- <<: *task_template
   name: external_auth_WT
   commands:
   - func: "do setup"
@@ -3348,6 +3357,7 @@ buildvariants:
   - name: httpinterface
   - name: integration_tests_standalone
   - name: integration_tests_replset
+  - name: integration_tests_sharded
   - name: sharding_gle_auth_basics_passthrough
   - name: sharding_gle_auth_basics_passthrough_WT
   - name: sharding_gle_auth_basics_passthrough_write_cmd
@@ -3448,6 +3458,7 @@ buildvariants:
   - name: dbtest_WT
   - name: integration_tests_standalone
   - name: integration_tests_replset
+  - name: integration_tests_sharded
   - name: jsCore
   - name: jsCore_WT
   - name: parallel_WT
@@ -4355,6 +4366,7 @@ buildvariants:
   - name: gle_auth_write_cmd_WT
   - name: httpinterface
   - name: integration_tests_replset
+  - name: integration_tests_sharded
   - name: integration_tests_standalone
   - name: jsCore_WT
   - name: jsCore_WT_ese
@@ -4867,6 +4879,7 @@ buildvariants:
   - name: httpinterface
   - name: integration_tests_standalone
   - name: integration_tests_replset
+  - name: integration_tests_sharded
   - name: jsCore
   - name: jsCore_WT
   - name: jsCore_compatibility
@@ -5576,6 +5589,7 @@ buildvariants:
   - name: gle_auth_write_cmd_WT
   - name: httpinterface
   - name: integration_tests_replset
+  - name: integration_tests_sharded
   - name: integration_tests_standalone
   - name: jsCore
   - name: jsCore_WT
@@ -5808,6 +5822,7 @@ buildvariants:
   - name: gle_auth_write_cmd_WT
   - name: httpinterface
   - name: integration_tests_replset
+  - name: integration_tests_sharded
   - name: integration_tests_standalone
   - name: jsCore
   - name: jsCore_WT
@@ -6322,6 +6337,7 @@ buildvariants:
   - name: gle_auth_write_cmd_WT
   - name: httpinterface
   - name: integration_tests_replset
+  - name: integration_tests_sharded
   - name: integration_tests_standalone
   # - name: jsCore
   - name: jsCore_WT
@@ -6457,6 +6473,7 @@ buildvariants:
   - name: gle_auth_write_cmd_WT
   - name: httpinterface
   - name: integration_tests_replset
+  - name: integration_tests_sharded
   - name: integration_tests_standalone
   # - name: jsCore
   - name: jsCore_WT
@@ -6975,6 +6992,7 @@ buildvariants:
   - name: gle_auth_write_cmd_WT
   - name: httpinterface
   - name: integration_tests_replset
+  - name: integration_tests_sharded
   - name: integration_tests_standalone
   - name: jsCore_WT
   - name: jsCore_WT_ese
@@ -7239,6 +7257,7 @@ buildvariants:
   - name: gle_auth_write_cmd_WT
   - name: httpinterface
   - name: integration_tests_replset
+  - name: integration_tests_sharded
   - name: integration_tests_standalone
   - name: jsCore_WT
   - name: jsCore_WT_ese
@@ -7896,6 +7915,7 @@ buildvariants:
   - name: gle_auth_basics_passthrough_write_cmd
   - name: gle_auth_write_cmd
   - name: integration_tests_replset
+  - name: integration_tests_sharded
   - name: integration_tests_standalone
   - name: jsCore
   - name: jsCore_WT
@@ -7976,6 +7996,7 @@ buildvariants:
   - name: httpinterface
   - name: integration_tests_standalone
   - name: integration_tests_replset
+  - name: integration_tests_sharded
   - name: sharding_gle_auth_basics_passthrough
   - name: sharding_gle_auth_basics_passthrough_write_cmd
   - name: jsCore
@@ -8058,6 +8079,7 @@ buildvariants:
   - name: gle_auth_basics_passthrough_write_cmd
   - name: gle_auth_write_cmd
   - name: integration_tests_replset
+  - name: integration_tests_sharded
   - name: integration_tests_standalone
   - name: jsCore
   - name: jsCore_WT
@@ -8144,6 +8166,7 @@ buildvariants:
   - name: gle_auth_basics_passthrough_write_cmd
   - name: gle_auth_write_cmd
   - name: integration_tests_replset
+  - name: integration_tests_sharded
   - name: integration_tests_standalone
   - name: jsCore
   - name: jsCore_WT
@@ -8315,6 +8338,7 @@ buildvariants:
   - name: gle_auth_write_cmd_WT
   - name: httpinterface
   - name: integration_tests_replset
+  - name: integration_tests_sharded
   - name: integration_tests_standalone
   - name: jsCore
   - name: jsCore_WT
@@ -8489,6 +8513,7 @@ buildvariants:
   - name: gle_auth_write_cmd_WT
   - name: httpinterface
   - name: integration_tests_replset
+  - name: integration_tests_sharded
   - name: integration_tests_standalone
   - name: jsCore
   - name: jsCore_WT
diff --git a/src/mongo/bson/SConscript b/src/mongo/bson/SConscript
index d2f20e3fc1c..f983418ea01 100644
--- a/src/mongo/bson/SConscript
+++ b/src/mongo/bson/SConscript
@@ -68,3 +68,16 @@ env.CppUnitTest(
         '$BUILD_DIR/mongo/base',
     ],
 )
+
+asioEnv = env.Clone()
+asioEnv.InjectThirdPartyIncludePaths('asio')
+
+asioEnv.CppIntegrationTest(
+    target='ugly_bson_integration_test',
+    source=[
+        'ugly_bson_integration_test.cpp'
+    ],
+    LIBDEPS=[
+        '$BUILD_DIR/mongo/executor/network_interface_asio_fixture',
+    ],
+)
diff --git a/src/mongo/bson/ugly_bson_integration_test.cpp b/src/mongo/bson/ugly_bson_integration_test.cpp
new file mode 100644
index 00000000000..aec92665824
--- /dev/null
+++ b/src/mongo/bson/ugly_bson_integration_test.cpp
@@ -0,0 +1,63 @@
+/**
+ *    Copyright (C) 2017 MongoDB Inc.
+ *
+ *    This program is free software: you can redistribute it and/or  modify
+ *    it under the terms of the GNU Affero General Public License, version 3,
+ *    as published by the Free Software Foundation.
+ *
+ *    This program is distributed in the hope that it will be useful,
+ *    but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *    GNU Affero General Public License for more details.
+ *
+ *    You should have received a copy of the GNU Affero General Public License
+ *    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ *    As a special exception, the copyright holders give permission to link the
+ *    code of portions of this program with the OpenSSL library under certain
+ *    conditions as described in each individual source file and distribute
+ *    linked combinations including the program with the OpenSSL library. You
+ *    must comply with the GNU Affero General Public License in all respects for
+ *    all of the code used other than as permitted herein. If you modify file(s)
+ *    with this exception, you may extend this exception to your version of the
+ *    file(s), but you are not obligated to do so. If you do not wish to do so,
+ *    delete this exception statement from your version. If you delete this
+ *    exception statement from all source files in the program, then also delete
+ *    it in the license file.
+ */
+
+#include "mongo/platform/basic.h"
+
+#include <algorithm>
+#include <exception>
+
+#include "mongo/client/connection_string.h"
+#include "mongo/executor/network_interface_asio_integration_fixture.h"
+#include "mongo/unittest/integration_test.h"
+#include "mongo/unittest/unittest.h"
+#include "mongo/util/assert_util.h"
+
+namespace mongo {
+namespace executor {
+namespace {
+
+class UglyBSONFixture : public NetworkInterfaceASIOIntegrationFixture {
+    void setUp() override {
+        startNet();
+    }
+};
+
+TEST_F(UglyBSONFixture, DuplicateFields) {
+    assertCommandFailsOnServer("admin",
+                               BSON("insert"
+                                    << "test"
+                                    << "documents"
+                                    << BSONArray()
+                                    << "documents"
+                                    << BSONArray()),
+                               ErrorCodes::FailedToParse);
+}
+
+}  // namespace
+}  // namespace executor
+}  // namespace mongo
diff --git a/src/mongo/db/commands/dbcommands.cpp b/src/mongo/db/commands/dbcommands.cpp
index d577aa5dde2..c282579a7f1 100644
--- a/src/mongo/db/commands/dbcommands.cpp
+++ b/src/mongo/db/commands/dbcommands.cpp
@@ -35,6 +35,7 @@
 #include <time.h>
 
 #include "mongo/base/disallow_copying.h"
+#include "mongo/base/simple_string_data_comparator.h"
 #include "mongo/base/status.h"
 #include "mongo/base/status_with.h"
 #include "mongo/bson/simple_bsonobj_comparator.h"
@@ -1222,22 +1223,6 @@ private:
     bool maintenanceModeSet;
 };
 
-namespace {
-
-// Symbolic names for indexes to make code more readable.
-const std::size_t kCmdOptionMaxTimeMSField = 0;
-const std::size_t kHelpField = 1;
-const std::size_t kShardVersionFieldIdx = 2;
-const std::size_t kQueryOptionMaxTimeMSField = 3;
-
-// We make an array of the fields we need so we can call getFields once. This saves repeated
-// scans over the command object.
-const std::array<StringData, 4> neededFieldNames{QueryRequest::cmdOptionMaxTimeMS,
-                                                 Command::kHelpFieldName,
-                                                 ChunkVersion::kShardVersionField,
-                                                 QueryRequest::queryOptionMaxTimeMS};
-}  // namespace
-
 void appendOpTimeMetadata(OperationContext* txn,
                           const rpc::RequestInterface& request,
                           BSONObjBuilder* metadataBob) {
@@ -1296,11 +1281,31 @@ void Command::execCommand(OperationContext* txn,
         std::string dbname = request.getDatabase().toString();
         unique_ptr<MaintenanceModeSetter> mmSetter;
 
-        std::array<BSONElement, std::tuple_size<decltype(neededFieldNames)>::value>
-            extractedFields{};
-        request.getCommandArgs().getFields(neededFieldNames, &extractedFields);
+        BSONElement cmdOptionMaxTimeMSField;
+        BSONElement helpField;
+        BSONElement shardVersionFieldIdx;
+        BSONElement queryOptionMaxTimeMSField;
+
+        StringMap<int> topLevelFields;
+        for (auto&& element : request.getCommandArgs()) {
+            StringData fieldName = element.fieldNameStringData();
+            if (fieldName == QueryRequest::cmdOptionMaxTimeMS) {
+                cmdOptionMaxTimeMSField = element;
+            } else if (fieldName == Command::kHelpFieldName) {
+                helpField = element;
+            } else if (fieldName == ChunkVersion::kShardVersionField) {
+                shardVersionFieldIdx = element;
+            } else if (fieldName == QueryRequest::queryOptionMaxTimeMS) {
+                queryOptionMaxTimeMSField = element;
+            }
+
+            uassert(ErrorCodes::FailedToParse,
+                    str::stream() << "Parsed command object contains duplicate top level key: "
+                                  << fieldName,
+                    topLevelFields[fieldName]++ == 0);
+        }
 
-        if (isHelpRequest(extractedFields[kHelpField])) {
+        if (Command::isHelpRequest(helpField)) {
             CurOp::get(txn)->ensureStarted();
             // We disable last-error for help requests due to SERVER-11492, because config servers
             // use help requests to determine which commands are database writes, and so must be
@@ -1367,12 +1372,11 @@ void Command::execCommand(OperationContext* txn,
         }
 
         // Handle command option maxTimeMS.
-        int maxTimeMS = uassertStatusOK(
-            QueryRequest::parseMaxTimeMS(extractedFields[kCmdOptionMaxTimeMSField]));
+        int maxTimeMS = uassertStatusOK(QueryRequest::parseMaxTimeMS(cmdOptionMaxTimeMSField));
 
         uassert(ErrorCodes::InvalidOptions,
                 "no such command option $maxTimeMs; use maxTimeMS instead",
-                extractedFields[kQueryOptionMaxTimeMSField].eoo());
+                queryOptionMaxTimeMSField.eoo());
 
         if (maxTimeMS > 0) {
             uassert(40119,
@@ -1389,10 +1393,8 @@ void Command::execCommand(OperationContext* txn,
             auto& oss = OperationShardingState::get(txn);
 
             auto commandNS = NamespaceString(command->parseNs(dbname, request.getCommandArgs()));
-            oss.initializeShardVersion(commandNS, extractedFields[kShardVersionFieldIdx]);
-
+            oss.initializeShardVersion(commandNS, shardVersionFieldIdx);
             auto shardingState = ShardingState::get(txn);
-
             if (oss.hasShardVersion()) {
                 if (serverGlobalParams.clusterRole != ClusterRole::ShardServer) {
                     uassertStatusOK(
diff --git a/src/mongo/s/s_only.cpp b/src/mongo/s/s_only.cpp
index 2fcfc4c30b2..f8f9365994e 100644
--- a/src/mongo/s/s_only.cpp
+++ b/src/mongo/s/s_only.cpp
@@ -96,13 +96,22 @@ void Command::execCommandClient(OperationContext* txn,
                                 BSONObjBuilder& result) {
     std::string dbname = nsToDatabase(ns);
 
-    if (cmdObj.getBoolField("help")) {
-        stringstream help;
-        help << "help for: " << c->getName() << " ";
-        c->help(help);
-        result.append("help", help.str());
-        appendCommandStatus(result, true, "");
-        return;
+    StringMap<int> topLevelFields;
+    for (auto&& element : cmdObj) {
+        StringData fieldName = element.fieldNameStringData();
+        if (fieldName == "help" && element.type() == Bool && element.Bool()) {
+            std::stringstream help;
+            help << "help for: " << c->getName() << " ";
+            c->help(help);
+            result.append("help", help.str());
+            Command::appendCommandStatus(result, true, "");
+            return;
+        }
+
+        uassert(ErrorCodes::FailedToParse,
+                str::stream() << "Parsed command object contains duplicate top level key: "
+                              << fieldName,
+                topLevelFields[fieldName]++ == 0);
     }
 
     Status status = checkAuthorization(c, txn, dbname, cmdObj);