OpenFeign / feign

Feign makes writing java http clients easier
Apache License 2.0
9.43k stars 1.92k forks source link

Incorrect content-type handling: application/json; charset=utf-8 #815

Closed lukbin closed 5 years ago

lukbin commented 5 years ago

I've noticed that while sending POST requests using FeignClient, response from remote service is not mapped to response object. If remote service returns content-type -> application/json; charset=utf-8 then responsePayload is ignored and not mapped to pojo. Content type is recognized as text/html.

application/json; charset=utf-8 - doesn't work application/json;charset=utf-8 - works fine

application/json; charset=utf-8 is generarated by express framework (Node.js) and it seems to be also in correct format.

kdavisk6 commented 5 years ago

Can you provide your Feign configuration and Interface definition? Feign delegates the response handling to the underlying Client and Decoder.

navaare commented 5 years ago

@lukbin

What makes you thinking that application/json; charset=utf-8 or application/json;charset=utf-8 is a correct content-type?

Event at https://docs.oracle.com/javaee/7/api/javax/ws/rs/core/MediaType.html there is no such one.

Perhaps, Spring and its RestTemplate support it BUT this is not obligatory as a such content-type is not a standard for JSON but only "application/json"

kdavisk6 commented 5 years ago

@lukbin

You cannot separate the charset from the media type, that is an invalid string. Spaces are not allowed in the Content-Type header if the client uses strict validation..

marcintokarski commented 5 years ago

@kdavisk6 according to https://tools.ietf.org/html/rfc2616#section-14.17 space before charset is allowed.

kdavisk6 commented 5 years ago

Thank you for the clarification. @lukbin it appears then that the server you are sending your request to is non compliant. As such, you may need to set the Content Type header directly using the Header annotation.

lukbin commented 5 years ago

according to @marcintokarski wrote, there is an example in rfc2616 spec:

Media types are defined in section 3.7. An example of the field is Content-Type: text/html; charset=ISO-8859-4

It looks like space is allowed, I'm not sure why such requrests are not compiliant ?

kdavisk6 commented 5 years ago

From the documentation presented here, it should be compliant, but it looks like the server you are sending the request to is not. What server software are you using? Or is this a service you do not control?

lukbin commented 5 years ago

Response header is generarated by express framework (Node.js). It returns header application/json; charset=utf-8 (with space). FeignClient cannot handle such response. I prepared server mock that returned the same response with the difference I described before (space removed) and FeignClient could handle that response.

kdavisk6 commented 5 years ago

Can you provide your Feign configuration?

lukbin commented 5 years ago

it's default configuration. I've definded mebClient.ribbon.listOfServers property to provide server address. @FeignClient(name = "mebClient") public interface MebClient { @GetMapping(value = "api/customers/{customerId}/", produces = MediaType.APPLICATION_JSON_VALUE) Customer getCustomer(@PathVariable("customerId") String customerId); }

kdavisk6 commented 5 years ago

Ok, a few items to consider here.

First, you are using Spring Cloud OpenFeign, an extension to OpenFeign. While most of the core concepts translate, they are not inter-changeable. In this particular instance, request encoding and decoding has been overridden by Spring's SpringEncoder and SpringDecoder respectively. I would first double check their documentation here

Second, @GetMapping is not supported in Spring Cloud OpenFeign, per it's documentation. Have you tried using @RequestMapping instead?

If all else fails, try creating the Feign client manually. This will remove Spring from the client and help determine if the issue is in OpenFeign or in your Spring configuration.

kdavisk6 commented 5 years ago

@lukbin

Where you able to resolve this?

kdavisk6 commented 5 years ago

I'm going to close this issue and recommend that questions like this be made on Stack Overflow, allowing more users to help you.

dalvan-bevilaqua commented 3 years ago

if using loombok add in your class @NoArgsConstructor @AllArgsConstructor