Skip to content
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ repos:
- id: biome-check
verbose: true
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: 'v0.12.4'
rev: 'v0.12.5'
hooks:
- id: ruff
args: [--fix, --exit-non-zero-on-fix]
Expand Down
26 changes: 26 additions & 0 deletions debug_toolbar/panels/sql/decoders.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import base64
import json


class DebugToolbarJSONDecoder(json.JSONDecoder):
Copy link
Member

Choose a reason for hiding this comment

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

Rather than the approach you've taken here, you may want to consider going with something like:

import base64
import json

def object_hook(obj):
    for key, value in obj.items():
        if isinstance(value, str) and value.startswith("__djdt_binary__"):
            _, encoded = value.split("__djdt_binary__", maxsplit=1)
            obj[key] = base64.b64decode(encoded)
    return obj


class DebugToolbarJSONDecoder(json.JSONDecoder):
    """Custom JSON decoder that reconstructs binary data during parsing."""
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        if self.object_hook is None:
            self.object_hook = object_hook

This will require you to change the tests. The decoding will only work if there's an actual object being passed in. Most of the tests are passing in a list of values. Using an object/dictionary is more representative of what will actually be used since this is for parameters for a SQL query which is a dictionary.

Copy link
Author

Choose a reason for hiding this comment

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

Oh, I got it, I did not know about this object_hook possibility.

"""Custom JSON decoder that reconstructs binary data during parsing."""

def decode(self, s):
"""Override decode to apply reconstruction after parsing."""
obj = super().decode(s)
return self._reconstruct_params(obj)

def _reconstruct_params(self, params):
"""Reconstruct parameters, handling lists and dicts recursively."""
if isinstance(params, list):
return [self._reconstruct_params(param) for param in params]
elif isinstance(params, dict):
if "__djdt_binary__" in params:
return base64.b64decode(params["__djdt_binary__"])
else:
return {
key: self._reconstruct_params(value)
for key, value in params.items()
}
else:
return params
12 changes: 9 additions & 3 deletions debug_toolbar/panels/sql/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from django.utils.functional import cached_property
from django.utils.translation import gettext_lazy as _

from debug_toolbar.panels.sql.decoders import DebugToolbarJSONDecoder
from debug_toolbar.panels.sql.utils import is_select_query, reformat_sql
from debug_toolbar.toolbar import DebugToolbar

Expand Down Expand Up @@ -69,10 +70,15 @@ def clean(self):
cleaned_data["query"] = query
return cleaned_data

def _get_query_params(self):
"""Get reconstructed parameters for the current query"""
query = self.cleaned_data["query"]
return json.loads(query["params"], cls=DebugToolbarJSONDecoder)

def select(self):
query = self.cleaned_data["query"]
sql = query["raw_sql"]
params = json.loads(query["params"])
params = self._get_query_params()
with self.cursor as cursor:
cursor.execute(sql, params)
headers = [d[0] for d in cursor.description]
Expand All @@ -82,7 +88,7 @@ def select(self):
def explain(self):
query = self.cleaned_data["query"]
sql = query["raw_sql"]
params = json.loads(query["params"])
params = self._get_query_params()
vendor = query["vendor"]
with self.cursor as cursor:
if vendor == "sqlite":
Expand All @@ -101,7 +107,7 @@ def explain(self):
def profile(self):
query = self.cleaned_data["query"]
sql = query["raw_sql"]
params = json.loads(query["params"])
params = self._get_query_params()
with self.cursor as cursor:
cursor.execute("SET PROFILING=1") # Enable profiling
cursor.execute(sql, params) # Execute SELECT
Expand Down
6 changes: 6 additions & 0 deletions debug_toolbar/panels/sql/tracking.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import base64
import contextlib
import contextvars
import datetime
Expand Down Expand Up @@ -126,6 +127,11 @@ def _decode(self, param):
if isinstance(param, dict):
return {key: self._decode(value) for key, value in param.items()}

# Handle binary data (e.g., GeoDjango EWKB geometry data)
if isinstance(param, (bytes, bytearray)):
# Mark as binary data for later reconstruction
return {"__djdt_binary__": base64.b64encode(param).decode("ascii")}
Copy link
Member

Choose a reason for hiding this comment

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

We should move this logic into a shared utility that way we don't have the magic value "__djdt_binary__" floating around the code-base. Plus it would be clearer how the logic gets paired together.

I'm also curious if we should be using this with the Store logic. If we do, then we could potentially ignore this and handle it within DebugToolbarJSONEncoder and then have DebugToolbarJSONDecoder as above. What do you think?

Copy link
Author

@eduzen eduzen Aug 23, 2025

Choose a reason for hiding this comment

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

Thank you for the feedback @tim-schilling ! I think an encoders.py or moving even everything into utils.py is a more consistent way.
About your second question, what do you mean with using the store logic?

Copy link
Member

Choose a reason for hiding this comment

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

The Store logic converts all the panels data into JSON. So this binary parameter would eventually be converted to JSON to be added to the Store. So if the Store handles binary data properly, it may remove the need for the SQL panel to deal with it itself. Does that make more sense?


