never_cache()/cache_control() decorators raise error on duck-typed requests.

[Terence Honles]

The cache decorators cache_control, never_cache and sensitive_post_parameters no longer work with Django REST framework because they strictly check for an HttpRequest instance.

[Carlton Gibson]

Hi Terence — Thanks for the report. Can you post a traceback quickly, or better a reproduce? (But a traceback would be good.)

[Carlton Gibson]

3fd82a62415e748002435e7bad06b5017507777c for #32468 looks likely.

[Carlton Gibson]

The check has been in sensitive_post_parameters ​for a long time.

[Mariusz Felisiak]

I don't see how this can be a regression, these decorators use functions from ​django.utils.cache which are documented as accepting HttpResponse 🤔 Also Response defined in django-rest-framework is a subclass of HttpResponse.

[jason]

Replying to Mariusz Felisiak:

I don't see how this can be a regression, these decorators use functions from ​django.utils.cache which are documented as accepting HttpResponse 🤔 Also Response defined in django-rest-framework is a subclass of HttpResponse.

​Response has never inherited from HttpResponse which has been the source of many issues like this even before 4.0. I am experiencing a lot of issues right now converting Django method views into DRF class-based views right now because all of the decorators keep breaking.

[Mariusz Felisiak]

Replying to jason:

​Request has never inherited from HttpRequest which has been the source of many issues like this even before 4.0. I am experiencing a lot of issues right now converting Django method views into DRF class-based views right now because all of the decorators keep breaking.

Sorry, I confused HttpRequest with HttpResponse. Nevertheless, IMO, if DRF is duck typing HttpRequest it should be fixed in DRF, e.g. by adding __instancecheck__(). What do you think Tom? I'm happy to provide a patch to DRF.

[Carlton Gibson]

I think this is going to cause trouble for a lot of folks.

The sensitive_post_parameters has been as it is for many years, but the
​caching usage is standard DRF. (Again that's been there a long time.)

The check in 3fd82a62415e748002435e7bad06b5017507777c seems overly tight.
What methods are being called?
Surely any Request-a-like exposing those is OK?

I don't think we can just change Request to claim to be a HttpRequest.

​DRF has an `isinstance()` check for this very thing.

Introduced in ​https://github.com/encode/django-rest-framework/pull/5618

See also:

• ​https://github.com/encode/django-rest-framework/issues/5446
• ​https://github.com/encode/django-rest-framework/issues/3848

(and others)

[Mariusz Felisiak]

This error was added to catch misuses of never_cache() and cache_control() decorators without method_decorator().

[Mariusz Felisiak]

We've discussed this with Carlton, and see two possible solutions:

• allow for HttpRequest duck-typing, or
• changing errors to be based on missing method_decorator() call, maybe checking that request is not a callable 🤔 or sth similar.

[jason]

Replying to Mariusz Felisiak:

This error was added to catch misuses of never_cache() and cache_control() decorators without method_decorator().

Breaking functionality in order to provide a nice error message seems backwards.
Then again, I can't think of a good excuse for DRF not to simply inherit from HttpRequest.
I recently fell down a rabbit-hole of github issues that were closed without resolution regarding this conflict, where DRF refuses to inherit because "composition pattern", and django refuses not to check because "nice error message" over and over.

Something has to give right?

[Mariusz Felisiak]

Something has to give right?

This ticket is accepted. Have you seen my last comment? We agreed that this should be fixed in Django itself.

[jason]

Replying to Mariusz Felisiak:

Something has to give right?

This ticket is accepted. Have you seen my last comment? We agreed that this should be fixed in Django itself.

Just saw it, don't mean to beat a dead horse, thank you for your quick response ^_

[Mariusz Felisiak]

​PR

[GitHub <noreply@…>]

In 40165eec:

Fixed #33350 -- Reallowed using cache decorators with duck-typed HttpRequest.

Regression in 3fd82a62415e748002435e7bad06b5017507777c.

Thanks Terence Honles for the report.

[Mariusz Felisiak <felisiak.mariusz@…>]

In c1d2e8b:

[4.0.x] Fixed #33350 -- Reallowed using cache decorators with duck-typed HttpRequest.

Regression in 3fd82a62415e748002435e7bad06b5017507777c.

Thanks Terence Honles for the report.
Backport of 40165eecc40f9e223702a41a0cb0958515bb1f82 from main