-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
KTOR-7586: improve parsing of supported media types #4410
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution! 🚀
I have some ideas on how to improve it. Please, check the comments.
application/java-byte-code,class | ||
application/java-serialized-object,ser | ||
application/java-vm,class | ||
application/javascript,js |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Media type application/javascript
is obsolete, so it should have lesser priority than text/javascript
, but all application/
media types now have priority over text/
types as they're upper in the list. Our tests have detected this change and WebjarsTest
fails with the message: expected: <text/javascript> but was: <application/javascript>
.
A simple solution is to move all deprecated types to the bottom of the list after some separator
...
text/json
...
# Deprecated media types
application/json
While this solution works well for changing the priority of deprecated types, we might have other inconsistencies between old and new order (like with .rpm
extension). Could you write down all inconsistencies?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
after some separator
are you suggesting to insert a comment into rawMimes
and ignore it when processing the string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we could ignore lines starting with #
in addition to already ignored empty lines
good points, thanks, please review the fixes |
Subsystem
Core, http
Motivation
https://youtrack.jetbrains.com/issue/KTOR-7586
Solution
improved parsing of supported media types