Opened 2 months ago

Closed 2 months ago

Last modified 2 months ago

#31877 closed Bug (fixed)

TemplateView.get_context_data()'s kwargs returns SimpleLazyObjects that causes a crash when filtering.

Reported by: Tim L. White Owned by: Adam (Chainz) Johnson
Component: Generic views Version: 3.1
Severity: Release blocker Keywords:
Cc: Adam (Chainz) Johnson, Tom Forbes Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Example Code that works in 3.0, but not in 3.1:

class OfferView(TemplateView):
    template_name = "offers/offer.html"

    def get_context_data(self, **kwargs):
        offer_slug = kwargs.get("offer_slug", "")

        offer = get_object_or_404(Account, slug=offer_slug)

        return {"offer": offer, "offer_slug": offer_slug}

In order to make this work in 3.1, you have to explicitly convert the result of kwargs.get() to a string to get the SimpleLazyObject to resolve:

class OfferView(TemplateView):
    template_name = "offers/offer.html"

    def get_context_data(self, **kwargs):
        offer_slug = kwargs.get("offer_slug", "")

        offer = get_object_or_404(Account, slug=str(offer_slug))

        return {"offer": offer, "offer_slug": offer_slug}

The error generated if you don't is:

Error binding parameter 0 - probably unsupported type

from django/db/backends/sqlite3/operations.py, line 144, in _quote_params_for_last_executed_query

In both cases, the urls.py looks like:

path(
        "/offers/<slug:offer_slug>/",
        OfferView.as_view(),
        name="offer_view",
    ),

When debugging, I found that offer_slug (coming in from kwargs.get) was of type 'SimpleLazyObject' in Django 3.1, and when I explicitly converted it to a string, get_object_or_404 behaved as expected.

This is using Python 3.7.8 with SQLite.

Change History (19)

comment:1 Changed 2 months ago by Tim L. White

Summary: In Django 3.1, get_object_or_404 does not resolve SimpleLazyObjects, and throws Error binding parameter 0 - probably unsupported type from SQLiteIn Django 3.1, get_object_or_404 does not resolve SimpleLazyObjects, and throws "Error binding parameter 0 - probably unsupported type" from SQLite

comment:2 Changed 2 months ago by Mariusz Felisiak

Cc: Adam (Chainz) Johnson added
Component: Database layer (models, ORM)Generic views
Keywords: SQLite removed
Severity: NormalRelease blocker
Summary: In Django 3.1, get_object_or_404 does not resolve SimpleLazyObjects, and throws "Error binding parameter 0 - probably unsupported type" from SQLiteTemplateView.get_context_data()'s kwargs returns SimpleLazyObjects that causes a crash when filtering.
Triage Stage: UnreviewedAccepted

Thanks for the report. get_object_or_404() and QuerySet.filter() with SimpleLazyObject throw the same exception in Django 2.2 or 3.0.

TemplateView.get_context_data()'s kwargs returns SimpleLazyObjects in Django 3.1 which causes a crash. Passing URL kwargs into context is deprecated (see #19878) but should still work in Django 3.1 and 3.2.

Regression in 4ed534758cb6a11df9f49baddecca5a6cdda9311.
Reproduced at 60626162f76f26d32a38d18151700cb041201fb3.

comment:3 Changed 2 months ago by Adam (Chainz) Johnson

Owner: changed from nobody to Adam (Chainz) Johnson
Status: newassigned

comment:4 Changed 2 months ago by Adam (Chainz) Johnson

Using lazy() instead of SimpleLazyObject() fixes this - PR is up.

comment:5 Changed 2 months ago by Mariusz Felisiak

Has patch: set

comment:6 Changed 2 months ago by Mariusz Felisiak

Triage Stage: AcceptedReady for checkin

comment:7 Changed 2 months ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: assignedclosed

In 20799cc0:

Fixes #31877 -- Used lazy() for TemplateView kwarg deprecation warning.

SimpleLazyObjects cause a crash when filtering.

Thanks Tim L. White for the report.
Regression in 4ed534758cb6a11df9f49baddecca5a6cdda9311.

comment:8 Changed 2 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In 9ae40d81:

[3.1.x] Fixes #31877 -- Used lazy() for TemplateView kwarg deprecation warning.

SimpleLazyObjects cause a crash when filtering.

Thanks Tim L. White for the report.
Regression in 4ed534758cb6a11df9f49baddecca5a6cdda9311.

Backport of 20799cc0a6d98816b9ef0577e24691bd26b80d7d from master

comment:9 Changed 2 months ago by Mariusz Felisiak

Resolution: fixed
Status: closednew

Passing deprecated keyword arguments to a queryset with lookups, e.g. Artist.objects.get(name__iexact=artist_name), still crashes on PostgreSQL:

django.db.utils.ProgrammingError: can't adapt type '__proxy__'

Thanks Mohit Solanki for the report.

comment:10 Changed 2 months ago by Adam (Chainz) Johnson

😟 Hmm this is a tough one.

psycopg2 uses type(obj) to look for its "adapter" - in its C extension: https://github.com/psycopg/psycopg2/blob/e14e3385b4809ec4223894f8c7a009b1560eb41d/psycopg/microprotocols.c#L151 . So this "proxy" approach may not be feasible.

I know of a more accurate proxy wrapper that proxies more attributes than Django's lazy objects - wrapt.ObjectProxy - https://wrapt.readthedocs.io/en/latest/wrappers.html#object-proxy. But docs there acknowledge that it can't even make type() work, whilst it makes isinstance() work.

Any ideas of alternative approaches?

comment:11 Changed 2 months ago by Mariusz Felisiak

Has patch: unset
Triage Stage: Ready for checkinAccepted

comment:12 Changed 2 months ago by Tom Forbes

There isn't really a general purpose way of wrapping primitive/arbitrary types like this in Python that won't hit some corner cases. You can make an object appear to be a duck by adapting it's quacking dynamically (i.e wrapt.ObjectProxy), but if someone looks close enough they can always see that it's actually dog. And on the whole that's a good thing, IMO.

Our use of kwargs makes this harder as we lose the ability to construct a container that can trigger the deprecation warning which would be the typical easy approach. There is no way to control what lands on the other side of get_context_data() (it's always a plain kwargs dict), and there is no way to construct a wrapper value that looks _exactly_ like the value it's wrapping.

That basically leaves only "crazy" approaches, some of which are fun to consider but none of which are suitable. Here's one that uses settrace() to do what we need:

import sys

class DeprecatedWrapper(dict):
    def __getitem__(self, key):
        warnings.warn("stop right there, scoundrel!")
        return super().__getitem__(key)

def wrap_kwargs(frame, *args):
    frame.f_locals['kwargs'] = DeprecatedWrapper(frame['kwargs'])
    sys.settrace(None)

class TemplateView(...):
    def get(...):
        ...
        sys.settrace(wrap_kwargs)
        context = self.get_context_data(**context_kwargs)
        return self.render_to_response(context)

Given these issues, I'm not sure if we can go ahead with deprecating this.

Last edited 2 months ago by Tom Forbes (previous) (diff)

comment:13 Changed 2 months ago by Tom Forbes

Cc: Tom Forbes added

comment:14 Changed 2 months ago by Mariusz Felisiak

Has patch: set

Agreed, we should revert it since we don't have a clear deprecation path.

PR

comment:15 Changed 2 months ago by Adam (Chainz) Johnson

I agree too.

comment:16 Changed 2 months ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: newclosed

In 04e87e79:

Refs #31877 -- Reverted "Fixes #31877 -- Used lazy() for TemplateView kwarg deprecation warning."

This reverts commit 20799cc0a6d98816b9ef0577e24691bd26b80d7d.

comment:17 Changed 2 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In bb8f6693:

Fixed #31877 -- Reverted "Fixed #19878 -- Deprecated TemplateView passing URL kwargs into context."

This reverts commit 4ed534758cb6a11df9f49baddecca5a6cdda9311.

comment:18 Changed 2 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In f247c66b:

[3.1.x] Refs #31877 -- Reverted "Fixes #31877 -- Used lazy() for TemplateView kwarg deprecation warning."

This reverts commit 20799cc0a6d98816b9ef0577e24691bd26b80d7d.

Backport of 04e87e79a0bd2b1b9fdc30f884a637a3268733f0 from master

comment:19 Changed 2 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In e81aa7a:

[3.1.x] Fixed #31877 -- Reverted "Fixed #19878 -- Deprecated TemplateView passing URL kwargs into context."

This reverts commit 4ed534758cb6a11df9f49baddecca5a6cdda9311.

Backport of bb8f66934d93faf80cd1a2dda65aaedce21a6fc5 from master

Note: See TracTickets for help on using tickets.
Back to Top