fix: RFC 2183/5987 compliant Content-Disposition header in /view endpoint #11537
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…oint The Content-Disposition header was missing the required disposition-type (attachment;) and proper encoding for non-ASCII filenames. Changes: - Add `create_content_disposition_header()` helper function - Update all 4 Content-Disposition headers in view_image endpoint - Provide ASCII fallback filename for legacy clients - Add RFC 5987 UTF-8 encoded filename* parameter for international support This fixes third-party download libraries (e.g., Go's mime.ParseMediaType) failing to parse the filename correctly. Fixes #8914
RUiNtheExtinct
requested review from
Kosinkadink,
comfyanonymous and
guill
as code owners
Quotes in filenames would cause malformed Content-Disposition headers: filename="file"name.png" ← Invalid header with unbalanced quotes Now quotes are replaced with single quotes in the ASCII fallback while the UTF-8 filename* uses proper percent-encoding (%22).
Previous approach replaced " with ' which corrupts the actual filename. RFC 2183 specifies that within a quoted-string, backslash is the escape character. Now properly escapes both \ and " with backslash prefix. The UTF-8 filename* parameter uses percent-encoding which correctly preserves the original filename for modern clients.
|
I believe this is a duplicate of #11377? |
Author
|
I see, closing it then. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the
Content-Dispositionheader in the/viewendpoint to comply with RFC 2183 and RFC 5987.Before:
Content-Disposition: filename="name.ext"After:
Content-Disposition: attachment; filename="name.ext"; filename*=UTF-8''name.extProblem
The current header format is missing the required disposition-type (
attachment;) per RFC 2183. This causes third-party download libraries (e.g., Go'smime.ParseMediaType) to fail to parse the filename, resulting in files being saved with incorrect names like "view" instead of the actual filename.Reported in #8914.
Changes
create_content_disposition_header()helper function that generates RFC-compliant headersview_imageendpointfilename=for legacy clients (non-ASCII chars replaced with?)filename*=UTF-8''parameter for proper international filename supportTesting
curl -I "http://localhost:8188/view?filename=test.png"Compatibility
This change is backward compatible:
filename*parameter for UTF-8 supportfilenameparameterattachment;disposition-type is universally supportedFixes #8914