Skip to content

made Browsable API base template cachable: omit CSRF token when unnecessary #7717

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 16, 2021

Conversation

jesteria
Copy link
Contributor

HTML responses generated by the Browsable API otherwise generate
inconsistent ETAGs -- due to the presence of CSRF tokens in the response
-- even when the API is read-only, (and as such when the response
contains no resource-modifying forms, i.e. neither POST nor PUT forms,
which might require the CSRF token).

While the template was appropriately including CSRF tokens only within
POST and PUT forms, its AJAX overlay included the CSRF token in every
response, regardless of whether it would be needed.

This change brings the logic of the script block into line with that
of the rest of the template -- and such that read-only APIs (and really
the Browsable API pages of any read-only resources) will not
needlessly include the CSRF token, and will now be safely cachable -- by
both back-end systems and by the user agent.

…essary

HTML responses generated by the Browsable API otherwise generate
inconsistent ETAGs -- due to the presence of CSRF tokens in the response
-- even when the API is read-only, (and as such when the response
contains no resource-modifying forms, i.e. neither POST nor PUT forms,
which might require the CSRF token).

While the template was appropriately including CSRF tokens only within
POST and PUT forms, its AJAX overlay included the CSRF token in *every*
response, regardless of whether it would be needed.

This change brings the logic of the `script` block into line with that
of the rest of the template -- and such that read-only APIs (and really
the Browsable API pages of *any* read-only resources) will not
needlessly include the CSRF token, and will now be safely cachable -- by
both back-end systems and by the user agent.
@tomchristie
Copy link
Member

Yup, I like this one. Thanks!

@tomchristie
Copy link
Member

Okay, actually I think I jumped the gun here, so I've opened a revert PR. Presumably we want to do the same for PATCH and DELETE forms too.

tomchristie added a commit that referenced this pull request Mar 17, 2021
stefanacin pushed a commit to stefanacin/django-rest-framework that referenced this pull request Mar 22, 2021
…essary (encode#7717)

HTML responses generated by the Browsable API otherwise generate
inconsistent ETAGs -- due to the presence of CSRF tokens in the response
-- even when the API is read-only, (and as such when the response
contains no resource-modifying forms, i.e. neither POST nor PUT forms,
which might require the CSRF token).

While the template was appropriately including CSRF tokens only within
POST and PUT forms, its AJAX overlay included the CSRF token in *every*
response, regardless of whether it would be needed.

This change brings the logic of the `script` block into line with that
of the rest of the template -- and such that read-only APIs (and really
the Browsable API pages of *any* read-only resources) will not
needlessly include the CSRF token, and will now be safely cachable -- by
both back-end systems and by the user agent.
stefanacin pushed a commit to stefanacin/django-rest-framework that referenced this pull request Mar 22, 2021
sigvef pushed a commit to sigvef/django-rest-framework that referenced this pull request Dec 3, 2022
…essary (encode#7717)

HTML responses generated by the Browsable API otherwise generate
inconsistent ETAGs -- due to the presence of CSRF tokens in the response
-- even when the API is read-only, (and as such when the response
contains no resource-modifying forms, i.e. neither POST nor PUT forms,
which might require the CSRF token).

While the template was appropriately including CSRF tokens only within
POST and PUT forms, its AJAX overlay included the CSRF token in *every*
response, regardless of whether it would be needed.

This change brings the logic of the `script` block into line with that
of the rest of the template -- and such that read-only APIs (and really
the Browsable API pages of *any* read-only resources) will not
needlessly include the CSRF token, and will now be safely cachable -- by
both back-end systems and by the user agent.
sigvef pushed a commit to sigvef/django-rest-framework that referenced this pull request Dec 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants