diff --git a/sentry_sdk/integrations/pymongo.py b/sentry_sdk/integrations/pymongo.py index 22da5d18ff..ab6d8add81 100644 --- a/sentry_sdk/integrations/pymongo.py +++ b/sentry_sdk/integrations/pymongo.py @@ -86,6 +86,36 @@ def _strip_pii(command: "Dict[str, Any]") -> "Dict[str, Any]": return command +def _bytes_safe_str(value: "Any") -> str: + """ + Convert a BSON value to a string without triggering ``BytesWarning``. + + ``str()`` on a ``bytes`` instance emits ``BytesWarning`` under + ``python -b`` and produces a useless ``b'...'`` representation, so + decode instead. This is for human-readable BSON values (collection + names etc.); for opaque identifiers such as the ``lsid`` session id + use ``_bytes_to_hex()`` instead, since decoding random bytes as UTF-8 + is lossy and would make different ids collide. + """ + if isinstance(value, bytes): + return value.decode("utf-8", errors="replace") + return str(value) + + +def _bytes_to_hex(value: "Any") -> str: + """ + Render a binary identifier (e.g. the BSON ``lsid.id`` ``Binary``, a + ``bytes`` subclass) as a stable, lossless hex string. + + ``str()`` on it emits ``BytesWarning`` under ``python -b`` (see #4782), + and decoding it as UTF-8 would be lossy: session ids are random bytes, + so replacement characters would make distinct ids collide. + """ + if isinstance(value, bytes): + return value.hex() + return str(value) + + def _get_db_data(event: "Any") -> "Dict[str, Any]": data = {} @@ -152,7 +182,7 @@ def started(self, event: "CommandStartedEvent") -> None: elif not should_send_default_pii(): command = _strip_pii(command) - query = json.dumps(command, default=str) + query = json.dumps(command, default=_bytes_safe_str) if has_span_streaming_enabled(client.options): span_first_data = { @@ -205,7 +235,7 @@ def started(self, event: "CommandStartedEvent") -> None: try: if lsid: lsid_id = lsid["id"] - data["operation_ids"]["session"] = str(lsid_id) + data["operation_ids"]["session"] = _bytes_to_hex(lsid_id) except KeyError: pass diff --git a/tests/integrations/pymongo/test_pymongo.py b/tests/integrations/pymongo/test_pymongo.py index 5bf6bb4e2c..fee7aae60b 100644 --- a/tests/integrations/pymongo/test_pymongo.py +++ b/tests/integrations/pymongo/test_pymongo.py @@ -1,11 +1,20 @@ +import datetime +import warnings + import pytest +from bson.binary import Binary from mockupdb import MockupDB, OpQuery from pymongo import MongoClient +from pymongo.monitoring import CommandStartedEvent, CommandSucceededEvent import sentry_sdk from sentry_sdk import capture_message, start_transaction from sentry_sdk.consts import SPANDATA -from sentry_sdk.integrations.pymongo import PyMongoIntegration, _strip_pii +from sentry_sdk.integrations.pymongo import ( + CommandTracer, + PyMongoIntegration, + _strip_pii, +) @pytest.fixture(scope="session") @@ -840,3 +849,124 @@ def test_span_streaming_status_on_failure(sentry_init, capture_items, mongo_serv assert segment["name"] == "test_segment" assert db_span["status"] == "error" + + +def test_bytes_safe_str(): + """_bytes_safe_str decodes bytes instead of str()-ing them (#4782).""" + from sentry_sdk.integrations.pymongo import _bytes_safe_str, _bytes_to_hex + + with warnings.catch_warnings(): + warnings.simplefilter("error", BytesWarning) + assert _bytes_safe_str(b"test_collection") == "test_collection" + # Invalid UTF-8 must not raise, it is replaced instead + assert _bytes_safe_str(b"caf\xe9") == "caf\ufffd" + # Non-bytes values keep the normal str() behavior + assert _bytes_safe_str(42) == "42" + assert _bytes_safe_str("plain") == "plain" + + # Binary identifiers must be hex-encoded, not decoded: session ids + # are random bytes, and UTF-8 decoding them is lossy (collisions). + assert _bytes_to_hex(b"\x00\x01\xff") == "0001ff" + assert _bytes_to_hex(42) == "42" + assert _bytes_to_hex("plain") == "plain" + # Distinct random ids stay distinct (the collision Bugbot flagged) + id1 = Binary(bytes(range(16)), 4) + id2 = Binary(bytes(range(1, 17)), 4) + assert _bytes_to_hex(id1) != _bytes_to_hex(id2) + assert _bytes_to_hex(id1) == "000102030405060708090a0b0c0d0e0f" + + +def _make_started_event(command, request_id=1): + return CommandStartedEvent( + command=command, + database_name="test_db", + request_id=request_id, + connection_id=("localhost", 27017), + operation_id=request_id, + ) + + +def _make_succeeded_event(command_name, request_id=1): + return CommandSucceededEvent( + duration=datetime.timedelta(seconds=1), + reply={"ok": 1}, + command_name=command_name, + request_id=request_id, + connection_id=("localhost", 27017), + operation_id=request_id, + ) + + +@pytest.mark.parametrize("with_pii", [False, True]) +def test_bytes_lsid_does_not_raise_byteswarning(sentry_init, capture_events, with_pii): + """ + The BSON logical session id (``lsid.id``) is a ``Binary`` instance, which + is a subclass of ``bytes``. ``str()`` on it raises ``BytesWarning`` under + ``python -b`` (see #4782). Turn BytesWarning into an error and make sure + the span is created with a decoded session id. + """ + sentry_init( + integrations=[PyMongoIntegration()], + traces_sample_rate=1.0, + send_default_pii=with_pii, + ) + events = capture_events() + + tracer = CommandTracer() + session_id = Binary(bytes(range(16)), 4) + started = _make_started_event( + { + "find": "test_collection", + "lsid": {"id": session_id}, + } + ) + succeeded = _make_succeeded_event("find") + + with warnings.catch_warnings(): + warnings.simplefilter("error", BytesWarning) + with start_transaction(): + tracer.started(started) + tracer.succeeded(succeeded) + + (event,) = events + (span,) = event["spans"] + + session = span["data"]["operation_ids"]["session"] + assert isinstance(session, str) + assert "b'" not in session + # Random binary ids must round-trip losslessly (hex), not be decoded + # as UTF-8 -- decoding random bytes loses information and collides. + assert session == bytes(range(16)).hex() + + +def test_bytes_collection_name_in_query_does_not_raise_byteswarning( + sentry_init, capture_events +): + """ + BSON values in the command can be ``bytes`` (e.g. a bytes collection + name). ``json.dumps(..., default=str)`` calls ``str()`` on them, which + raises ``BytesWarning`` under ``python -b`` (#4782). The query JSON must + contain the decoded name, not ``b'...'``. + """ + sentry_init( + integrations=[PyMongoIntegration()], + traces_sample_rate=1.0, + send_default_pii=True, + ) + events = capture_events() + + tracer = CommandTracer() + started = _make_started_event({"find": b"test_collection", "limit": 1}) + succeeded = _make_succeeded_event("find") + + with warnings.catch_warnings(): + warnings.simplefilter("error", BytesWarning) + with start_transaction(): + tracer.started(started) + tracer.succeeded(succeeded) + + (event,) = events + (span,) = event["spans"] + + assert "test_collection" in span["description"] + assert "b'" not in span["description"]