Skip to content
New issue

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

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

Already on GitHub? # to your account

fix(serve-static): use application/octet-stream if the mime type is not detected #3415

Merged
merged 1 commit into from
Sep 17, 2024

Conversation

usualoma
Copy link
Member

Currently, if the mimeType could not be detected, the Content-Type is not set, so it falls back to “text/plain”. There may be cases where “text/plain” is acceptable, but I don't think it is appropriate for binary data to be returned as "text/plain", so I think it should be "application/octet-stream”. Even on general web servers, the default is "application/octet-stream" for files that cannot be detected from the file extension.

The author should do the following, if applicable

  • Add tests
  • Run tests
  • bun run format:fix && bun run lint:fix to format the code

Copy link

codecov bot commented Sep 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.77%. Comparing base (0edb243) to head (ffe1407).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3415      +/-   ##
==========================================
- Coverage   95.78%   95.77%   -0.01%     
==========================================
  Files         155      155              
  Lines        9314     9310       -4     
  Branches     2771     2771              
==========================================
- Hits         8921     8917       -4     
  Misses        393      393              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@yusukebe yusukebe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@yusukebe
Copy link
Member

@usualoma

Thank you. You are right. It's should be application/octet-stream Merging.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants