BulkWrite operations fails on references with extra fields

[alcaeus]

This was previously reported in alcaeus/mongo-php-adapter#101. Doing a BulkWrite update with certain documents causes the following warning: mongoc: WARNING > mongoc_bulk_operation_replace_one(): replacement document may not contain $ or . in keys. Ignoring document..

Looking into the error I've found that this occurs when doing a bulk write containing a reference that has an extra field before $ref:

$filter = ['x' => 11];
$replacement = ['test' => ['type' => 'test', '$ref' => 'collection', '$id' => 2]];

$operation = new ReplaceOne($this->getDatabaseName(), $this->getCollectionName(), $filter, $replacement);

This error does not occur when passing the extra field in the DBRef document at the end, e.g. ['$ref' => 'collection', '$id' => 2, 'type' => 'test'].

On the other hand, doing the very same thing in an update operation does not cause this error, the update goes through just fine. I'll be adding a pull request containing three tests for the scenarios: replace with an extra field after the reference, replace with an extra field before the reference and an update with an extra field before the reference.

[jmikola]

Per the DBRef documentation:

The order of fields in the DBRef matters, and you must use the above sequence when using a DBRef.

Also, there is this definition from SERVER-12263:

DBRefs are objects whose first two fields must be $ref and $id (in that order). An optional $db field, if present, must appear third. Other fields may follow (they can't have a $ prefix, of course).

That said, I'm not sure why the server seems to allow this:

> db.foo.insert({_id:1})
WriteResult({ "nInserted" : 1 })
> db.foo.update({_id:1}, {$set:{ref:{a:"a","$ref":"b","$id":"c"}}})
WriteResult({ "nMatched" : 1, "nUpserted" : 0, "nModified" : 1 })
> db.foo.find()
{ "_id" : 1, "ref" : { "a" : "a", "$ref" : "b", "$id" : "c" } }
> db.foo.update({_id:1}, {$set:{ref:{"$ref":"b","$id":"c","a":"a"}}})
WriteResult({ "nMatched" : 1, "nUpserted" : 0, "nModified" : 1 })
> db.foo.find()
{ "_id" : 1, "ref" : DBRef("b", "c") }

The exception is most certainly due to the C driver's bson_validate() function, which walks a document before serializing it for a CRUD method. When validating replacement documents, we disallow any $-prefixed keys; however, an allowance is made for the DBRef keys (in their proper order). IMO, the validation is correct and drivers/applications should not be creating these malformed DBRef documents.

Is ODM creating these out-of-order values?

[jwage]

We create references with https://github.com/doctrine/mongodb-odm/blob/master/lib/Doctrine/ODM/MongoDB/DocumentManager.php#L667

We create them in order as far as I can tell.

[jwage]

It appears that it is actually our test that is broken. We have a test where the reference is manually created and it is created out of order.

[alcaeus]

Is ODM creating these out-of-order values?

From what I can tell, ODM creates them in the correct order, also mongo-php-adapter just loops over the document for conversion purposes, so no re-ordering should occur there.

IMO, the validation is correct and drivers/applications should not be creating these malformed DBRef documents.

Reading the DBRef documentation, this is indeed the case but seems to not be applied to documents being inserted or updated with update operators. If the exception occurs on replace, it most definitely should also occur on inserts and updates as well.

[jmikola]

Based on @jwage's response above, this was caused by a test creating an invalid DBRef document. It's unfortunate that the server doesn't consider such documents invalid for insert/update, but I don't fault the driver here. This was only picked up because we need to check that BSON documents do not contain $-prefixed keys for replacement arguments (to comply with our CRUD specification). I've opened SERVER-23786 to request that the server start checking for this.

[alcaeus]

Great, thanks @jmikola!