Skip to content

PYTHON-5873 Reject whitespace in ObjectId hex strings #3134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
GoldenEmperor1177 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

PYTHON-5873 Reject whitespace in ObjectId hex strings #3134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions bson/objectid.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ def __init__(self, oid: Optional[Union[str, ObjectId, bytes]] = None) -> None:
self.__id = bytes.fromhex(oid)
except (TypeError, ValueError):
_raise_invalid_id(oid)
# bytes.fromhex() skips ASCII whitespace, so a 24-character
# string can decode to fewer than 12 bytes.
if len(self.__id) != 12:
_raise_invalid_id(oid)
else:
_raise_invalid_id(oid)
elif isinstance(oid, ObjectId):
Expand Down
6 changes: 6 additions & 0 deletions doc/changelog.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,15 @@ Bug fixes
``ValueError``, even though ``maxPoolSize=0`` means "no limit". The
``minPoolSize <= maxPoolSize`` validation now treats both ``0`` and ``None``
as unlimited (`PYTHON-6149`_).
- Fixed a bug where :class:`~bson.objectid.ObjectId` accepted a 24-character
string containing whitespace and created an ObjectId shorter than 12 bytes
from it. It now raises :class:`~bson.errors.InvalidId`, and
:meth:`~bson.objectid.ObjectId.is_valid` returns ``False`` for such a string
(`PYTHON-5873`_).

.. _PYTHON-6172: https://jira.mongodb.org/browse/PYTHON-6172
.. _PYTHON-6149: https://jira.mongodb.org/browse/PYTHON-6149
.. _PYTHON-5873: https://jira.mongodb.org/browse/PYTHON-5873

Changes in Version 4.19.0 (2026/10/14)
--------------------------------------
Expand Down
12 changes: 12 additions & 0 deletions test/test_objectid.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,16 @@ def test_from_hex(self):
ObjectId("123456789012123456789012")
self.assertRaises(InvalidId, ObjectId, "123456789012123456789G12")

def test_from_hex_whitespace(self):
# bytes.fromhex() skips whitespace, which must not shorten the ObjectId.
self.assertRaises(InvalidId, ObjectId, "0123 0011")
self.assertRaises(InvalidId, ObjectId, "00 " * 8)
self.assertRaises(InvalidId, ObjectId, "01\n" * 8)
self.assertRaises(InvalidId, ObjectId, " " * 24)
# The string form of an ObjectId round-trips.
oid = ObjectId()
self.assertEqual(oid, ObjectId(str(oid)))

Comment on lines +66 to +69

@NoahStapp NoahStapp Oct 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't related to the rest of the test. Existing tests already verify that ObjectId round-trips, and the ObjectId being created here doesn't contain any whitespace.

def test_repr_str(self):
self.assertEqual(
repr(ObjectId("1234567890abcdef12345678")), "ObjectId('1234567890abcdef12345678')"
Expand Down Expand Up @@ -154,6 +164,8 @@ def test_is_valid(self):
self.assertFalse(ObjectId.is_valid(""))
self.assertFalse(ObjectId.is_valid("12345678901"))
self.assertFalse(ObjectId.is_valid("1234567890123"))
self.assertFalse(ObjectId.is_valid("0123 0011"))
self.assertFalse(ObjectId.is_valid(" " * 24))
Comment on lines +167 to +168

@NoahStapp NoahStapp Oct 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.assertFalse(ObjectId.is_valid("0123 0011"))
self.assertFalse(ObjectId.is_valid(" " * 24))
self.assertFalse(ObjectId.is_valid("0123 0011"))
self.assertFalse(ObjectId.is_valid("00 " * 8))
self.assertFalse(ObjectId.is_valid("01\n" * 8))
self.assertFalse(ObjectId.is_valid(" " * 24))

Consistency across both test cases.


self.assertTrue(ObjectId.is_valid(b"123456789012"))
self.assertTrue(ObjectId.is_valid("123456789012123456789012"))
Expand Down