diff --git a/bson/__init__.py b/bson/__init__.py index 4a511f53c..b4e6aecdc 100644 --- a/bson/__init__.py +++ b/bson/__init__.py @@ -202,7 +202,10 @@ def _get_object(data, view, position, obj_end, opts, dummy): obj = _elements_to_dict(data, view, position + 4, end, opts) position += obj_size - if "$ref" in obj: + # If DBRef validation fails, return a normal doc. + if (isinstance(obj.get('$ref'), str) and + "$id" in obj and + isinstance(obj.get('$db'), (str, type(None)))): return (DBRef(obj.pop("$ref"), obj.pop("$id", None), obj.pop("$db", None), obj), position) return obj, position diff --git a/bson/_cbsonmodule.c b/bson/_cbsonmodule.c index 2eb9e992d..67ee01c72 100644 --- a/bson/_cbsonmodule.c +++ b/bson/_cbsonmodule.c @@ -1506,6 +1506,71 @@ static PyObject* _cbson_dict_to_bson(PyObject* self, PyObject* args) { return result; } +/* + * Hook for optional decoding BSON documents to DBRef. + */ +static PyObject *_dbref_hook(PyObject* self, PyObject* value) { + struct module_state *state = GETSTATE(self); + PyObject* dbref = NULL; + PyObject* dbref_type = NULL; + PyObject* ref = NULL; + PyObject* id = NULL; + PyObject* database = NULL; + PyObject* ret = NULL; + int db_present = 0; + + /* Decoding for DBRefs */ + if (PyMapping_HasKeyString(value, "$ref") && PyMapping_HasKeyString(value, "$id")) { /* DBRef */ + ref = PyMapping_GetItemString(value, "$ref"); + /* PyMapping_GetItemString returns NULL to indicate error. */ + if (!ref) { + goto invalid; + } + id = PyMapping_GetItemString(value, "$id"); + /* PyMapping_GetItemString returns NULL to indicate error. */ + if (!id) { + goto invalid; + } + + if (PyMapping_HasKeyString(value, "$db")) { + database = PyMapping_GetItemString(value, "$db"); + if (!database) { + goto invalid; + } + db_present = 1; + } else { + database = Py_None; + Py_INCREF(database); + } + + // check types + if (!(PyUnicode_Check(ref) && (database == Py_None || PyUnicode_Check(database)))) { + ret = value; + goto invalid; + } + + PyMapping_DelItemString(value, "$ref"); + PyMapping_DelItemString(value, "$id"); + if (db_present) { + PyMapping_DelItemString(value, "$db"); + } + + if ((dbref_type = _get_object(state->DBRef, "bson.dbref", "DBRef"))) { + dbref = PyObject_CallFunctionObjArgs(dbref_type, ref, id, database, value, NULL); + Py_DECREF(value); + ret = dbref; + } + } else { + ret = value; + } +invalid: + Py_XDECREF(dbref_type); + Py_XDECREF(ref); + Py_XDECREF(id); + Py_XDECREF(database); + return ret; +} + static PyObject* get_value(PyObject* self, PyObject* name, const char* buffer, unsigned* position, unsigned char type, unsigned max, const codec_options_t* options) { @@ -1552,7 +1617,6 @@ static PyObject* get_value(PyObject* self, PyObject* name, const char* buffer, } case 3: { - PyObject* collection; uint32_t size; if (max < 4) { @@ -1585,55 +1649,10 @@ static PyObject* get_value(PyObject* self, PyObject* name, const char* buffer, goto invalid; } - /* Decoding for DBRefs */ - if (PyMapping_HasKeyString(value, "$ref")) { /* DBRef */ - PyObject* dbref = NULL; - PyObject* dbref_type; - PyObject* id; - PyObject* database; - - collection = PyMapping_GetItemString(value, "$ref"); - /* PyMapping_GetItemString returns NULL to indicate error. */ - if (!collection) { - goto invalid; - } - PyMapping_DelItemString(value, "$ref"); - - if (PyMapping_HasKeyString(value, "$id")) { - id = PyMapping_GetItemString(value, "$id"); - if (!id) { - Py_DECREF(collection); - goto invalid; - } - PyMapping_DelItemString(value, "$id"); - } else { - id = Py_None; - Py_INCREF(id); - } - - if (PyMapping_HasKeyString(value, "$db")) { - database = PyMapping_GetItemString(value, "$db"); - if (!database) { - Py_DECREF(collection); - Py_DECREF(id); - goto invalid; - } - PyMapping_DelItemString(value, "$db"); - } else { - database = Py_None; - Py_INCREF(database); - } - - if ((dbref_type = _get_object(state->DBRef, "bson.dbref", "DBRef"))) { - dbref = PyObject_CallFunctionObjArgs(dbref_type, collection, id, database, value, NULL); - Py_DECREF(dbref_type); - } - Py_DECREF(value); - value = dbref; - - Py_DECREF(id); - Py_DECREF(collection); - Py_DECREF(database); + /* Hook for DBRefs */ + value = _dbref_hook(self, value); + if (!value) { + goto invalid; } *position += size; diff --git a/bson/json_util.py b/bson/json_util.py index 5498bf044..fe0bfe069 100644 --- a/bson/json_util.py +++ b/bson/json_util.py @@ -121,9 +121,6 @@ _RE_OPT_TABLE = { "x": re.X, } -# Dollar-prefixed keys which may appear in DBRefs. -_DBREF_KEYS = frozenset(['$id', '$ref', '$db']) - class DatetimeRepresentation: LEGACY = 0 @@ -463,7 +460,9 @@ def object_pairs_hook(pairs, json_options=DEFAULT_JSON_OPTIONS): def object_hook(dct, json_options=DEFAULT_JSON_OPTIONS): if "$oid" in dct: return _parse_canonical_oid(dct) - if "$ref" in dct: + if (isinstance(dct.get('$ref'), str) and + "$id" in dct and + isinstance(dct.get('$db'), (str, type(None)))): return _parse_canonical_dbref(dct) if "$date" in dct: return _parse_canonical_datetime(dct, json_options) @@ -675,10 +674,6 @@ def _parse_canonical_regex(doc): def _parse_canonical_dbref(doc): """Decode a JSON DBRef to bson.dbref.DBRef.""" - for key in doc: - if key.startswith('$') and key not in _DBREF_KEYS: - # Other keys start with $, so dct cannot be parsed as a DBRef. - return doc return DBRef(doc.pop('$ref'), doc.pop('$id'), database=doc.pop('$db', None), **doc) diff --git a/doc/changelog.rst b/doc/changelog.rst index 828d8b038..d7a68436e 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -104,6 +104,14 @@ Breaking Changes in 4.0 - Changed the default JSON encoding representation from legacy to relaxed. The json_mode parameter for :const:`bson.json_util.dumps` now defaults to :const:`~bson.json_util.RELAXED_JSON_OPTIONS`. +- Changed the BSON and JSON decoding behavior of :class:`~bson.dbref.DBRef` + to match the behavior outlined in the `DBRef specification`_ version 1.0. + Specifically, PyMongo now only decodes a subdocument into a + :class:`~bson.dbref.DBRef` if and only if, it contains both ``$ref`` and + ``$id`` fields and the ``$ref``, ``$id``, and ``$db`` fields are of the + correct type. Otherwise the document is returned as normal. Previously, any + subdocument containing a ``$ref`` field would be decoded as a + :class:`~bson.dbref.DBRef`. - The "tls" install extra is no longer necessary or supported and will be ignored by pip. - The ``hint`` option is now required when using ``min`` or ``max`` queries @@ -123,6 +131,7 @@ See the `PyMongo 4.0 release notes in JIRA`_ for the list of resolved issues in this release. .. _PyMongo 4.0 release notes in JIRA: https://jira.mongodb.org/secure/ReleaseNote.jspa?projectId=10004&version=18463 +.. _DBRef specification: https://github.com/mongodb/specifications/blob/5a8c8d7/source/dbref.rst Changes in Version 3.12.0 ------------------------- diff --git a/doc/migrate-to-pymongo4.rst b/doc/migrate-to-pymongo4.rst index 2394f29ca..0e512da77 100644 --- a/doc/migrate-to-pymongo4.rst +++ b/doc/migrate-to-pymongo4.rst @@ -752,3 +752,16 @@ Name is a required argument for pymongo.driver_info.DriverInfo ``name`` is now a required argument for the :class:`pymongo.driver_info.DriverInfo` class. +DBRef BSON/JSON decoding behavior +................................. + +Changed the BSON and JSON decoding behavior of :class:`~bson.dbref.DBRef` +to match the behavior outlined in the `DBRef specification`_ version 1.0. +Specifically, PyMongo now only decodes a subdocument into a +:class:`~bson.dbref.DBRef` if and only if, it contains both ``$ref`` and +``$id`` fields and the ``$ref``, ``$id``, and ``$db`` fields are of the +correct type. Otherwise the document is returned as normal. Previously, any +subdocument containing a ``$ref`` field would be decoded as a +:class:`~bson.dbref.DBRef`. + +.. _DBRef specification: https://github.com/mongodb/specifications/blob/5a8c8d7/source/dbref.rst diff --git a/test/bson_corpus/binary.json b/test/bson_corpus/binary.json index 324c56abd..beb2e07a7 100644 --- a/test/bson_corpus/binary.json +++ b/test/bson_corpus/binary.json @@ -50,6 +50,11 @@ "canonical_bson": "1D000000057800100000000573FFD26444B34C6990E8E7D1DFC035D400", "canonical_extjson": "{\"x\" : { \"$binary\" : {\"base64\" : \"c//SZESzTGmQ6OfR38A11A==\", \"subType\" : \"05\"}}}" }, + { + "description": "subtype 0x07", + "canonical_bson": "1D000000057800100000000773FFD26444B34C6990E8E7D1DFC035D400", + "canonical_extjson": "{\"x\" : { \"$binary\" : {\"base64\" : \"c//SZESzTGmQ6OfR38A11A==\", \"subType\" : \"07\"}}}" + }, { "description": "subtype 0x80", "canonical_bson": "0F0000000578000200000080FFFF00", @@ -94,8 +99,20 @@ "string": "{\"x\" : { \"$uuid\" : { \"data\" : \"73ffd264-44b3-4c69-90e8-e7d1dfc035d4\"}}}" }, { - "description": "$uuid invalid value", + "description": "$uuid invalid value--too short", "string": "{\"x\" : { \"$uuid\" : \"73ffd264-44b3-90e8-e7d1dfc035d4\"}}" + }, + { + "description": "$uuid invalid value--too long", + "string": "{\"x\" : { \"$uuid\" : \"73ffd264-44b3-4c69-90e8-e7d1dfc035d4-789e4\"}}" + }, + { + "description": "$uuid invalid value--misplaced hyphens", + "string": "{\"x\" : { \"$uuid\" : \"73ff-d26444b-34c6-990e8e-7d1dfc035d4\"}}" + }, + { + "description": "$uuid invalid value--too many hyphens", + "string": "{\"x\" : { \"$uuid\" : \"----d264-44b3-4--9-90e8-e7d1dfc0----\"}}" } ] } diff --git a/test/bson_corpus/code.json b/test/bson_corpus/code.json index 6f37349ad..b8482b254 100644 --- a/test/bson_corpus/code.json +++ b/test/bson_corpus/code.json @@ -20,48 +20,48 @@ }, { "description": "two-byte UTF-8 (\u00e9)", - "canonical_bson": "190000000261000D000000C3A9C3A9C3A9C3A9C3A9C3A90000", - "canonical_extjson": "{\"a\" : \"\\u00e9\\u00e9\\u00e9\\u00e9\\u00e9\\u00e9\"}" + "canonical_bson": "190000000D61000D000000C3A9C3A9C3A9C3A9C3A9C3A90000", + "canonical_extjson": "{\"a\" : {\"$code\" : \"\\u00e9\\u00e9\\u00e9\\u00e9\\u00e9\\u00e9\"}}" }, { "description": "three-byte UTF-8 (\u2606)", - "canonical_bson": "190000000261000D000000E29886E29886E29886E298860000", - "canonical_extjson": "{\"a\" : \"\\u2606\\u2606\\u2606\\u2606\"}" + "canonical_bson": "190000000D61000D000000E29886E29886E29886E298860000", + "canonical_extjson": "{\"a\" : {\"$code\" : \"\\u2606\\u2606\\u2606\\u2606\"}}" }, { "description": "Embedded nulls", - "canonical_bson": "190000000261000D0000006162006261620062616261620000", - "canonical_extjson": "{\"a\" : \"ab\\u0000bab\\u0000babab\"}" + "canonical_bson": "190000000D61000D0000006162006261620062616261620000", + "canonical_extjson": "{\"a\" : {\"$code\" : \"ab\\u0000bab\\u0000babab\"}}" } ], "decodeErrors": [ { "description": "bad code string length: 0 (but no 0x00 either)", - "bson": "0C0000000261000000000000" + "bson": "0C0000000D61000000000000" }, { "description": "bad code string length: -1", - "bson": "0C000000026100FFFFFFFF00" + "bson": "0C0000000D6100FFFFFFFF00" }, { "description": "bad code string length: eats terminator", - "bson": "10000000026100050000006200620000" + "bson": "100000000D6100050000006200620000" }, { "description": "bad code string length: longer than rest of document", - "bson": "120000000200FFFFFF00666F6F6261720000" + "bson": "120000000D00FFFFFF00666F6F6261720000" }, { "description": "code string is not null-terminated", - "bson": "1000000002610004000000616263FF00" + "bson": "100000000D610004000000616263FF00" }, { "description": "empty code string, but extra null", - "bson": "0E00000002610001000000000000" + "bson": "0E0000000D610001000000000000" }, { "description": "invalid UTF-8", - "bson": "0E00000002610002000000E90000" + "bson": "0E0000000D610002000000E90000" } ] } diff --git a/test/bson_corpus/dbref.json b/test/bson_corpus/dbref.json index 1fe12c6f6..41c0b09d0 100644 --- a/test/bson_corpus/dbref.json +++ b/test/bson_corpus/dbref.json @@ -1,5 +1,5 @@ { - "description": "DBRef", + "description": "Document type (DBRef sub-documents)", "bson_type": "0x03", "valid": [ { @@ -26,6 +26,26 @@ "description": "Document with key names similar to those of a DBRef", "canonical_bson": "3e0000000224726566000c0000006e6f742d612d646272656600072469640058921b3e6e32ab156a22b59e022462616e616e6100050000007065656c0000", "canonical_extjson": "{\"$ref\": \"not-a-dbref\", \"$id\": {\"$oid\": \"58921b3e6e32ab156a22b59e\"}, \"$banana\": \"peel\"}" + }, + { + "description": "DBRef with additional dollar-prefixed and dotted fields", + "canonical_bson": "48000000036462726566003c0000000224726566000b000000636f6c6c656374696f6e00072469640058921b3e6e32ab156a22b59e10612e62000100000010246300010000000000", + "canonical_extjson": "{\"dbref\": {\"$ref\": \"collection\", \"$id\": {\"$oid\": \"58921b3e6e32ab156a22b59e\"}, \"a.b\": {\"$numberInt\": \"1\"}, \"$c\": {\"$numberInt\": \"1\"}}}" + }, + { + "description": "Sub-document resembles DBRef but $id is missing", + "canonical_bson": "26000000036462726566001a0000000224726566000b000000636f6c6c656374696f6e000000", + "canonical_extjson": "{\"dbref\": {\"$ref\": \"collection\"}}" + }, + { + "description": "Sub-document resembles DBRef but $ref is not a string", + "canonical_bson": "2c000000036462726566002000000010247265660001000000072469640058921b3e6e32ab156a22b59e0000", + "canonical_extjson": "{\"dbref\": {\"$ref\": {\"$numberInt\": \"1\"}, \"$id\": {\"$oid\": \"58921b3e6e32ab156a22b59e\"}}}" + }, + { + "description": "Sub-document resembles DBRef but $db is not a string", + "canonical_bson": "4000000003646272656600340000000224726566000b000000636f6c6c656374696f6e00072469640058921b3e6e32ab156a22b59e1024646200010000000000", + "canonical_extjson": "{\"dbref\": {\"$ref\": \"collection\", \"$id\": {\"$oid\": \"58921b3e6e32ab156a22b59e\"}, \"$db\": {\"$numberInt\": \"1\"}}}" } ] } diff --git a/test/bson_corpus/double.json b/test/bson_corpus/double.json index 7be4ff45e..d5b8fb3d7 100644 --- a/test/bson_corpus/double.json +++ b/test/bson_corpus/double.json @@ -30,13 +30,13 @@ { "description": "1.2345678921232E+18", "canonical_bson": "100000000164002a1bf5f41022b14300", - "canonical_extjson": "{\"d\" : {\"$numberDouble\": \"1.2345678921232e+18\"}}", + "canonical_extjson": "{\"d\" : {\"$numberDouble\": \"1.2345678921232E+18\"}}", "relaxed_extjson": "{\"d\" : 1.2345678921232E+18}" }, { "description": "-1.2345678921232E+18", "canonical_bson": "100000000164002a1bf5f41022b1c300", - "canonical_extjson": "{\"d\" : {\"$numberDouble\": \"-1.2345678921232e+18\"}}", + "canonical_extjson": "{\"d\" : {\"$numberDouble\": \"-1.2345678921232E+18\"}}", "relaxed_extjson": "{\"d\" : -1.2345678921232E+18}" }, { diff --git a/test/bson_corpus/symbol.json b/test/bson_corpus/symbol.json index 4e46cb951..3dd3577eb 100644 --- a/test/bson_corpus/symbol.json +++ b/test/bson_corpus/symbol.json @@ -50,31 +50,31 @@ "decodeErrors": [ { "description": "bad symbol length: 0 (but no 0x00 either)", - "bson": "0C0000000261000000000000" + "bson": "0C0000000E61000000000000" }, { "description": "bad symbol length: -1", - "bson": "0C000000026100FFFFFFFF00" + "bson": "0C0000000E6100FFFFFFFF00" }, { "description": "bad symbol length: eats terminator", - "bson": "10000000026100050000006200620000" + "bson": "100000000E6100050000006200620000" }, { "description": "bad symbol length: longer than rest of document", - "bson": "120000000200FFFFFF00666F6F6261720000" + "bson": "120000000E00FFFFFF00666F6F6261720000" }, { "description": "symbol is not null-terminated", - "bson": "1000000002610004000000616263FF00" + "bson": "100000000E610004000000616263FF00" }, { "description": "empty symbol, but extra null", - "bson": "0E00000002610001000000000000" + "bson": "0E0000000E610001000000000000" }, { "description": "invalid UTF-8", - "bson": "0E00000002610002000000E90000" + "bson": "0E0000000E610002000000E90000" } ] } diff --git a/test/crud/unified/aggregate-merge.json b/test/crud/unified/aggregate-merge.json index c34e93a69..ac61ceb8a 100644 --- a/test/crud/unified/aggregate-merge.json +++ b/test/crud/unified/aggregate-merge.json @@ -1,6 +1,6 @@ { "description": "aggregate-merge", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "4.1.11" diff --git a/test/crud/unified/bulkWrite-arrayFilters-clientError.json b/test/crud/unified/bulkWrite-arrayFilters-clientError.json index 6073890dd..63815e323 100644 --- a/test/crud/unified/bulkWrite-arrayFilters-clientError.json +++ b/test/crud/unified/bulkWrite-arrayFilters-clientError.json @@ -1,6 +1,6 @@ { "description": "bulkWrite-arrayFilters-clientError", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "maxServerVersion": "3.5.5" diff --git a/test/crud/unified/bulkWrite-arrayFilters.json b/test/crud/unified/bulkWrite-arrayFilters.json index 4d7b4b794..70ee014f7 100644 --- a/test/crud/unified/bulkWrite-arrayFilters.json +++ b/test/crud/unified/bulkWrite-arrayFilters.json @@ -1,6 +1,6 @@ { "description": "bulkWrite-arrayFilters", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "3.5.6" diff --git a/test/crud/unified/bulkWrite-delete-hint-clientError.json b/test/crud/unified/bulkWrite-delete-hint-clientError.json index c55067be2..2961b55dc 100644 --- a/test/crud/unified/bulkWrite-delete-hint-clientError.json +++ b/test/crud/unified/bulkWrite-delete-hint-clientError.json @@ -1,6 +1,6 @@ { "description": "bulkWrite-delete-hint-clientError", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "maxServerVersion": "3.3.99" diff --git a/test/crud/unified/bulkWrite-delete-hint-serverError.json b/test/crud/unified/bulkWrite-delete-hint-serverError.json index 30b9010ac..fa9952209 100644 --- a/test/crud/unified/bulkWrite-delete-hint-serverError.json +++ b/test/crud/unified/bulkWrite-delete-hint-serverError.json @@ -1,6 +1,6 @@ { "description": "bulkWrite-delete-hint-serverError", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "3.4.0", diff --git a/test/crud/unified/bulkWrite-delete-hint.json b/test/crud/unified/bulkWrite-delete-hint.json index 31f386532..9fcdecefd 100644 --- a/test/crud/unified/bulkWrite-delete-hint.json +++ b/test/crud/unified/bulkWrite-delete-hint.json @@ -1,6 +1,6 @@ { "description": "bulkWrite-delete-hint", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "4.3.4" diff --git a/test/crud/unified/bulkWrite-update-hint-clientError.json b/test/crud/unified/bulkWrite-update-hint-clientError.json index 68a92065a..d5eb71c29 100644 --- a/test/crud/unified/bulkWrite-update-hint-clientError.json +++ b/test/crud/unified/bulkWrite-update-hint-clientError.json @@ -1,6 +1,6 @@ { "description": "bulkWrite-update-hint-clientError", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "maxServerVersion": "3.3.99" diff --git a/test/crud/unified/bulkWrite-update-hint-serverError.json b/test/crud/unified/bulkWrite-update-hint-serverError.json index 2a9a6795c..b0f7e1b38 100644 --- a/test/crud/unified/bulkWrite-update-hint-serverError.json +++ b/test/crud/unified/bulkWrite-update-hint-serverError.json @@ -1,6 +1,6 @@ { "description": "bulkWrite-update-hint-serverError", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "3.4.0", diff --git a/test/crud/unified/bulkWrite-update-hint.json b/test/crud/unified/bulkWrite-update-hint.json index b8445d80d..420635989 100644 --- a/test/crud/unified/bulkWrite-update-hint.json +++ b/test/crud/unified/bulkWrite-update-hint.json @@ -1,6 +1,6 @@ { "description": "bulkWrite-update-hint", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "4.2.0" diff --git a/test/crud/unified/deleteMany-hint-clientError.json b/test/crud/unified/deleteMany-hint-clientError.json index 285f567f0..66320122b 100644 --- a/test/crud/unified/deleteMany-hint-clientError.json +++ b/test/crud/unified/deleteMany-hint-clientError.json @@ -1,6 +1,6 @@ { "description": "deleteMany-hint-clientError", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "maxServerVersion": "3.3.99" diff --git a/test/crud/unified/deleteMany-hint-serverError.json b/test/crud/unified/deleteMany-hint-serverError.json index 90bfb89fc..88d4a6557 100644 --- a/test/crud/unified/deleteMany-hint-serverError.json +++ b/test/crud/unified/deleteMany-hint-serverError.json @@ -1,6 +1,6 @@ { "description": "deleteMany-hint-serverError", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "3.4.0", diff --git a/test/crud/unified/deleteMany-hint.json b/test/crud/unified/deleteMany-hint.json index b0cdc0304..59d903d20 100644 --- a/test/crud/unified/deleteMany-hint.json +++ b/test/crud/unified/deleteMany-hint.json @@ -1,6 +1,6 @@ { "description": "deleteMany-hint", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "4.3.4" diff --git a/test/crud/unified/deleteOne-hint-clientError.json b/test/crud/unified/deleteOne-hint-clientError.json index b6b2932bd..cf629f59e 100644 --- a/test/crud/unified/deleteOne-hint-clientError.json +++ b/test/crud/unified/deleteOne-hint-clientError.json @@ -1,6 +1,6 @@ { "description": "deleteOne-hint-clientError", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "maxServerVersion": "3.3.99" diff --git a/test/crud/unified/deleteOne-hint-serverError.json b/test/crud/unified/deleteOne-hint-serverError.json index 8b1398c75..15541ed85 100644 --- a/test/crud/unified/deleteOne-hint-serverError.json +++ b/test/crud/unified/deleteOne-hint-serverError.json @@ -1,6 +1,6 @@ { "description": "deleteOne-hint-serverError", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "3.4.0", diff --git a/test/crud/unified/deleteOne-hint.json b/test/crud/unified/deleteOne-hint.json index 9e3970a54..bcc4bc234 100644 --- a/test/crud/unified/deleteOne-hint.json +++ b/test/crud/unified/deleteOne-hint.json @@ -1,6 +1,6 @@ { "description": "deleteOne-hint", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "4.3.4" diff --git a/test/crud/unified/find-allowdiskuse-clientError.json b/test/crud/unified/find-allowdiskuse-clientError.json index a47fd8e25..5bd954e79 100644 --- a/test/crud/unified/find-allowdiskuse-clientError.json +++ b/test/crud/unified/find-allowdiskuse-clientError.json @@ -1,6 +1,6 @@ { "description": "find-allowdiskuse-clientError", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "maxServerVersion": "3.0.99" diff --git a/test/crud/unified/find-allowdiskuse-serverError.json b/test/crud/unified/find-allowdiskuse-serverError.json index a7907ba25..dc58f8f0e 100644 --- a/test/crud/unified/find-allowdiskuse-serverError.json +++ b/test/crud/unified/find-allowdiskuse-serverError.json @@ -1,6 +1,6 @@ { "description": "find-allowdiskuse-serverError", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "3.2", diff --git a/test/crud/unified/find-allowdiskuse.json b/test/crud/unified/find-allowdiskuse.json index 8d4cf66bf..789bb7fbf 100644 --- a/test/crud/unified/find-allowdiskuse.json +++ b/test/crud/unified/find-allowdiskuse.json @@ -1,6 +1,6 @@ { "description": "find-allowdiskuse", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "4.3.1" diff --git a/test/crud/unified/findOneAndDelete-hint-clientError.json b/test/crud/unified/findOneAndDelete-hint-clientError.json index d04125edd..c6ff46786 100644 --- a/test/crud/unified/findOneAndDelete-hint-clientError.json +++ b/test/crud/unified/findOneAndDelete-hint-clientError.json @@ -1,6 +1,6 @@ { "description": "findOneAndDelete-hint-clientError", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "maxServerVersion": "4.0.99" diff --git a/test/crud/unified/findOneAndDelete-hint-serverError.json b/test/crud/unified/findOneAndDelete-hint-serverError.json index 23c01f48f..b87410272 100644 --- a/test/crud/unified/findOneAndDelete-hint-serverError.json +++ b/test/crud/unified/findOneAndDelete-hint-serverError.json @@ -1,6 +1,6 @@ { "description": "findOneAndDelete-hint-serverError", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "4.2.0", diff --git a/test/crud/unified/findOneAndDelete-hint.json b/test/crud/unified/findOneAndDelete-hint.json index 0180010dc..8b53f2bd3 100644 --- a/test/crud/unified/findOneAndDelete-hint.json +++ b/test/crud/unified/findOneAndDelete-hint.json @@ -1,6 +1,6 @@ { "description": "findOneAndDelete-hint", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "4.3.4" diff --git a/test/crud/unified/findOneAndReplace-hint-clientError.json b/test/crud/unified/findOneAndReplace-hint-clientError.json index c483b2302..6b07eb1f4 100644 --- a/test/crud/unified/findOneAndReplace-hint-clientError.json +++ b/test/crud/unified/findOneAndReplace-hint-clientError.json @@ -1,6 +1,6 @@ { "description": "findOneAndReplace-hint-clientError", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "maxServerVersion": "4.0.99" diff --git a/test/crud/unified/findOneAndReplace-hint-serverError.json b/test/crud/unified/findOneAndReplace-hint-serverError.json index e8f1c8936..7fbf5a0ea 100644 --- a/test/crud/unified/findOneAndReplace-hint-serverError.json +++ b/test/crud/unified/findOneAndReplace-hint-serverError.json @@ -1,6 +1,6 @@ { "description": "findOneAndReplace-hint-serverError", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "4.2.0", diff --git a/test/crud/unified/findOneAndReplace-hint.json b/test/crud/unified/findOneAndReplace-hint.json index 13ac6a9c9..d07c5921a 100644 --- a/test/crud/unified/findOneAndReplace-hint.json +++ b/test/crud/unified/findOneAndReplace-hint.json @@ -1,6 +1,6 @@ { "description": "findOneAndReplace-hint", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "4.3.1" diff --git a/test/crud/unified/findOneAndUpdate-hint-clientError.json b/test/crud/unified/findOneAndUpdate-hint-clientError.json index dace72b0a..d0b51313c 100644 --- a/test/crud/unified/findOneAndUpdate-hint-clientError.json +++ b/test/crud/unified/findOneAndUpdate-hint-clientError.json @@ -1,6 +1,6 @@ { "description": "findOneAndUpdate-hint-clientError", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "maxServerVersion": "4.0.99" diff --git a/test/crud/unified/findOneAndUpdate-hint-serverError.json b/test/crud/unified/findOneAndUpdate-hint-serverError.json index 1413ced2e..99fd9938f 100644 --- a/test/crud/unified/findOneAndUpdate-hint-serverError.json +++ b/test/crud/unified/findOneAndUpdate-hint-serverError.json @@ -1,6 +1,6 @@ { "description": "findOneAndUpdate-hint-serverError", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "4.2.0", diff --git a/test/crud/unified/findOneAndUpdate-hint.json b/test/crud/unified/findOneAndUpdate-hint.json index 68cef18ef..5be6d2b3e 100644 --- a/test/crud/unified/findOneAndUpdate-hint.json +++ b/test/crud/unified/findOneAndUpdate-hint.json @@ -1,6 +1,6 @@ { "description": "findOneAndUpdate-hint", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "4.3.1" diff --git a/test/crud/unified/replaceOne-hint.json b/test/crud/unified/replaceOne-hint.json index edb1ceb7c..6926e9d8d 100644 --- a/test/crud/unified/replaceOne-hint.json +++ b/test/crud/unified/replaceOne-hint.json @@ -1,6 +1,6 @@ { "description": "replaceOne-hint", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "4.2.0" diff --git a/test/crud/unified/updateMany-hint-clientError.json b/test/crud/unified/updateMany-hint-clientError.json index 99c66c919..5da878e29 100644 --- a/test/crud/unified/updateMany-hint-clientError.json +++ b/test/crud/unified/updateMany-hint-clientError.json @@ -1,6 +1,6 @@ { "description": "updateMany-hint-clientError", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "maxServerVersion": "3.3.99" diff --git a/test/crud/unified/updateMany-hint-serverError.json b/test/crud/unified/updateMany-hint-serverError.json index cc5ecfe26..c81f36b13 100644 --- a/test/crud/unified/updateMany-hint-serverError.json +++ b/test/crud/unified/updateMany-hint-serverError.json @@ -1,6 +1,6 @@ { "description": "updateMany-hint-serverError", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "3.4.0", diff --git a/test/crud/unified/updateMany-hint.json b/test/crud/unified/updateMany-hint.json index e5f707fb5..929be5299 100644 --- a/test/crud/unified/updateMany-hint.json +++ b/test/crud/unified/updateMany-hint.json @@ -1,6 +1,6 @@ { "description": "updateMany-hint", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "4.2.0" diff --git a/test/crud/unified/updateOne-hint-clientError.json b/test/crud/unified/updateOne-hint-clientError.json index 8c0ddbd1d..d4f1a5343 100644 --- a/test/crud/unified/updateOne-hint-clientError.json +++ b/test/crud/unified/updateOne-hint-clientError.json @@ -1,6 +1,6 @@ { "description": "updateOne-hint-clientError", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "maxServerVersion": "3.3.99" diff --git a/test/crud/unified/updateOne-hint-serverError.json b/test/crud/unified/updateOne-hint-serverError.json index d8a46da94..05fb03331 100644 --- a/test/crud/unified/updateOne-hint-serverError.json +++ b/test/crud/unified/updateOne-hint-serverError.json @@ -1,6 +1,6 @@ { "description": "updateOne-hint-serverError", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "3.4.0", diff --git a/test/crud/unified/updateOne-hint.json b/test/crud/unified/updateOne-hint.json index 9277c605f..484e00757 100644 --- a/test/crud/unified/updateOne-hint.json +++ b/test/crud/unified/updateOne-hint.json @@ -1,6 +1,6 @@ { "description": "updateOne-hint", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "4.2.0" diff --git a/test/crud/unified/updateWithPipelines.json b/test/crud/unified/updateWithPipelines.json index 12ae04665..164f2f6a1 100644 --- a/test/crud/unified/updateWithPipelines.json +++ b/test/crud/unified/updateWithPipelines.json @@ -1,6 +1,6 @@ { "description": "updateWithPipelines", - "schemaVersion": "1.1", + "schemaVersion": "1.0", "runOnRequirements": [ { "minServerVersion": "4.1.11" diff --git a/test/test_bson.py b/test/test_bson.py index ee30c8948..89f4a1117 100644 --- a/test/test_bson.py +++ b/test/test_bson.py @@ -490,8 +490,7 @@ class TestBSON(unittest.TestCase): ref_only = {'ref': {'$ref': 'collection'}} id_only = {'ref': {'$id': ObjectId()}} - self.assertEqual(DBRef('collection', id=None), - decode(encode(ref_only))['ref']) + self.assertEqual(ref_only, decode(encode(ref_only))) self.assertEqual(id_only, decode(encode(id_only))) def test_bytes_as_keys(self): diff --git a/test/test_bson_corpus.py b/test/test_bson_corpus.py index 68703cc70..7893395c6 100644 --- a/test/test_bson_corpus.py +++ b/test/test_bson_corpus.py @@ -40,20 +40,27 @@ from test import unittest _TEST_PATH = os.path.join( os.path.dirname(os.path.realpath(__file__)), 'bson_corpus') -_TESTS_TO_SKIP = set([ +_TESTS_TO_SKIP = { # Python cannot decode dates after year 9999. 'Y10K', -]) +} -_NON_PARSE_ERRORS = set([ +_NON_PARSE_ERRORS = { # {"$date": } is our legacy format which we still need to parse. 'Bad $date (number, not string or hash)', # This variant of $numberLong may have been generated by an old version # of mongoexport. 'Bad $numberLong (number, not string)', + # Python's UUID constructor is very permissive. + '$uuid invalid value--misplaced hyphens', # We parse Regex flags with extra characters, including nulls. 'Null byte in $regularExpression options', -]) +} + +_IMPLCIT_LOSSY_TESTS = { + # JSON decodes top-level $ref+$id as a DBRef but BSON doesn't. + 'Document with key names similar to those of a DBRef' +} _DEPRECATED_BSON_TYPES = { # Symbol @@ -128,15 +135,22 @@ def create_test(case_spec): cEJ = valid_case['canonical_extjson'] rEJ = valid_case.get('relaxed_extjson') dEJ = valid_case.get('degenerate_extjson') + if description in _IMPLCIT_LOSSY_TESTS: + valid_case.setdefault('lossy', True) lossy = valid_case.get('lossy') + # BSON double, use lowercase 'e+' to match Python's encoding + if bson_type == '0x01': + cEJ = cEJ.replace('E+', 'e+') + decoded_bson = decode_bson(cB) if not lossy: # Make sure we can parse the legacy (default) JSON format. legacy_json = json_util.dumps( decoded_bson, json_options=json_util.LEGACY_JSON_OPTIONS) - self.assertEqual(decode_extjson(legacy_json), decoded_bson) + self.assertEqual( + decode_extjson(legacy_json), decoded_bson, description) if deprecated: if 'converted_bson' in valid_case: @@ -158,7 +172,7 @@ def create_test(case_spec): if not (sys.platform.startswith("java") and description == 'NaN with payload'): # Test round-tripping canonical bson. - self.assertEqual(encode_bson(decoded_bson), cB) + self.assertEqual(encode_bson(decoded_bson), cB, description) self.assertJsonEqual(encode_extjson(decoded_bson), cEJ) # Test round-tripping canonical extended json. @@ -191,24 +205,24 @@ def create_test(case_spec): binascii.unhexlify(decode_error_case['bson'].encode('utf8'))) for parse_error_case in case_spec.get('parseErrors', []): + description = parse_error_case['description'] + if description in _NON_PARSE_ERRORS: + decode_extjson(parse_error_case['string']) + continue if bson_type == '0x13': self.assertRaises( DecimalException, Decimal128, parse_error_case['string']) elif bson_type == '0x00': - description = parse_error_case['description'] - if description in _NON_PARSE_ERRORS: - decode_extjson(parse_error_case['string']) - else: - try: - doc = decode_extjson(parse_error_case['string']) - # Null bytes are validated when encoding to BSON. - if 'Null' in description: - to_bson(doc) - raise AssertionError('exception not raised for test ' - 'case: ' + description) - except (ValueError, KeyError, TypeError, InvalidId, - InvalidDocument): - pass + try: + doc = decode_extjson(parse_error_case['string']) + # Null bytes are validated when encoding to BSON. + if 'Null' in description: + to_bson(doc) + raise AssertionError('exception not raised for test ' + 'case: ' + description) + except (ValueError, KeyError, TypeError, InvalidId, + InvalidDocument): + pass elif bson_type == '0x05': try: decode_extjson(parse_error_case['string']) diff --git a/test/test_dbref.py b/test/test_dbref.py index d71c2c68e..964947351 100644 --- a/test/test_dbref.py +++ b/test/test_dbref.py @@ -18,6 +18,7 @@ import pickle import sys sys.path[0:0] = [""] +from bson import encode, decode from bson.dbref import DBRef from bson.objectid import ObjectId from test import unittest @@ -131,5 +132,108 @@ class TestDBRef(unittest.TestCase): self.assertNotEqual(hash(dbref_1a), hash(dbref_2a)) + +# https://github.com/mongodb/specifications/blob/master/source/dbref.rst#test-plan +class TestDBRefSpec(unittest.TestCase): + def test_decoding_1_2_3(self): + for doc in [ + # 1, Valid documents MUST be decoded to a DBRef: + {"$ref": "coll0", "$id": ObjectId("60a6fe9a54f4180c86309efa")}, + {"$ref": "coll0", "$id": 1}, + {"$ref": "coll0", "$id": None}, + {"$ref": "coll0", "$id": 1, "$db": "db0"}, + # 2, Valid documents with extra fields: + {"$ref": "coll0", "$id": 1, "$db": "db0", "foo": "bar"}, + {"$ref": "coll0", "$id": 1, "foo": True, "bar": False}, + {"$ref": "coll0", "$id": 1, "meta": {"foo": 1, "bar": 2}}, + {"$ref": "coll0", "$id": 1, "$foo": "bar"}, + {"$ref": "coll0", "$id": 1, "foo.bar": 0}, + # 3, Valid documents with out of order fields: + {"$id": 1, "$ref": "coll0"}, + {"$db": "db0", "$ref": "coll0", "$id": 1}, + {"foo": 1, "$id": 1, "$ref": "coll0"}, + {"foo": 1, "$ref": "coll0", "$id": 1, "$db": "db0"}, + {"foo": 1, "$ref": "coll0", "$id": 1, "$db": "db0", "bar": 1}, + ]: + with self.subTest(doc=doc): + decoded = decode(encode({'dbref': doc})) + dbref = decoded['dbref'] + self.assertIsInstance(dbref, DBRef) + self.assertEqual(dbref.collection, doc['$ref']) + self.assertEqual(dbref.id, doc['$id']) + self.assertEqual(dbref.database, doc.get('$db')) + for extra in set(doc.keys()) - {"$ref", "$id", "$db"}: + self.assertEqual(getattr(dbref, extra), doc[extra]) + + def test_decoding_4_5(self): + for doc in [ + # 4, Documents missing required fields MUST NOT be decoded to a + # DBRef: + {"$ref": "coll0"}, + {"$id": ObjectId("60a6fe9a54f4180c86309efa")}, + {"$db": "db0"}, + # 5, Documents with invalid types for $ref or $db MUST NOT be + # decoded to a DBRef + {"$ref": True, "$id": 1}, + {"$ref": "coll0", "$id": 1, "$db": 1}, + ]: + with self.subTest(doc=doc): + decoded = decode(encode({'dbref': doc})) + dbref = decoded['dbref'] + self.assertIsInstance(dbref, dict) + + def test_encoding_1_2(self): + for doc in [ + # 1, Encoding DBRefs with basic fields: + {"$ref": "coll0", "$id": ObjectId("60a6fe9a54f4180c86309efa")}, + {"$ref": "coll0", "$id": 1}, + {"$ref": "coll0", "$id": None}, + {"$ref": "coll0", "$id": 1, "$db": "db0"}, + # 2, Encoding DBRefs with extra, optional fields: + {"$ref": "coll0", "$id": 1, "$db": "db0", "foo": "bar"}, + {"$ref": "coll0", "$id": 1, "foo": True, "bar": False}, + {"$ref": "coll0", "$id": 1, "meta": {"foo": 1, "bar": 2}}, + {"$ref": "coll0", "$id": 1, "$foo": "bar"}, + {"$ref": "coll0", "$id": 1, "foo.bar": 0}, + ]: + with self.subTest(doc=doc): + # Decode the test input to a DBRef via a BSON roundtrip. + encoded_doc = encode({'dbref': doc}) + decoded = decode(encoded_doc) + dbref = decoded['dbref'] + self.assertIsInstance(dbref, DBRef) + # Encode the DBRef. + encoded_dbref = encode(decoded) + self.assertEqual(encoded_dbref, encoded_doc) + # Ensure extra fields are present. + for extra in set(doc.keys()) - {"$ref", "$id", "$db"}: + self.assertEqual(getattr(dbref, extra), doc[extra]) + + def test_encoding_3(self): + for doc in [ + # 3, Encoding DBRefs re-orders any out of order fields during + # decoding: + {"$id": 1, "$ref": "coll0"}, + {"$db": "db0", "$ref": "coll0", "$id": 1}, + {"foo": 1, "$id": 1, "$ref": "coll0"}, + {"foo": 1, "$ref": "coll0", "$id": 1, "$db": "db0"}, + {"foo": 1, "$ref": "coll0", "$id": 1, "$db": "db0", "bar": 1}, + ]: + with self.subTest(doc=doc): + # Decode the test input to a DBRef via a BSON roundtrip. + encoded_doc = encode({'dbref': doc}) + decoded = decode(encoded_doc) + dbref = decoded['dbref'] + self.assertIsInstance(dbref, DBRef) + # Encode the DBRef. + encoded_dbref = encode(decoded) + # BSON does not match because DBRef fields are reordered. + self.assertNotEqual(encoded_dbref, encoded_doc) + self.assertEqual(decode(encoded_dbref), decode(encoded_doc)) + # Ensure extra fields are present. + for extra in set(doc.keys()) - {"$ref", "$id", "$db"}: + self.assertEqual(getattr(dbref, extra), doc[extra]) + + if __name__ == "__main__": unittest.main()