# make sure datetime, date and time are converted to string by force_str
CONVERT_TYPES = (datetime.datetime, datetime.date, datetime.time)
try:
Expand Down
8 changes: 8 additions & 0 deletions docs/changes.rst
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
Change log
==========

Pending
-------

* Fixed SQL Explain functionality for GeoDjango queries with binary parameters.
Binary data (such as EWKB geometry) is now properly handled through base64
encoding, preventing "parse error - invalid geometry" errors when using
Explain on spatial queries.

6.0.0 (2025-07-22)
------------------

Expand Down
137 changes: 137 additions & 0 deletions tests/panels/test_sql_geodjango_fix.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
"""
Tests for GeoDjango binary parameter handling fix
"""

import base64
import json

from debug_toolbar.panels.sql.decoders import DebugToolbarJSONDecoder
from debug_toolbar.panels.sql.tracking import NormalCursorMixin

from ..base import BaseTestCase


class MockCursor:
"""Mock cursor for testing"""


class MockConnection:
"""Mock database connection for testing"""

vendor = "postgresql"
alias = "default"


class MockLogger:
"""Mock logger for testing"""

def record(self, **kwargs):
pass


class TestCursor(NormalCursorMixin):
"""Test cursor that can be instantiated"""

def __init__(self):
self.cursor = MockCursor()
self.db = MockConnection()
self.logger = MockLogger()


class GeoDjangoBinaryParameterTest(BaseTestCase):
"""Test cases for GeoDjango binary parameter handling"""

def test_binary_parameter_encoding_decoding(self):
"""Test that binary parameters are properly encoded and decoded"""
cursor = TestCursor()

# Test binary data similar to GeoDjango EWKB geometry
binary_data = b"\x01\x01\x00\x00\x20\xe6\x10\x00\x00\xff\xfe\xfd"
encoded = cursor._decode(binary_data)

self.assertIsInstance(encoded, dict)
self.assertIn("__djdt_binary__", encoded)

expected_b64 = base64.b64encode(binary_data).decode("ascii")
self.assertEqual(encoded["__djdt_binary__"], expected_b64)

json_params = json.dumps([encoded])
reconstructed = json.loads(json_params, cls=DebugToolbarJSONDecoder)

self.assertEqual(len(reconstructed), 1)
self.assertEqual(reconstructed[0], binary_data)
self.assertIsInstance(reconstructed[0], bytes)
Comment on lines +61 to +63
Copy link
Member

Choose a reason for hiding this comment

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

I don't think there's a benefit to being this verbose. Try to be more concise such as:

Suggested change
self.assertEqual(len(reconstructed), 1)
self.assertEqual(reconstructed[0], binary_data)
self.assertIsInstance(reconstructed[0], bytes)
self.assertEqual(reconstructed, [binary_data])


def test_mixed_parameter_types(self):
"""Test that mixed parameter types are handled correctly"""
cursor = TestCursor()

params = [
"string_param",
42,
b"\x01\x02\x03",
None,
["nested", "list"],
]

encoded_params = [cursor._decode(p) for p in params]

json_str = json.dumps(encoded_params)
reconstructed = json.loads(json_str, cls=DebugToolbarJSONDecoder)

self.assertEqual(reconstructed[0], "string_param") # string unchanged
self.assertEqual(reconstructed[1], 42) # int unchanged
self.assertEqual(reconstructed[2], b"\x01\x02\x03") # binary restored
self.assertIsNone(reconstructed[3]) # None unchanged
self.assertEqual(reconstructed[4], ["nested", "list"]) # list unchanged

def test_nested_binary_data(self):
"""Test binary data nested in lists and dicts"""
cursor = TestCursor()

nested_params = [
[b"\x01\x02", "string", b"\x03\x04"],
{"key": b"\x05\x06", "other": "value"},
]

encoded = [cursor._decode(p) for p in nested_params]

json_str = json.dumps(encoded)
reconstructed = json.loads(json_str, cls=DebugToolbarJSONDecoder)

self.assertEqual(reconstructed[0][0], b"\x01\x02")
self.assertEqual(reconstructed[0][1], "string")
self.assertEqual(reconstructed[0][2], b"\x03\x04")

self.assertEqual(reconstructed[1]["key"], b"\x05\x06")
self.assertEqual(reconstructed[1]["other"], "value")

def test_empty_binary_data(self):
"""Test handling of empty binary data"""
cursor = TestCursor()

empty_bytes = b""
encoded = cursor._decode(empty_bytes)

self.assertIsInstance(encoded, dict)
self.assertIn("__djdt_binary__", encoded)

json_str = json.dumps([encoded])
reconstructed = json.loads(json_str, cls=DebugToolbarJSONDecoder)

self.assertEqual(reconstructed[0], empty_bytes)

def test_bytearray_support(self):
"""Test that bytearray is also handled as binary data"""
cursor = TestCursor()

byte_array = bytearray(b"\x01\x02\x03\x04")
encoded = cursor._decode(byte_array)

self.assertIn("__djdt_binary__", encoded)

json_str = json.dumps([encoded])
reconstructed = json.loads(json_str, cls=DebugToolbarJSONDecoder)

self.assertEqual(reconstructed[0], byte_array)
self.assertIsInstance(reconstructed[0], bytes)