DOCK-2589: Only add utf-8 charset to text and json mime types #6014
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #6014 +/- ##
==========================================
Coverage 74.44% 74.44%
- Complexity 5493 5495 +2
==========================================
Files 381 381
Lines 19785 19788 +3
Branches 2043 2044 +1
==========================================
+ Hits 14728 14731 +3
Misses 4076 4076
Partials 981 981
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -39,8 +39,12 @@ public class CharsetResponseFilter implements ContainerResponseFilter { | |||
| public void filter(ContainerRequestContext requestContext, ContainerResponseContext responseContext) { | |||
| MediaType contentType = responseContext.getMediaType(); | |||
| if (contentType != null) { | |||
| if (!contentType.toString().toLowerCase().contains("charset=utf-8")) { | |||
| responseContext.getHeaders().putSingle("Content-Type", contentType.toString() + ";charset=UTF-8"); | |||
| boolean isText = "text".equals(contentType.getType()); | |||
There was a problem hiding this comment.
To be paranoid, should we have equalsIgnoreCase here and on the next line?
There was a problem hiding this comment.
Done. I've never seen a non-standardly-capitalized mime type but iT COuLd HapPeN.
Next line compares "types" and should do it correctly.
|
Description
During review testing of #6013, Charles noticed that the parameter
charsetwas being set toutf-8for responses that contained Zip content (mime typeapplication/zip). For example:The Zip mime type https://www.iana.org/assignments/media-types/application/zip doesn't support any parameters, and it's binary data, so the response header is nonsensical.
This PR changes the webservice to only set the charset to
utf-8fortextand JSON responses. Thanks to Charles for pinpointing where in the codebase this was happening!Generally, per standards,
textmime types should support thecharsetparameter: https://datatracker.ietf.org/doc/html/rfc2046#section-4.1.2Opinions vary as to whether setting
charsetfor JSON is necessary, proper, and/or good:https://stackoverflow.com/questions/9254891/what-does-content-type-application-json-charset-utf-8-really-mean
However, it's what we've been doing for many years now, and it seems to work fine, so continuing to do so avoids breaking anything that happened to depend on that behavior.
Review Instructions
Retrieve a Zip per the endpoint in the ticket, and confirm that the
charsetis not defined in the mime type. Retrieve a JSON response (for example, theorganizationsendpoint), and confirm that thecharsetis set toutf-8.Issue
https://ucsc-cgl.atlassian.net/browse/DOCK-2589
#6010
Security and Privacy
If there are any concerns that require extra attention from the security team, highlight them here and check the box when complete.
e.g. Does this change...
Please make sure that you've checked the following before submitting your pull request. Thanks!
mvn clean install@RolesAllowedannotation