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

AbstractJackson2HttpMessageConverter - Check for encoding breaks JSON to POJO de-serialization experience #25247

Closed
kruegsch opened this issue Jun 12, 2020 · 9 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: regression A bug that is also a regression
Milestone

Comments

@kruegsch
Copy link

Affects: 5.2.7 (Spring Boot 2.3.1)

Due to a fix in Issue #25076
the AbstractJackson2HttpMessageConverter also blocks the out-of-the-box JSON to POJO de-serialization for responses with an encoding/character set not being unicode.

In our case a GET request to https://appleid.apple.com/auth/keys responses with a
Content-Type: application/json;charset=ISO-8859-1, which results now in
org.springframework.web.client.UnknownContentTypeException: Could not extract response: no suitable HttpMessageConverter found for response type [class x.y.z.AppleKeyRequest] and content type [application/json;charset=ISO-8859-1].

It would be nice to have an option to include non compliant JSON encodings for the given default converters or to have the encoding check as opt-in to be on par with previous default behavior.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 12, 2020
@jhoeller jhoeller added in: web Issues in web modules (web, webmvc, webflux, websocket) type: regression A bug that is also a regression and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jun 15, 2020
@jhoeller jhoeller added this to the 5.2.8 milestone Jun 15, 2020
@jhoeller
Copy link
Contributor

@poutsma I suppose this also affects 5.1.16 since the same change got backported.

@poutsma
Copy link
Contributor

poutsma commented Jun 15, 2020

@poutsma I suppose this also affects 5.1.16 since the same change got backported.

It does, yes.

@poutsma poutsma added the for: backport-to-5.1.x Marks an issue as a candidate for backport to 5.1.x label Jun 15, 2020
@spring-projects-issues spring-projects-issues added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.1.x Marks an issue as a candidate for backport to 5.1.x labels Jun 15, 2020
poutsma added a commit to poutsma/spring-framework that referenced this issue Jun 15, 2020
This commit makes sure that Jackson-based message converters and
decoders can deal with non-unicode input. It does so by reading
non-unicode input messages with a InputStreamReader.

This commit also adds additional tests forthe canRead/canWrite methods
on both codecs and message converters.

Closes: spring-projectsgh-25247
@jeffjensen
Copy link

jeffjensen commented Jun 15, 2020

We were caught by this problem because we use
@RequestMapping(produces = MediaType.APPLICATION_JSON_VALUE)
because MediaType APPLICATION_JSON_UTF8 is deprecated.

Is there a better constant class to use?
Should we use the UTF8 one instead?
Is there something else to configure instead to comply with the stricter checking?

@poutsma
Copy link
Contributor

poutsma commented Jun 16, 2020

@jeffjensen This issue should only occur when dealing with non-unicode JSON. By default (i.e. without a charset specified in the Content-Type) JSON is UTF-8 encoded. It was only because of a bug in Chrome that the explicit UTF-8 parameter was required. That bug is now fixed, and that is why we deprecated the constant.

Can you elaborate more about your setup? Is your service producing non-unicode JSON?

poutsma added a commit that referenced this issue Jun 16, 2020
This commit makes sure that Jackson-based message converters and
decoders can deal with non-unicode input. It does so by reading
non-unicode input messages with a InputStreamReader.

This commit also adds additional tests forthe canRead/canWrite methods
on both codecs and message converters.

Closes: gh-25247
@jeffjensen
Copy link

Not intentionally. Our puzzler is most of the responses are the same with Boot 2.3.0 and 2.3.1, but there is a situation in error handling where:

  • With Boot 2.3.0, Content-Type:"application/json"
  • With Boot 2.3.1, Content-Type:"application/json;charset=ISO-8859-1"

It happens when going through this method:

    private void populateResponse(final HttpServletResponse response, final String realmName,
                                  final HttpStatus httpStatus, final String responseBody) throws IOException
    {
        //add same headers as in BasicAuthenticationEntryPoint.commence method for HTTP standards compliance.
        response.addHeader("WWW-Authenticate", "Basic realm=\"" + realmName + "\"");
        response.setStatus(httpStatus.value());
        response.setContentType(MediaType.APPLICATION_JSON_VALUE);
        response.getWriter().write(responseBody);
    }

When I add UTF-8 to this line:
response.setContentType(MediaType.APPLICATION_JSON_VALUE + ";charset=UTF-8");
the tests pass as charset=ISO-8859-1 with 2.3.1 does not happen.

@poutsma
Copy link
Contributor

poutsma commented Jun 16, 2020

@kruegsch This should now be fixed. Can you please try a recent 5.2.8 snapshot to see if the problem has been resolved?

@jeffjensen I am not sure you are facing the same issue. Feel free to try the snapshot, to see if it resolves the problem. If not then please create a separate issue, because it was something else causing this.

@kruegsch
Copy link
Author

@kruegsch This should now be fixed. Can you please try a recent 5.2.8 snapshot to see if the problem has been resolved?

@poutsma
Works just fine with the 5.2.8.BUILD-SNAPSHOT. Thanks!

@jeffjensen
Copy link

@poutsma
You are correct, we have a different issue and 5.2.8.BUILD-SNAPSHOT has no impact. Thank you for the assistance.

@poutsma
Copy link
Contributor

poutsma commented Jun 17, 2020

@jeffjensen The problem could be related, but I really can't tell without more information. So please create a new issue, with a sample if possible.

FelixFly pushed a commit to FelixFly/spring-framework that referenced this issue Aug 16, 2020
This commit makes sure that Jackson-based message converters and
decoders can deal with non-unicode input. It does so by reading
non-unicode input messages with a InputStreamReader.

This commit also adds additional tests forthe canRead/canWrite methods
on both codecs and message converters.

Closes: spring-projectsgh-25247
zx20110729 pushed a commit to zx20110729/spring-framework that referenced this issue Feb 18, 2022
This commit makes sure that Jackson-based message converters and
decoders can deal with non-unicode input. It does so by reading
non-unicode input messages with a InputStreamReader.

This commit also adds additional tests forthe canRead/canWrite methods
on both codecs and message converters.

Closes: spring-projectsgh-25247
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

5 participants