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

Building FeignException can throw llegalCharsetNameException #2540

Closed
kubav182 opened this issue Sep 6, 2024 · 1 comment · Fixed by #2545
Closed

Building FeignException can throw llegalCharsetNameException #2540

kubav182 opened this issue Sep 6, 2024 · 1 comment · Fixed by #2545
Labels
good first issue Issues that are good for first time contributors to tackle help wanted Issues we need help with tackling

Comments

@kubav182
Copy link
Contributor

kubav182 commented Sep 6, 2024

WHERE:
In this class https://github.com/OpenFeign/feign/blob/master/core/src/main/java/feign/FeignException.java is piece of code:

if (!Charset.isSupported(group)) {
    return null;
}

WHATS WRONG:
Some external apis can send header Content-Type: text/xml;charset="utf-8" in response. Problem is with quotes.
Method Charset.isSupported throws exception when quotes are part of input string as it checks valid characters.
So building FeignException throws exception in this case.

EXPECTED BEHAVIOUR:
IF statement does not throw exception. It returns true or null in this case. We can't change external api and preprocess response headers is impossible as they are unmodifiable.

SOLUTION:
A. If you want to return true just sanitize String input group and remove for example quotes. Or change regexp to extract it without quotes.
group.replaceAll("\"", "");
B. Catch llegalCharsetNameException and return null from IF statement.

@kdavisk6
Copy link
Member

Sounds like you have a good handle on the issue, I recommend submitting a pull request.

@kdavisk6 kdavisk6 added help wanted Issues we need help with tackling good first issue Issues that are good for first time contributors to tackle labels Sep 11, 2024
kubav182 pushed a commit to kubav182/feign that referenced this issue Sep 11, 2024
FeignException does not throw exception during extracting response charset now.

These content types are all valid:
text/html;charset=utf-8
text/html;charset=UTF-8
Text/HTML;Charset="utf-8"
text/html; charset="utf-8"
kubav182 pushed a commit to kubav182/feign that referenced this issue Sep 11, 2024
While building FeignException, ignore quotes in regexp, ignore case and catch IllegalCharsetNameException to be sure it does not raise in other cases.

These content types are all valid and results in utf-8:
text/html;charset=utf-8
text/html;charset=UTF-8
Text/HTML;Charset="utf-8"
text/html; charset="utf-8"

Fixes OpenFeign#2540
kubav182 pushed a commit to kubav182/feign that referenced this issue Sep 12, 2024
While building FeignException, ignore quotes in regexp, ignore case and catch IllegalCharsetNameException to be sure it does not raise in other cases.

These content types are all valid and results in utf-8:
text/html;charset=utf-8
text/html;charset=UTF-8
Text/HTML;Charset="utf-8"
text/html; charset="utf-8"

Fixes OpenFeign#2540
kubav182 pushed a commit to kubav182/feign that referenced this issue Sep 12, 2024
While building FeignException, ignore quotes in regexp, ignore case and catch IllegalCharsetNameException to be sure it does not raise in other cases.

These content types are all valid and results in utf-8:
text/html;charset=utf-8
text/html;charset=UTF-8
Text/HTML;Charset="utf-8"
text/html; charset="utf-8"

Fixes OpenFeign#2540
velo pushed a commit that referenced this issue Sep 16, 2024
While building FeignException, ignore quotes in regexp, ignore case and catch IllegalCharsetNameException to be sure it does not raise in other cases.

These content types are all valid and results in utf-8:
text/html;charset=utf-8
text/html;charset=UTF-8
Text/HTML;Charset="utf-8"
text/html; charset="utf-8"

Fixes #2540

Co-authored-by: Jakub Venglar <venglar@rixo.cz>
velo pushed a commit that referenced this issue Oct 7, 2024
While building FeignException, ignore quotes in regexp, ignore case and catch IllegalCharsetNameException to be sure it does not raise in other cases.

These content types are all valid and results in utf-8:
text/html;charset=utf-8
text/html;charset=UTF-8
Text/HTML;Charset="utf-8"
text/html; charset="utf-8"

Fixes #2540

Co-authored-by: Jakub Venglar <venglar@rixo.cz>
velo pushed a commit that referenced this issue Oct 8, 2024
While building FeignException, ignore quotes in regexp, ignore case and catch IllegalCharsetNameException to be sure it does not raise in other cases.

These content types are all valid and results in utf-8:
text/html;charset=utf-8
text/html;charset=UTF-8
Text/HTML;Charset="utf-8"
text/html; charset="utf-8"

Fixes #2540

Co-authored-by: Jakub Venglar <venglar@rixo.cz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issues that are good for first time contributors to tackle help wanted Issues we need help with tackling
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants