Skip to content
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

Correct content-disposition response header to be of type 'attachment' #1570

Merged

Conversation

vierbergenlars
Copy link
Contributor

@vierbergenlars vierbergenlars commented Aug 17, 2023

Having Content-Disposition set to 'form-data' is only valid in multipart form submissions (in the request), never in a response.
In a response, the content-disposition should only be set to either 'attachment' or 'inline': https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition#as_a_response_header_for_the_main_body

Given the name="attachment" part of the current (incorrect) content-disposition header, I believe the goal was to have content-disposition: attachment set as header.

When there is no filename available for the content, Content-Disposition: attachment should be set as well to always force a download of the content, otherwise a browser would fall back to the default (which is inline).

Using Content-Disposition: inline is problematic for resources created by untrusted users: HTML pages can contain scripts that would run in the context of the API, so care should be taken to always set Content-Disposition: attachment.

Having Content-Disposition set to 'form-data' is only valid in multipart form submissions (in the request), never in a response.
In a response, the content-disposition should only be set to either 'attachment' or 'inline'
@paulcwarren
Copy link
Owner

Thanks Lars

@paulcwarren paulcwarren merged commit 86d0321 into paulcwarren:main Aug 18, 2023
6 checks passed
@vierbergenlars vierbergenlars deleted the correct-content-disposition branch August 24, 2023 14:03
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.

None yet

2 participants