[Django] #34048: Do not add cache control header if is set to false or a falsy value
Django
--------------------------------------+-------------------------------
Reporter: Wiktor | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: dev
Severity: Normal | Keywords: cache control
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
--------------------------------------+-------------------------------
Currently when one sets no_cache to a falsy value, e.g.:
{{{
@cache_control(no_cache=False)
def some_view()
pass
}}}
Cache-Control header will be set to no_cache=False.
This might be confusing and might lead to a security issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/34048>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Wiktor | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: dev
Keywords: cache control | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Wiktor):
PR: https://github.com/django/django/pull/16099
--
Ticket URL: <https://code.djangoproject.com/ticket/34048#comment:1>
Django
-------------------------------+--------------------------------------
Reporter: Wiktor | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: cache control | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Old description:
> Currently when one sets no_cache to a falsy value, e.g.:
>
> {{{
> @cache_control(no_cache=False)
> def some_view()
> pass
> }}}
>
> Cache-Control header will be set to no_cache=False.
>
> This might be confusing and might lead to a security issue.
New description:
Currently when one sets no_cache to a falsy value, e.g.:
{{{
@cache_control(no_cache=False)
def some_view()
pass
}}}
Cache-Control header will be set to no_cache=False.
This might be confusing and might lead to a security issue.
Instead it should be just excluded.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/34048#comment:2>
Django
-------------------------------+--------------------------------------
Reporter: Wiktor | Owner: nobody
Type: Bug | Status: new
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: cache control | Triage Stage: Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
* cc: Wiktor (added)
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/34048#comment:3>
Django
Reporter: Wiktor | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jan Urban):
* cc: Jan Urban (added)
* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/34048#comment:4>
Django
--------------------------------------+------------------------------------
Reporter: Wiktor | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: cache control | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
* cc: Flavio Curella (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/34048#comment:5>
Django
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: assigned
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: cache control | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
* owner: nobody => Wiktor
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/34048#comment:6>
Django
--------------------------------------+------------------------------------
Reporter: Wiktor | Owner: Wiktor
Type: Cleanup/optimization | Status: assigned
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: cache control | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Jan Urban):
This ticket is about avoiding the possible misunderstanding of how `no-
cache` works in browser.
This ticket should not change the functionality of `no-cache` header and
the end result of the `patch_cache_control` and how the header is
interpreted. (As far as I know)
Needs more verification and discussion, if we should lead the hand of
others.
As mentioned as it could be security issue as well if the value is
interpreted wrongly by the others.
I do not know how exactly the `no-cache` works and what are the
conventions for it and best practice, but this sounded like it make it
more explicit.
--
Ticket URL: <https://code.djangoproject.com/ticket/34048#comment:7>
Django
Reporter: Wiktor | Owner: Wiktor
Type: | Status: closed
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution: needsinfo
Keywords: cache control | Triage Stage:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* status: assigned => closed
* resolution: => needsinfo
* stage: Accepted => Unreviewed
Comment:
Closing as "needsinfo" as I'm not sure how this can create a security
issue and why `False` should be treated differently. `True` always takes
precedence over other values, so passing `False` will not override. On the
other hand when `no-cache` is not set to `True` than the value is cast to
string and added to the list of field names. Please provide more details.
--
Ticket URL: <https://code.djangoproject.com/ticket/34048#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Wiktor | Owner: Wiktor
Type: | Status: closed
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution: needsinfo
Keywords: cache control | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Wiktor):
Setting
{{{
no_cache = False
cache_control(no_cache=no_cache)
}}}
will end up creating header
{{{
Cache-Control: no-cache=False
}}}
which is not intuitive and might lead to an unexpected caching behavior
(thus I mentioned potential security issue). What I'd expect is an empty
`Cache-Control`.
{{{
Cache-Control:
}}}
More context about no-cache could be found in RFC: https://www.rfc-
editor.org/rfc/rfc7234#section-5.2.2.2
The "no-cache" response directive indicates that the response MUST
NOT be used to satisfy a subsequent request without successful
validation on the origin server. This allows an origin server to
prevent a cache from using it to satisfy a request without contacting
it, even by caches that have been configured to send stale responses.
[...]
For `True` it works as expected (it just adds `Cache-Control: no-cache`).
What do you suggest in that case? I understand that maybe the behavior
should be changed for all the keyword arguments (True - turn them on,
False - turn them off), however I find `no-cache` more sensitive.
--
Ticket URL: <https://code.djangoproject.com/ticket/34048#comment:9>