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

Fixed extracting charset from response. #2545

Merged
merged 1 commit into from
Sep 16, 2024
Merged

Conversation

kubav182
Copy link
Contributor

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

@kdavisk6
Copy link
Member

Please correct the build by running ./mvnw install locally at least once to ensure formatting, test cases, and license requirements.

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
Copy link
Contributor Author

I ran install with toolchain skipped. License header was changed. I also added parametrized test to FeignExceptionTest to cover different cases.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Some suggestions could not be made:

  • annotation-error-decoder/pom.xml
    • lines 32-31
  • apt-test-generator/pom.xml
    • lines 33-32
    • lines 62-61
  • benchmark/pom.xml
    • lines 31-30
  • core/pom.xml
    • lines 32-31
  • dropwizard-metrics4/pom.xml
    • lines 30-29
  • dropwizard-metrics5/pom.xml
    • lines 30-29
  • example-github-with-coroutine/pom.xml
    • lines 32-31
  • example-github/pom.xml
    • lines 32-31
  • example-wikipedia-with-springboot/pom.xml
    • lines 33-32
  • example-wikipedia/pom.xml
    • lines 33-32
  • fastjson2/pom.xml
    • lines 32-31
  • googlehttpclient/pom.xml
    • lines 32-31
  • gson/pom.xml
    • lines 32-31
  • hc5/pom.xml
    • lines 32-31
  • httpclient/pom.xml
    • lines 32-31
  • hystrix/pom.xml
    • lines 32-31
  • jackson-jaxb/pom.xml
    • lines 32-31
    • lines 82-83
  • jackson-jr/pom.xml
    • lines 32-31
  • jackson/pom.xml
    • lines 32-31
  • jakarta/pom.xml
    • lines 33-32
  • java11/pom.xml
    • lines 30-29
  • jaxb-jakarta/pom.xml
    • lines 32-33
    • lines 54-56
  • jaxb/pom.xml
    • lines 32-31
    • lines 47-51
  • jaxrs/pom.xml
    • lines 32-31
  • jaxrs2/pom.xml
    • lines 32-31
  • jaxrs3/pom.xml
    • lines 34-33
  • jaxrs4/pom.xml
    • lines 34-33
  • json/pom.xml
    • lines 32-31
  • kotlin/pom.xml
    • lines 34-33
  • micrometer/pom.xml
    • lines 30-29
  • mock/pom.xml
    • lines 32-31
  • moshi/pom.xml
    • lines 32-31
  • okhttp/pom.xml
    • lines 32-31
  • pom.xml
    • lines 537-538
  • reactive/pom.xml
    • lines 31-30
  • ribbon/pom.xml
    • lines 32-31
  • sax/pom.xml
    • lines 32-31
  • slf4j/pom.xml
    • lines 32-31
  • soap-jakarta/pom.xml
    • lines 32-33
    • lines 63-62
    • lines 79-81
  • soap/pom.xml
    • lines 32-31
    • lines 55-60
    • lines 70-71
  • spring/pom.xml
    • lines 34-33
  • spring4/pom.xml
    • lines 32-31

@kdavisk6 kdavisk6 added the ready to merge Will be merged if no other member ask for changes label Sep 14, 2024
@kdavisk6
Copy link
Member

These changes are approved. We'll leave this PR open for a few more days to allow for additional feedback. If there is none, we'll merge this.

Thanks again for your help!

@kdavisk6 kdavisk6 added the waiting for feedback Issues waiting for a response from either to the author or other maintainers label Sep 14, 2024
@velo velo merged commit 27f6aa6 into OpenFeign:master Sep 16, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Will be merged if no other member ask for changes waiting for feedback Issues waiting for a response from either to the author or other maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Building FeignException can throw llegalCharsetNameException
3 participants