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

Add support for additional mime-types. #750

Closed
wants to merge 1 commit into from

Conversation

nojaf
Copy link
Collaborator

@nojaf nojaf commented Jun 2, 2022

Fixes #749.

@baronfel
Copy link
Collaborator

baronfel commented Jun 2, 2022

This is a good escape hatch, but should the default set of MIME times be expanded? .mp4 is pretty common these days.

For reference, here is the set of file-extension-based MIME types that AspNetCore knows out of the box today. I think it would be nice to have common extensions like these Just Work ™️

@nojaf
Copy link
Collaborator Author

nojaf commented Jun 2, 2022

Perhaps, but that begs the question should it be done here or in Suave itself?

@baronfel
Copy link
Collaborator

baronfel commented Jun 2, 2022

I think it could be a two-pronged approach - fix here as an intermediate, and provide the same fix upstream to Suave to let them decide if they want to take it.

@nojaf
Copy link
Collaborator Author

nojaf commented Jun 2, 2022

Sounds reasonable, I would need the escape hatch for the more obscure stuff in my project.

@nojaf
Copy link
Collaborator Author

nojaf commented Jun 7, 2022

@dsyme
, @eiriktsarpalis how do you want to see this fixed?

@dsyme
Copy link
Contributor

dsyme commented Jun 7, 2022

how do you want to see this fixed?

I think expanding the list here to include all the entries from ASP.NET Core makes sense, and then also contributing that to Suave.

@dsyme
Copy link
Contributor

dsyme commented Jun 7, 2022

@nojaf The rest of this PR is fine. The one thing is that I don't really know if there are standard command line flags on web servers for this sort of thing - if there is we should follow that standard for the name of the flag - if not what you have is fine.

@nojaf
Copy link
Collaborator Author

nojaf commented Jun 8, 2022

Closing in favour of #752.

@nojaf nojaf closed this Jun 8, 2022
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.

Video content is not being served.
3 participants