graphene-django icon indicating copy to clipboard operation
graphene-django copied to clipboard

Prevent TypeError when bytes passed to cursor.execute with debug middleware

Open bpartridge opened this issue 3 years ago • 3 comments

If DjangoDebugMiddleware is installed, calling cursor.execute(b) where b is a bytes object causes the recording (and thus the entire database call) to throw a TypeError due to https://github.com/graphql-python/graphene-django/blob/775644b5369bdc5fbb45d3535ae391a069ebf9d4/graphene_django/debug/sql/tracking.py#L126 :

"is_select": sql.lower().strip().startswith("select"),

Calling execute with a bytes parameter, to my knowledge, is not currently done within the high-level abstractions in the Django ORM, but is very much supported by psycopg2, as evidenced by the use in psycopg2's own execute_values in https://github.com/psycopg/psycopg2/blob/2_9_3/lib/extras.py#L1270 which codebases may use when bypassing the ORM:

cur.execute(b''.join(parts))

This fix ensures that the sql parameter is safely decoded before scanning whether it begins with SELECT; since this is the only usage, the change is trivial.

The only workaround if code calls execute_values is to disable the DjangoDebugMiddleware altogether, which is far from ideal.

bpartridge avatar Feb 10 '22 02:02 bpartridge

Just following up on this - do I need to do anything to move this into review? I can add tests but wanted to confirm that this wouldn't be a wontfix before doing so.

bpartridge avatar Feb 21 '22 16:02 bpartridge

@bpartridge Hello, sorry it took so long, we are revisiting this now, can you add tests please?

firaskafri avatar Apr 10 '23 07:04 firaskafri

Hi all, sorry I missed the comments from April here. I'll rebase this branch on main and add unit tests this week!

bpartridge avatar Sep 24 '23 18:09 bpartridge