-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Content-Type parsing breaks on valid test cases #51842
Comments
In HTTP headers, including the Understanding the
|
Considering the simplicity or naivety of the existing parser, probably oversight.
Breaking changes are sometimes necessary. |
Thanks for the clarification! Once the confusion with parameter orders is resolved, I can try updating my branch with a fix. |
Steps to reproduce
The current implementation of the Content-Type parser breaks in some valid test cases:
application/pdf; name=test.pdf
worksapplication/pdf; name="test with spaces.pdf"
does not work, yields inContent-Type: application/pdf; name="test
, which is an invalid header value and breaks reverse proxies (for example Traefik 3.0)The current Regex solution fails with evaluating quoted values with spaces, and all in all is very prone to unexpected errors:
application/pdf; name=";charset=utf-8"
would parseapplication/pdf; name="
as the content type and readutf-8"
as the charset, which is wrong twice: First of all it was never set, and then it incorrectly includes the quote as part of the charset.Expected behavior
The Content-Type parser must not yield invalid header values and must parse Content-Type headers according to RFC 9110.
Additional Notes
While working on a fix, a few questions rose up:
rails/actionpack/test/controller/mime/respond_to_test.rb
Lines 350 to 355 in 72fccfb
text/html; fragment
is not a valid Content-Type header.To me, the current way Media- and MIME types, charset and the Content-Type header are parsed and serialized seems to be a mess that breaks in many valid cases (example) and leads to unexpected, hard-to-debug errors. While trying to fix this issue, the special handling of
charset
everywhere made it impossible to fix the issue for me without breaking changes.Suggestion
An idea I had while trying to fix the issue is introducing a
ContentType
type, that holds aMime
string and zero or more key/value pairs for parameter. Such a struct should be easily serializable and deserializable, and setting the charset on a response could then just be setting the according parameter on there without serializing and parsing the Content-Type header string multiple times.System configuration
Rails version: 7.0.8.1
Ruby version: 3.2.4
The text was updated successfully, but these errors were encountered: