SERVER-6669 SERVER-4713 Validate that a positional match is found bef…

…ore applying an update mod with the positional operator.

jstests/multiVersion/multi_version_sharding_passthrough.js

@@ -319,6 +319,7 @@ var v22Only = [ /^all3$/,
                 /^updatei$/,
                 /^updatej$/,
                 /^updatek$/,
+                /^updatel$/,
                 /^use_power_of_2$/,
                 /^useindexonobjgtlt$/,
                 /^where4$/,

jstests/updatel.js

@@ -0,0 +1,48 @@
+// The positional operator allows an update modifier field path to contain a sentinel ('$') path
+// part that is replaced with the numeric position of an array element matched by the update's query
+// spec.  <http://docs.mongodb.org/manual/reference/operators/#_S_>
+
+// If no array element position from a query is available to substitute for the positional operator
+// setinel ('$'), the update fails with an error.  SERVER-6669 SERVER-4713
+
+t = db.jstests_updatel;
+t.drop();
+
+
+
+// The collection is empty, forcing an upsert.  In this case the query has no array position match
+// to substiture for the positional operator.  SERVER-4713
+t.update( {}, { $set:{ 'a.$.b':1 } }, true );
+assert( db.getLastError(), "An error is reported." );
+assert.eq( 0, t.count(), "No upsert occurred." );
+
+
+
+// Save a document to the collection so it is no longer empty.
+t.save( { _id:0 } );
+
+// Now, with an existing document, trigger an update rather than an upsert.  The query has no array
+// position match to substiture for the positional operator.  SERVER-6669
+t.update( {}, { $set:{ 'a.$.b':1 } } );
+assert( db.getLastError(), "An error is reported." );
+assert.eq( [ { _id:0 } ], t.find().toArray(), "No update occurred." );
+
+
+
+// Now, try with an update by _id (without a query array match).
+t.update( { _id:0 }, { $set:{ 'a.$.b':1 } } );
+assert( db.getLastError(), "An error is reported." );
+assert.eq( [ { _id:0 } ], t.find().toArray(), "No update occurred." );
+
+
+
+// Seed the collection with a document suitable for the following check.
+t.remove();
+t.save( { _id:0, a:[ { b:{ c:1 } } ] } );
+
+// Now, attempt to apply an update with two nested positional operators.  There is a positional
+// query match for the first positional operator but not the second.  Note that dollar sign
+// substitution for multiple positional opertors is not implemented (SERVER-831).
+t.update( { 'a.b.c':1 }, { $set:{ 'a.$.b.$.c':2 } } );
+assert( db.getLastError(), "An error is reported" );
+assert.eq( [ { _id:0, a:[ { b:{ c:1 } } ] } ], t.find().toArray(), "No update occurred." );

src/mongo/SConscript

@@ -540,6 +540,7 @@ env.StaticLibrary("serveronly", serverOnlyFiles,
                   LIBDEPS=["coreshard",
                            "db/auth/authmongod",
                            "db/fts/ftsmongod",
+                           "db/ops/update",
                            "dbcmdline",
                            "defaultversion",
                            "geoparser",

src/mongo/db/ops/update.cpp

@@ -268,10 +268,6 @@ namespace mongo {
                 debug.nscanned++;
 
                 if ( mods.get() && mods->hasDynamicArray() ) {
-                    // The Cursor must have a Matcher to record an elemMatchKey.  But currently
-                    // a modifier on a dynamic array field may be applied even if there is no
-                    // elemMatchKey, so a matcher cannot be required.
-                    //verify( c->matcher() );
                     details.requestElemMatchKey();
                 }
 

src/mongo/db/ops/update_internal.cpp

@@ -21,6 +21,7 @@
 #include <algorithm> // for max
 
 #include "mongo/db/oplog.h"
+#include "mongo/db/ops/field_ref.h"
 #include "mongo/db/jsobjmanipulator.h"
 #include "mongo/db/pdfile.h"
 #include "mongo/util/mongoutils/str.h"
@@ -539,6 +540,24 @@ namespace mongo {
             ModState& ms = *mss->_mods[i->first];
 
             const Mod& m = i->second;
+
+            // Check for any positional operators that have not been replaced with a numeric field
+            // name (from a query match element).
+            // Only perform this positional operator validation in 'strictApply' mode.  When
+            // replicating from a legacy primary that does not implement this validation, the
+            // secondary bypasses validation and remains consistent with the primary.
+            if ( m.strictApply ) {
+                FieldRef fieldRef;
+                fieldRef.parse( m.fieldName );
+                StringData positionalOpField( "$" );
+                for( size_t i = 0; i < fieldRef.numParts(); ++i ) {
+                     uassert( 16650,
+                              "Cannot apply the positional operator without a corresponding query "
+                              "field containing an array.",
+                              fieldRef.getPart( i ).compare( positionalOpField ) != 0 );
+                }
+            }
+
             BSONElement e = obj.getFieldDotted(m.fieldName);
 
             ms.m = &m;

src/mongo/dbtests/updatetests.cpp

@@ -2157,6 +2157,77 @@ namespace UpdateTests {
                                modSetState->getOpLogRewrite() );
             }
         };
+
+        class PositionalWithoutElemMatchKey {
+        public:
+            void run() {
+                BSONObj querySpec = BSONObj();
+                BSONObj modSpec = BSON( "$set" << BSON( "a.$" << 1 ) );
+                ModSet modSet( modSpec );
+
+                // A positional operator must be replaced with an array index before calling
+                // prepare().
+                ASSERT_THROWS( modSet.prepare( querySpec ), UserException );
+            }
+        };
+
+        class PositionalWithoutNestedElemMatchKey {
+        public:
+            void run() {
+                BSONObj querySpec = BSONObj();
+                BSONObj modSpec = BSON( "$set" << BSON( "a.b.c.$.e.f" << 1 ) );
+                ModSet modSet( modSpec );
+
+                // A positional operator must be replaced with an array index before calling
+                // prepare().
+                ASSERT_THROWS( modSet.prepare( querySpec ), UserException );
+            }
+        };
+
+        class DbrefPassesPositionalValidation {
+        public:
+            void run() {
+                BSONObj querySpec = BSONObj();
+                BSONObj modSpec = BSON( "$set" << BSON( "a.$ref" << "foo" << "a.$id" << 0 ) );
+                ModSet modSet( modSpec );
+
+                // A positional operator must be replaced with an array index before calling
+                // prepare(), but $ prefixed fields encoding dbrefs are allowed.
+                modSet.prepare( querySpec ); // Does not throw.
+            }
+        };
+
+        class NoPositionalValidationOnReplication {
+        public:
+            void run() {
+                BSONObj querySpec = BSONObj();
+                BSONObj modSpec = BSON( "$set" << BSON( "a.$" << 1 ) );
+                ModSet modSet( modSpec, set<string>(), NULL, true );
+
+                // No positional operator validation is performed if a ModSet is 'forReplication'.
+                modSet.prepare( querySpec ); // Does not throw.
+            }
+        };
+
+        class NoPositionalValidationOnPartialFixedArrayReplication {
+        public:
+            void run() {
+                BSONObj querySpec = BSONObj( BSON( "a.b" << 1 ) );
+                BSONObj modSpec = BSON( "$set" << BSON( "a.$.b.$" << 1 ) );
+                ModSet modSet( modSpec, set<string>(), NULL, true );
+
+                // Attempt to fix the positional operator fields.
+                scoped_ptr<ModSet> fixedMods( modSet.fixDynamicArray( "0" ) );
+
+                // The first positional field is replaced, but the second is not (until SERVER-831
+                // is implemented).
+                ASSERT( fixedMods->haveModForField( "a.0.b.$" ) );
+
+                // No positional operator validation is performed if a ModSet is 'forReplication',
+                // even after an attempt to fix the positional operator fields.
+                fixedMods->prepare( querySpec ); // Does not throw.
+            }
+        };
     };
 
     namespace basic {
@@ -2510,6 +2581,11 @@ namespace UpdateTests {
             // add< ModSetTests::BitRewriteNonExistingField >();
             add< ModSetTests::SetIsNotRewritten >();
             add< ModSetTests::UnsetIsNotRewritten >();
+            add< ModSetTests::PositionalWithoutElemMatchKey >();
+            add< ModSetTests::PositionalWithoutNestedElemMatchKey >();
+            add< ModSetTests::DbrefPassesPositionalValidation >();
+            add< ModSetTests::NoPositionalValidationOnReplication >();
+            add< ModSetTests::NoPositionalValidationOnPartialFixedArrayReplication >();
 
             add< basic::inc1 >();
             add< basic::inc2 >();