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

fix: Response.charset does not support RFC 7231 compliant Content-Type headers using quotation marks as application/json; charset="utf-8" #2444

Merged
merged 3 commits into from
Jun 12, 2024

Conversation

stefanvitz
Copy link
Contributor

Hi,

when migrating from rest template to feign i noticed that the feign Response object does not determine the charset in an RFC compliant way. As this charset() method is used e.g. by the JacksonDecoder class decoding an response with an RFC compliant Content-Type header like application/json; charset="utf-8" will fail.

See also:
https://datatracker.ietf.org/doc/html/rfc7231#section-3.1.1.1

For example, the following
examples are all equivalent, but the first is preferred for
consistency:

 text/html;charset=utf-8
 text/html;charset=UTF-8
 Text/HTML;Charset="utf-8"
 text/html; charset="utf-8"

@stefanvitz stefanvitz force-pushed the feature/fix-feign-response-charset branch 3 times, most recently from 24db9db to a56438c Compare June 6, 2024 10:15
…e headers using quotation marks as application/json; charset="utf-8"
@stefanvitz stefanvitz force-pushed the feature/fix-feign-response-charset branch from a56438c to 8faee8f Compare June 6, 2024 10:16
@@ -314,7 +320,7 @@ public Reader asReader() {
}

@Override
public Reader asReader(Charset charset) throws IOException {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we undo these throws IOException?

My concern is breaking existing feign users

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is just on the implementation which is private final.
see above:

private static final class InputStreamBody implements Response.Body
so no feign user would be able to override this. and the exception is not thrown

@velo velo merged commit eed6dbe into OpenFeign:master Jun 12, 2024
3 checks passed
@stefanvitz
Copy link
Contributor Author

thx for the fast processing :)

velo added a commit that referenced this pull request Oct 7, 2024
…e headers using quotation marks as application/json; charset="utf-8" (#2444)

Co-authored-by: Stefan Vitz (C804185) <[email protected]>
Co-authored-by: Marvin <[email protected]>
velo added a commit that referenced this pull request Oct 8, 2024
…e headers using quotation marks as application/json; charset="utf-8" (#2444)

Co-authored-by: Stefan Vitz (C804185) <[email protected]>
Co-authored-by: Marvin <[email protected]>
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.

2 participants