Repository navigation
AddJsonBody() ignores contentType argument #1645
Description
Expected Behavior
IRestRequest.AddJsonBody(obj, contentType) will send correct ContentType field value
Actual Behavior
IRestRequest.AddJsonBody(obj, contentType) force sends application/json ContentType field
Steps to Reproduce the Problem
- Enable Wireshark
- Send some object with custom contentType (for example,
hello/world) - See sent packet with default HTTP header:
ContentType: application/json(should beContentType: hello/world)
Specifications
- Version: 106.13.0
- Platform: Windows 10 x64 Pro
- Subsystem: Windows
Activity
AJIOB commentedon Nov 2, 2021
Looks like the library uses
ISerializer.ContentTypefield instead ofIRestSerializer.SupportedContentTypes:https://github.com/restsharp/RestSharp/blob/dev/src/RestSharp/RestRequestExtensions.cs#L41
alexeyzimarev commentedon Nov 7, 2021
It's quite inconsistent that we allow specifying the content type for
AddJsonBody. Normally, you'd need a custom serialiser if you want to change its content type.AJIOB commentedon Nov 7, 2021
@alexeyzimarev Why is it inconsistent to specify custom contentType, that is supported by default serializer?
I really need to use
application/my_custom_type+json, sorry for inconsistent example.But nowadays I need to use an old version, where that works correctly (105.x).
alexeyzimarev commentedon Nov 7, 2021
What I mean is that taking the content type from the serialiser is correct, as the serialiser knows how to form the request correctly, and its content type. But I agree that life could be easier if all your server wants is a normal JSON but for some reason, it needs a custom content type (which is also JSON).
The previous major version works correctly as it doesn't really support custom serialisers other than this weird way to add content handlers.
In your case the execution path is different. If won't get into
requestSerializer != nullcondition, so it will try finding the serialiser based on the data type. It will find the default JSON serialiser.Here is where you expect the custom content type to be assigned (last line):
The first argument should be
body.ContentType ?? serializer.ContentTypefor your case to work.Please feel free to submit a PR. A test would be nice as well.
alexeyzimarev commentedon Dec 6, 2021
The
SupportedContentTypeproperty is used for deserialization only as we need to match deserializers with the response content type. So it's a list. TheContentTypeis what the serializer provides as the content type of the request.alexeyzimarev commentedon Dec 6, 2021
@AJIOB here's the scenario:
The reason for it to work in v105 is that it was serializing immediately, but it stopped working when serializers were moved to the client. As you form the request without it to know about the client instance, the request has no access to client serializers. It's a trade-off to support more serializers easier by moving them to the client, and it solved loads of issues.
Your particular issue is very easy to fix if you create your own serializer where the
ContentTypeproperty has a value you need to be passed as the request content type. You literally can take any existing serializer code and replace theContentTypeproperty value, then add it to the client.In terms of allowing to pass the content type when adding the JSON body to the request, it allows you to have more than one JSON serializer on the client, and choose one according to the content type provided. For example, you can have one default serializer and one custom serializer with your own content type.
I am not exactly sure how to change the behaviour without breaking too much. It is possible to fix if the request would be created from the client, like
client.CreateRequest, so the request will be aware of the client itself and its serializers. But it would be a major breaking change.AJIOB commentedon Dec 6, 2021
Maybe we can use it as a possible variant for serializer too, can't we?
AJIOB commentedon Dec 6, 2021
@alexeyzimarev Yes, you are right. Thank you.
IMHO, it looks like a crunch, not a stable solution.
alexeyzimarev commentedon Dec 8, 2021
If you compare it with
HttpClientJSON extensions forSystem.Text.Json, you can see that they allow sending the content type. It's because their API is very explicit:PostJsonAsync, and you provide the payload right there. I am also not a big fan of how historically requests are made in RestSharp when you add the request body as a parameter. That way, the API tells you "add more than one body" and then it fails. I tried to make parameters more explicit, so I marked theParameterconstructor obsolete, but I got tons of complaints. I think RestSharp suffers from its legacy API design, and, unless it's fixed with lots of breaking changes (and more complaints), it will always be a compromise.As I was writing this, I got an idea, although it's another "hack". It's possible to select the serializer based on
DataType, in addition to content type. So, if you only have one JSON serializer configured, it should be possible to find it and use your custom content type. However, it will be pretty much unclear which one to choose if you have more than one JSON serializer configured. I don't remember exactly if we allow more than one serializer for the same data type, I will check. Maybe it's still fixable/alexeyzimarev commentedon Dec 8, 2021
I think this one should fix the issue
https://github.com/restsharp/RestSharp/pull/1659/files#diff-4f3307c975077908dedf201eecd8049bdc7f884b15ce060d5cf090b51619051bR53
alexeyzimarev commentedon Dec 8, 2021
@AJIOB try 106.15, it should work. Feel free to re-open the issue if it doesn't.