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

Handling missing mime types gracefully #534

Closed
jake-arkinstall opened this issue Sep 10, 2022 · 1 comment
Closed

Handling missing mime types gracefully #534

jake-arkinstall opened this issue Sep 10, 2022 · 1 comment
Assignees
Labels
feature Code based project improvement good first issue an issue suitable to start contributing to the project

Comments

@jake-arkinstall
Copy link
Contributor

If a key isn't present in the mime_types map, an exception is thrown. This is because mime_types.at() is used at, e.g.,

set_header("Content-Type", mime_types.at(contentType));
.

I propose that the resolution is to search for the provided mime_type within the map using mime_types.find. If it is not found, a warning should be logged, but the content should still be delivered using the following logic:

  • Does the provided type look like a MIME type already? I.e. is it one of the IANA-registered types (application, audio, example, font, image, model, text, video), followed by a slash, followed by text? If so, send that verbatim.

  • Otherwise send text/plain.

Although this approach isn't quite as lightweight as simply sending text/plain, there are some benefits:

  • I'd be able, for example, to send content as text/csv without modifying the mime types map.
  • It would also allow people to use the full mime-type format, including parameters (e.g. text/plain;charset=UTF-8)
  • Developers coming from other technologies that use full mime types won't be faced with an internal server error for following an old habit.
@dranikpg
Copy link
Member

I fully agree, the current solution is way to limited. If you need to construct custom mime-types now, you can always set the Content-Type header yourself. But this feels like a workaround.

Jake, if you want, you can implement the fix youself and make your first contribution to Crow! 🔥

Else, I'll handle this in the following week.

@dranikpg dranikpg added feature Code based project improvement good first issue an issue suitable to start contributing to the project labels Sep 10, 2022
dranikpg pushed a commit that referenced this issue Sep 12, 2022
* Added tests for content-type to mime-type detection.

Added a custom_content_types test case that verifies that a user can
specify the mime-type through the contentType parameter upon creation
of a response. If their contentType does not appear in the mime_types
map, but looks like a valid mime type already, it should be used as the
mime type.

Validating against the full list of valid mime types
(https://www.iana.org/assignments/media-types/media-types.xhtml)
would be too intensive, so we merely verify that the parent type
(application, audio, font, text, image, etc) is a valid RFC6838
type, and that the subtype is at least one character. Thus we can
verify that custom/type fails, and incomplete strings such as
image/ and /json fail.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Code based project improvement good first issue an issue suitable to start contributing to the project
Projects
None yet
Development

No branches or pull requests

2 participants