Skip to content

Fix Content-Disposition header missing attachment prefix#13093

Open
themachinehf wants to merge 1 commit intoComfy-Org:masterfrom
themachinehf:fix/content-disposition-header
Open

Fix Content-Disposition header missing attachment prefix#13093
themachinehf wants to merge 1 commit intoComfy-Org:masterfrom
themachinehf:fix/content-disposition-header

Conversation

@themachinehf
Copy link

Adds missing attachment directive to Content-Disposition headers in server.py to ensure browsers properly download files instead of attempting to display them inline. Fixes 4 instances in the file download endpoint.

Add missing 'attachment;' directive to Content-Disposition headers in
server.py to ensure browsers properly download files instead of
attempting to display them inline.

Fixes 4 instances in the file download endpoint.
@coderabbitai
Copy link

coderabbitai bot commented Mar 21, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f4c2bfca-e4cb-45f7-9c0a-9ef10469e27f

📥 Commits

Reviewing files that changed from the base of the PR and between a11f68d and 43d8f56.

📒 Files selected for processing (1)
  • server.py

📝 Walkthrough

Walkthrough

The change modifies the Content-Disposition response header in the view_image endpoint of server.py. The header format is updated from filename="..." to attachment; filename="..." across all image-serving code paths, including rendered preview images (webp/jpeg formats), PNG outputs for both RGB and alpha channels, and the fallback file response handler. This change explicitly designates image responses as downloadable attachments rather than inline content.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding the 'attachment' directive to Content-Disposition headers to fix how browsers handle file downloads.
Description check ✅ Passed The description is directly related to the changeset, explaining the purpose of adding the 'attachment;' directive to Content-Disposition headers across four instances in the file download endpoint.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can generate a title for your PR based on the changes.

Add @coderabbitai placeholder anywhere in the title of your PR and CodeRabbit will replace it with a title based on the changes in the PR. You can change the placeholder by changing the reviews.auto_title_placeholder setting.

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.

1 participant