diff --git a/gridfs/asynchronous/grid_file.py b/gridfs/asynchronous/grid_file.py index 3bbe3754fe..f3d816f4dd 100644 --- a/gridfs/asynchronous/grid_file.py +++ b/gridfs/asynchronous/grid_file.py @@ -284,8 +284,8 @@ async def delete(self, file_id: Any, session: Optional[AsyncClientSession] = Non ``delete`` no longer ensures indexes. """ _disallow_transactions(session) - await self._files.delete_one({"_id": file_id}, session=session) - await self._chunks.delete_many({"files_id": file_id}, session=session) + await self._files.delete_one({"_id": {"$eq": file_id}}, session=session) + await self._chunks.delete_many({"files_id": {"$eq": file_id}}, session=session) async def list(self, session: Optional[AsyncClientSession] = None) -> list[str]: """List the names of all files stored in this instance of @@ -341,7 +341,7 @@ async def find_one( Added ``session`` parameter. """ if filter is not None and not isinstance(filter, abc.Mapping): - filter = {"_id": filter} + filter = {"_id": {"$eq": filter}} _disallow_transactions(session) async for f in self.find(filter, *args, session=session, **kwargs): @@ -455,6 +455,8 @@ async def exists( if kwargs: f = await self._files.find_one(kwargs, ["_id"], session=session) else: + if document_or_id is not None and not isinstance(document_or_id, abc.Mapping): + document_or_id = {"_id": {"$eq": document_or_id}} f = await self._files.find_one(document_or_id, ["_id"], session=session) return f is not None @@ -831,8 +833,8 @@ async def delete(self, file_id: Any, session: Optional[AsyncClientSession] = Non Added ``session`` parameter. """ _disallow_transactions(session) - res = await self._files.delete_one({"_id": file_id}, session=session) - await self._chunks.delete_many({"files_id": file_id}, session=session) + res = await self._files.delete_one({"_id": {"$eq": file_id}}, session=session) + await self._chunks.delete_many({"files_id": {"$eq": file_id}}, session=session) if not res.deleted_count: raise NoFile(f"no file could be deleted because none matched {file_id}") @@ -1044,7 +1046,7 @@ async def rename( """ _disallow_transactions(session) result = await self._files.update_one( - {"_id": file_id}, {"$set": {"filename": new_filename}}, session=session + {"_id": {"$eq": file_id}}, {"$set": {"filename": new_filename}}, session=session ) if not result.matched_count: raise NoFile( @@ -1198,8 +1200,12 @@ async def _ensure_indexes(self) -> None: async def abort(self) -> None: """Remove all chunks/files that may have been uploaded and close.""" - await self._coll.chunks.delete_many({"files_id": self._file["_id"]}, session=self._session) - await self._coll.files.delete_one({"_id": self._file["_id"]}, session=self._session) + await self._coll.chunks.delete_many( + {"files_id": {"$eq": self._file["_id"]}}, session=self._session + ) + await self._coll.files.delete_one( + {"_id": {"$eq": self._file["_id"]}}, session=self._session + ) object.__setattr__(self, "_closed", True) @property @@ -1248,7 +1254,9 @@ def __setattr__(self, name: str, value: Any) -> None: self._file[name] = value if self._closed: if _IS_SYNC: - self._coll.files.update_one({"_id": self._file["_id"]}, {"$set": {name: value}}) + self._coll.files.update_one( + {"_id": {"$eq": self._file["_id"]}}, {"$set": {name: value}} + ) else: raise AttributeError( "AsyncGridIn does not support __setattr__ after being closed(). Set the attribute before closing the file or use AsyncGridIn.set() instead" @@ -1257,7 +1265,9 @@ def __setattr__(self, name: str, value: Any) -> None: async def set(self, name: str, value: Any) -> None: self._file[name] = value if self._closed: - await self._coll.files.update_one({"_id": self._file["_id"]}, {"$set": {name: value}}) + await self._coll.files.update_one( + {"_id": {"$eq": self._file["_id"]}}, {"$set": {name: value}} + ) async def _flush_data(self, data: Any, force: bool = False) -> None: """Flush `data` to a chunk.""" @@ -1582,7 +1592,9 @@ async def readlines(self, size: int = -1) -> list[bytes]: async def open(self) -> None: if not self._file: _disallow_transactions(self._session) - self._file = await self._files.find_one({"_id": self._file_id}, session=self._session) + self._file = await self._files.find_one( + {"_id": {"$eq": self._file_id}}, session=self._session + ) if not self._file: raise NoFile( f"no file in gridfs collection {self._files!r} with _id {self._file_id!r}" @@ -1853,7 +1865,7 @@ def __aiter__(self) -> _AsyncGridOutChunkIterator: return self def _create_cursor(self) -> None: - filter = {"files_id": self._id} + filter = {"files_id": {"$eq": self._id}} if self._next_chunk > 0: filter["n"] = {"$gte": self._next_chunk} _disallow_transactions(self._session) diff --git a/gridfs/grid_file_shared.py b/gridfs/grid_file_shared.py index 81585b003c..470f3f14d2 100644 --- a/gridfs/grid_file_shared.py +++ b/gridfs/grid_file_shared.py @@ -120,7 +120,9 @@ def setter(self: Any, value: Any) -> Any: if warn_str: warnings.warn(warn_str, stacklevel=2, category=DeprecationWarning) if self._closed: - self._coll.files.update_one({"_id": self._file["_id"]}, {"$set": {field_name: value}}) + self._coll.files.update_one( + {"_id": {"$eq": self._file["_id"]}}, {"$set": {field_name: value}} + ) self._file[field_name] = value if read_only: diff --git a/gridfs/synchronous/grid_file.py b/gridfs/synchronous/grid_file.py index 7bd2aad75f..ac7301e170 100644 --- a/gridfs/synchronous/grid_file.py +++ b/gridfs/synchronous/grid_file.py @@ -284,8 +284,8 @@ def delete(self, file_id: Any, session: Optional[ClientSession] = None) -> None: ``delete`` no longer ensures indexes. """ _disallow_transactions(session) - self._files.delete_one({"_id": file_id}, session=session) - self._chunks.delete_many({"files_id": file_id}, session=session) + self._files.delete_one({"_id": {"$eq": file_id}}, session=session) + self._chunks.delete_many({"files_id": {"$eq": file_id}}, session=session) def list(self, session: Optional[ClientSession] = None) -> list[str]: """List the names of all files stored in this instance of @@ -339,7 +339,7 @@ def find_one( Added ``session`` parameter. """ if filter is not None and not isinstance(filter, abc.Mapping): - filter = {"_id": filter} + filter = {"_id": {"$eq": filter}} _disallow_transactions(session) for f in self.find(filter, *args, session=session, **kwargs): @@ -453,6 +453,8 @@ def exists( if kwargs: f = self._files.find_one(kwargs, ["_id"], session=session) else: + if document_or_id is not None and not isinstance(document_or_id, abc.Mapping): + document_or_id = {"_id": {"$eq": document_or_id}} f = self._files.find_one(document_or_id, ["_id"], session=session) return f is not None @@ -827,8 +829,8 @@ def delete(self, file_id: Any, session: Optional[ClientSession] = None) -> None: Added ``session`` parameter. """ _disallow_transactions(session) - res = self._files.delete_one({"_id": file_id}, session=session) - self._chunks.delete_many({"files_id": file_id}, session=session) + res = self._files.delete_one({"_id": {"$eq": file_id}}, session=session) + self._chunks.delete_many({"files_id": {"$eq": file_id}}, session=session) if not res.deleted_count: raise NoFile(f"no file could be deleted because none matched {file_id}") @@ -1036,7 +1038,7 @@ def rename( """ _disallow_transactions(session) result = self._files.update_one( - {"_id": file_id}, {"$set": {"filename": new_filename}}, session=session + {"_id": {"$eq": file_id}}, {"$set": {"filename": new_filename}}, session=session ) if not result.matched_count: raise NoFile( @@ -1186,8 +1188,10 @@ def _ensure_indexes(self) -> None: def abort(self) -> None: """Remove all chunks/files that may have been uploaded and close.""" - self._coll.chunks.delete_many({"files_id": self._file["_id"]}, session=self._session) - self._coll.files.delete_one({"_id": self._file["_id"]}, session=self._session) + self._coll.chunks.delete_many( + {"files_id": {"$eq": self._file["_id"]}}, session=self._session + ) + self._coll.files.delete_one({"_id": {"$eq": self._file["_id"]}}, session=self._session) object.__setattr__(self, "_closed", True) @property @@ -1236,7 +1240,9 @@ def __setattr__(self, name: str, value: Any) -> None: self._file[name] = value if self._closed: if _IS_SYNC: - self._coll.files.update_one({"_id": self._file["_id"]}, {"$set": {name: value}}) + self._coll.files.update_one( + {"_id": {"$eq": self._file["_id"]}}, {"$set": {name: value}} + ) else: raise AttributeError( "GridIn does not support __setattr__ after being closed(). Set the attribute before closing the file or use GridIn.set() instead" @@ -1245,7 +1251,9 @@ def __setattr__(self, name: str, value: Any) -> None: def set(self, name: str, value: Any) -> None: self._file[name] = value if self._closed: - self._coll.files.update_one({"_id": self._file["_id"]}, {"$set": {name: value}}) + self._coll.files.update_one( + {"_id": {"$eq": self._file["_id"]}}, {"$set": {name: value}} + ) def _flush_data(self, data: Any, force: bool = False) -> None: """Flush `data` to a chunk.""" @@ -1570,7 +1578,9 @@ def readlines(self, size: int = -1) -> list[bytes]: def open(self) -> None: if not self._file: _disallow_transactions(self._session) - self._file = self._files.find_one({"_id": self._file_id}, session=self._session) + self._file = self._files.find_one( + {"_id": {"$eq": self._file_id}}, session=self._session + ) if not self._file: raise NoFile( f"no file in gridfs collection {self._files!r} with _id {self._file_id!r}" @@ -1841,7 +1851,7 @@ def __iter__(self) -> GridOutChunkIterator: return self def _create_cursor(self) -> None: - filter = {"files_id": self._id} + filter = {"files_id": {"$eq": self._id}} if self._next_chunk > 0: filter["n"] = {"$gte": self._next_chunk} _disallow_transactions(self._session) diff --git a/test/asynchronous/test_gridfs_bucket.py b/test/asynchronous/test_gridfs_bucket.py index 6b3503d122..ac850f1063 100644 --- a/test/asynchronous/test_gridfs_bucket.py +++ b/test/asynchronous/test_gridfs_bucket.py @@ -33,6 +33,7 @@ import gridfs from bson.binary import Binary from bson.int64 import Int64 +from bson.min_key import MinKey from bson.objectid import ObjectId from bson.son import SON from gridfs.errors import CorruptGridFile, NoFile @@ -42,6 +43,7 @@ NotPrimaryError, ServerSelectionTimeoutError, WriteConcernError, + WriteError, ) from pymongo.read_preferences import ReadPreference from test.asynchronous import AsyncIntegrationTest, async_client_context, unittest @@ -475,6 +477,34 @@ async def test_abort(self): await gin.write(b"test4") self.assertEqual(0, await self.db.fs.chunks.count_documents({"files_id": gin._id})) + # Server versions older than 5.0 do not support document values with + # "$"-prefixed keys. + @async_client_context.require_version_min(5, 0) + async def test_abort_with_injected_file_id_does_not_delete_other_files(self): + bucket = self.fs + file1_bytes = b"\x11\x22\x33\x44" + await bucket.upload_from_stream("file1", file1_bytes) + + upload_stream = bucket.open_upload_stream_with_id( + {"$gt": MinKey()}, "file2", chunk_size_bytes=2 + ) + await upload_stream.write(b"\x55\x66\x77\x88") + try: + await upload_stream.abort() + except WriteError: + # Newer server versions reject an equality match on "_id" whose + # operand itself resembles a query operator, even when the + # driver safely wraps it in "$eq". The chunks are still cleaned + # up in that case; only the (never created) files document + # delete is rejected. + pass + + self.assertEqual( + file1_bytes, await (await bucket.open_download_stream_by_name("file1")).read() + ) + with self.assertRaises(NoFile): + await bucket.open_download_stream_by_name("file2") + async def test_download_to_stream(self): file1 = BytesIO(b"hello world") # Test with one chunk. diff --git a/test/gridfs/queries-use-eq.json b/test/gridfs/queries-use-eq.json new file mode 100644 index 0000000000..e068168b91 --- /dev/null +++ b/test/gridfs/queries-use-eq.json @@ -0,0 +1,528 @@ +{ + "description": "gridfs-queriesUseEq", + "schemaVersion": "1.1", + "createEntities": [ + { + "client": { + "id": "client0", + "observeEvents": [ + "commandStartedEvent" + ] + } + }, + { + "database": { + "id": "database0", + "client": "client0", + "databaseName": "gridfs-tests" + } + }, + { + "bucket": { + "id": "bucket0", + "database": "database0" + } + }, + { + "collection": { + "id": "bucket0_files_collection", + "database": "database0", + "collectionName": "fs.files" + } + }, + { + "collection": { + "id": "bucket0_chunks_collection", + "database": "database0", + "collectionName": "fs.chunks" + } + } + ], + "initialData": [ + { + "collectionName": "fs.files", + "databaseName": "gridfs-tests", + "documents": [ + { + "_id": { + "$oid": "000000000000000000000001" + }, + "length": 2, + "chunkSize": 4, + "uploadDate": { + "$date": "1970-01-01T00:00:00.000Z" + }, + "filename": "file-1" + }, + { + "_id": { + "$oid": "000000000000000000000002" + }, + "length": 0, + "chunkSize": 4, + "uploadDate": { + "$date": "1970-01-01T00:00:00.000Z" + }, + "filename": "file-2" + } + ] + }, + { + "collectionName": "fs.chunks", + "databaseName": "gridfs-tests", + "documents": [ + { + "_id": { + "$oid": "000000000000000000000011" + }, + "files_id": { + "$oid": "000000000000000000000001" + }, + "n": 0, + "data": { + "$binary": { + "base64": "ESI=", + "subType": "00" + } + } + } + ] + } + ], + "_yamlAnchors": { + "injectedId": { + "$gt": { + "$minKey": 1 + } + }, + "unchangedOutcome": [ + { + "collectionName": "fs.files", + "databaseName": "gridfs-tests", + "documents": [ + { + "_id": { + "$oid": "000000000000000000000001" + }, + "length": 2, + "chunkSize": 4, + "uploadDate": { + "$date": "1970-01-01T00:00:00.000Z" + }, + "filename": "file-1" + }, + { + "_id": { + "$oid": "000000000000000000000002" + }, + "length": 0, + "chunkSize": 4, + "uploadDate": { + "$date": "1970-01-01T00:00:00.000Z" + }, + "filename": "file-2" + } + ] + }, + { + "collectionName": "fs.chunks", + "databaseName": "gridfs-tests", + "documents": [ + { + "_id": { + "$oid": "000000000000000000000011" + }, + "files_id": { + "$oid": "000000000000000000000001" + }, + "n": 0, + "data": { + "$binary": { + "base64": "ESI=", + "subType": "00" + } + } + } + ] + } + ] + }, + "tests": [ + { + "description": "download uses $eq to query the files and chunks collections", + "operations": [ + { + "name": "download", + "object": "bucket0", + "arguments": { + "id": { + "$oid": "000000000000000000000001" + } + }, + "expectResult": { + "$$matchesHexBytes": "1122" + } + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "find": "fs.files", + "filter": { + "_id": { + "$eq": { + "$oid": "000000000000000000000001" + } + } + } + } + } + }, + { + "commandStartedEvent": { + "command": { + "find": "fs.chunks", + "filter": { + "files_id": { + "$eq": { + "$oid": "000000000000000000000001" + } + } + } + } + } + } + ] + } + ] + }, + { + "description": "delete uses $eq to delete the files collection document and chunks", + "operations": [ + { + "name": "delete", + "object": "bucket0", + "arguments": { + "id": { + "$oid": "000000000000000000000001" + } + } + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "delete": "fs.files", + "deletes": [ + { + "q": { + "_id": { + "$eq": { + "$oid": "000000000000000000000001" + } + } + }, + "limit": 1 + } + ] + } + } + }, + { + "commandStartedEvent": { + "command": { + "delete": "fs.chunks", + "deletes": [ + { + "q": { + "files_id": { + "$eq": { + "$oid": "000000000000000000000001" + } + } + }, + "limit": 0 + } + ] + } + } + } + ] + } + ], + "outcome": [ + { + "collectionName": "fs.files", + "databaseName": "gridfs-tests", + "documents": [ + { + "_id": { + "$oid": "000000000000000000000002" + }, + "length": 0, + "chunkSize": 4, + "uploadDate": { + "$date": "1970-01-01T00:00:00.000Z" + }, + "filename": "file-2" + } + ] + }, + { + "collectionName": "fs.chunks", + "databaseName": "gridfs-tests", + "documents": [] + } + ] + }, + { + "description": "rename uses $eq to update the files collection document", + "operations": [ + { + "name": "rename", + "object": "bucket0", + "arguments": { + "id": { + "$oid": "000000000000000000000001" + }, + "newFilename": "file-1-renamed" + } + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "update": "fs.files", + "updates": [ + { + "q": { + "_id": { + "$eq": { + "$oid": "000000000000000000000001" + } + } + }, + "u": { + "$set": { + "filename": "file-1-renamed" + } + }, + "multi": { + "$$unsetOrMatches": false + }, + "upsert": { + "$$unsetOrMatches": false + } + } + ] + } + } + } + ] + } + ], + "outcome": [ + { + "collectionName": "fs.files", + "databaseName": "gridfs-tests", + "documents": [ + { + "_id": { + "$oid": "000000000000000000000001" + }, + "length": 2, + "chunkSize": 4, + "uploadDate": { + "$date": "1970-01-01T00:00:00.000Z" + }, + "filename": "file-1-renamed" + }, + { + "_id": { + "$oid": "000000000000000000000002" + }, + "length": 0, + "chunkSize": 4, + "uploadDate": { + "$date": "1970-01-01T00:00:00.000Z" + }, + "filename": "file-2" + } + ] + } + ] + }, + { + "description": "delete with a file id containing a query operator does not delete any files", + "operations": [ + { + "name": "delete", + "object": "bucket0", + "arguments": { + "id": { + "$gt": { + "$minKey": 1 + } + } + }, + "expectError": { + "isError": true + } + } + ], + "outcome": [ + { + "collectionName": "fs.files", + "databaseName": "gridfs-tests", + "documents": [ + { + "_id": { + "$oid": "000000000000000000000001" + }, + "length": 2, + "chunkSize": 4, + "uploadDate": { + "$date": "1970-01-01T00:00:00.000Z" + }, + "filename": "file-1" + }, + { + "_id": { + "$oid": "000000000000000000000002" + }, + "length": 0, + "chunkSize": 4, + "uploadDate": { + "$date": "1970-01-01T00:00:00.000Z" + }, + "filename": "file-2" + } + ] + }, + { + "collectionName": "fs.chunks", + "databaseName": "gridfs-tests", + "documents": [ + { + "_id": { + "$oid": "000000000000000000000011" + }, + "files_id": { + "$oid": "000000000000000000000001" + }, + "n": 0, + "data": { + "$binary": { + "base64": "ESI=", + "subType": "00" + } + } + } + ] + } + ] + }, + { + "description": "download with a file id containing a query operator does not return any files", + "operations": [ + { + "name": "download", + "object": "bucket0", + "arguments": { + "id": { + "$gt": { + "$minKey": 1 + } + } + }, + "expectError": { + "isClientError": true + } + } + ] + }, + { + "description": "rename with a file id containing a query operator does not rename any files", + "operations": [ + { + "name": "rename", + "object": "bucket0", + "arguments": { + "id": { + "$gt": { + "$minKey": 1 + } + }, + "newFilename": "injected" + }, + "expectError": { + "isError": true + } + } + ], + "outcome": [ + { + "collectionName": "fs.files", + "databaseName": "gridfs-tests", + "documents": [ + { + "_id": { + "$oid": "000000000000000000000001" + }, + "length": 2, + "chunkSize": 4, + "uploadDate": { + "$date": "1970-01-01T00:00:00.000Z" + }, + "filename": "file-1" + }, + { + "_id": { + "$oid": "000000000000000000000002" + }, + "length": 0, + "chunkSize": 4, + "uploadDate": { + "$date": "1970-01-01T00:00:00.000Z" + }, + "filename": "file-2" + } + ] + }, + { + "collectionName": "fs.chunks", + "databaseName": "gridfs-tests", + "documents": [ + { + "_id": { + "$oid": "000000000000000000000011" + }, + "files_id": { + "$oid": "000000000000000000000001" + }, + "n": 0, + "data": { + "$binary": { + "base64": "ESI=", + "subType": "00" + } + } + } + ] + } + ] + } + ] +} diff --git a/test/retryable_reads/unified/gridfs-download-serverErrors.json b/test/retryable_reads/unified/gridfs-download-serverErrors.json index 5bb7eee0b2..dc87dadce4 100644 --- a/test/retryable_reads/unified/gridfs-download-serverErrors.json +++ b/test/retryable_reads/unified/gridfs-download-serverErrors.json @@ -128,7 +128,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -141,7 +143,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -205,7 +209,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -218,7 +224,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -282,7 +290,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -295,7 +305,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -359,7 +371,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -372,7 +386,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -436,7 +452,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -449,7 +467,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -513,7 +533,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -526,7 +548,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -590,7 +614,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -603,7 +629,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -667,7 +695,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -680,7 +710,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -744,7 +776,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -757,7 +791,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -821,7 +857,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -834,7 +872,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -898,7 +938,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -911,7 +953,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -975,7 +1019,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -988,7 +1034,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -1077,7 +1125,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, diff --git a/test/retryable_reads/unified/gridfs-download.json b/test/retryable_reads/unified/gridfs-download.json index 69fe8ff7c8..9b2f1d93d4 100644 --- a/test/retryable_reads/unified/gridfs-download.json +++ b/test/retryable_reads/unified/gridfs-download.json @@ -109,7 +109,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -173,7 +175,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -186,7 +190,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -283,7 +289,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -339,7 +347,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, @@ -352,7 +362,9 @@ "find": "fs.files", "filter": { "_id": { - "$oid": "000000000000000000000001" + "$eq": { + "$oid": "000000000000000000000001" + } } } }, diff --git a/test/test_gridfs_bucket.py b/test/test_gridfs_bucket.py index 4d1ca7b811..90b063a9b1 100644 --- a/test/test_gridfs_bucket.py +++ b/test/test_gridfs_bucket.py @@ -33,6 +33,7 @@ import gridfs from bson.binary import Binary from bson.int64 import Int64 +from bson.min_key import MinKey from bson.objectid import ObjectId from bson.son import SON from gridfs.errors import CorruptGridFile, NoFile @@ -41,6 +42,7 @@ NotPrimaryError, ServerSelectionTimeoutError, WriteConcernError, + WriteError, ) from pymongo.read_preferences import ReadPreference from pymongo.synchronous.mongo_client import MongoClient @@ -445,6 +447,32 @@ def test_abort(self): gin.write(b"test4") self.assertEqual(0, self.db.fs.chunks.count_documents({"files_id": gin._id})) + # Server versions older than 5.0 do not support document values with + # "$"-prefixed keys. + @client_context.require_version_min(5, 0) + def test_abort_with_injected_file_id_does_not_delete_other_files(self): + bucket = self.fs + file1_bytes = b"\x11\x22\x33\x44" + bucket.upload_from_stream("file1", file1_bytes) + + upload_stream = bucket.open_upload_stream_with_id( + {"$gt": MinKey()}, "file2", chunk_size_bytes=2 + ) + upload_stream.write(b"\x55\x66\x77\x88") + try: + upload_stream.abort() + except WriteError: + # Newer server versions reject an equality match on "_id" whose + # operand itself resembles a query operator, even when the + # driver safely wraps it in "$eq". The chunks are still cleaned + # up in that case; only the (never created) files document + # delete is rejected. + pass + + self.assertEqual(file1_bytes, (bucket.open_download_stream_by_name("file1")).read()) + with self.assertRaises(NoFile): + bucket.open_download_stream_by_name("file2") + def test_download_to_stream(self): file1 = BytesIO(b"hello world") # Test with one chunk.