RicoSuter / NSwag

The Swagger/OpenAPI toolchain for .NET, ASP.NET Core and TypeScript.
http://NSwag.org
MIT License
6.69k stars 1.24k forks source link

C# generated client using FormUrlEncodedContent instead of StringContent #3414

Open leo-mck opened 3 years ago

leo-mck commented 3 years ago

I am working with an API that generate a specification like this:

{
    "swagger": "2.0",
    "info": {
        "version": "v1",
        "title": "SwaggerTest"
    },
    "host": "localhost:44386",
    "schemes": [
        "https"
    ],
    "paths": {
        "/api/Values": {
            "post": {
                "tags": [
                    "Values"
                ],
                "operationId": "Values_Post",
                "consumes": [
                    "application/json",
                    "text/json",
                    "application/xml",
                    "text/xml",
                    "application/x-www-form-urlencoded"
                ],
                "produces": [],
                "parameters": [
                    {
                        "name": "value",
                        "in": "body",
                        "required": true,
                        "schema": {
                            "type": "string"
                        }
                    }
                ],
                "responses": {
                    "204": {
                        "description": "No Content"
                    }
                }
            }
        }
    },
    "definitions": {}
}

This used to generate code like this:

var content_ = new System.Net.Http.StringContent(Newtonsoft.Json.JsonConvert.SerializeObject(value, _settings.Value));
content_.Headers.ContentType = System.Net.Http.Headers.MediaTypeHeaderValue.Parse("application/json");

Today I updated NSwagStudio and now it generates code like this:

var json_ = Newtonsoft.Json.JsonConvert.SerializeObject(value, _settings.Value);
var dictionary_ = Newtonsoft.Json.JsonConvert.DeserializeObject<System.Collections.Generic.Dictionary<string, string>>(json_, _settings.Value);
var content_ = new System.Net.Http.FormUrlEncodedContent(dictionary_);
content_.Headers.ContentType = System.Net.Http.Headers.MediaTypeHeaderValue.Parse("application/json");

This looks like an error from the API since it should only accept application/json but I do not have access to the API source code, if I manually delete the application/x-www-form-urlencoded from consumes, it works like before but I would like to understand if this change is a bug or if there is something I can configure on client-side to be able to generate the code as it was.

AGlezB commented 3 years ago

Same here. This happened from 13.10.8 to 13.10.9 with this change in src/NSwag.CodeGeneration.CSharp/Templates/Client.Class.liquid:

https://github.com/RicoSuter/NSwag/commit/df18a5d7a2d4a8000e8d172640aa183f95f481d8

When both json and x-www-form-urlencoded are present in the spec the template should go for json, which is more flexible, or allow some way configure it as a preference.

joearmonic commented 3 years ago

is there any way of configuring this to use the json serialized object instead of converting to dictionary and using the form url content for the content? Although I prefer the behaviour until 13.10.8 I would like to have a fix to avoid inconveniences and help people in our teams to going forward with the latest versions if they needed.

vincenzolauretta commented 3 years ago

Hi, hopefully this helps someone who has this issue on Asp.net (.NET Framework)

I added the following custom filter class:

    internal class AssignContentTypeFilter : IOperationFilter
    {
        public void Apply(Operation operation, SchemaRegistry schemaRegistry, ApiDescription apiDescription)
        {
            operation.consumes.Clear();
            operation.consumes.Add("application/json");

            operation.produces.Clear();
            operation.produces.Add("application/json");
        }
    }

then in SwaggerConfig.cs:

c.OperationFilter<AssignContentTypeFilter>();

reference link: https://stackoverflow.com/questions/58973329/asp-net-core-3-0-swashbuckle-restrict-responses-content-types-globally

RicoSuter commented 3 years ago

I added the following custom filter class:

Just as a note: This seems to be using and apply only to Swashbuckle as spec generator (not NSwag).

yfital commented 3 years ago

Hey,

We're having an issue with this as well, a bit different though: Generated object:

    [System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "10.4.3.0 (Newtonsoft.Json v12.0.0.0)")]
    public partial class JobRequest 
    {
        [Newtonsoft.Json.JsonProperty("SessionJobIdentifier", Required = Newtonsoft.Json.Required.Default, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
        public SessionJobIdentifier? SessionJobIdentifier { get; set; }= default!;

    }

    [System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "10.4.3.0 (Newtonsoft.Json v12.0.0.0)")]
    public partial class SessionJobIdentifier 
    {
        [Newtonsoft.Json.JsonProperty("SessionId", Required = Newtonsoft.Json.Required.Default, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
        public int? SessionId { get; set; }= default!;

        [Newtonsoft.Json.JsonProperty("JobId", Required = Newtonsoft.Json.Required.Default, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
        public string? JobId { get; set; }= default!;

    }

Deserialization throws: Newtonsoft.Json.JsonReaderException: 'Unexpected character encountered while parsing value: {. Path 'SessionJobIdentifier', line 1, position 25.'

On this line:

                    var json_ = Newtonsoft.Json.JsonConvert.SerializeObject(value, _settings.Value);
                    var dictionary_ = Newtonsoft.Json.JsonConvert.DeserializeObject<System.Collections.Generic.Dictionary<string, string>>(json_, _settings.Value);

Notice, Serialization into regular object works fine, it's something with the deserialization into dictionary E.G. Newtonsoft.Json.JsonConvert.DeserializeObject<JobRequest>(json_, _settings.Value)

Works fine

We also don't understand why the generated code moved to "dictionary"

igorvorona commented 3 years ago

Having the same issue with FormUrlEncodedContent generated instead of StringContent. It fails in controller's endpoint unable to populate model class [FromBody]:

public async Task MyEndpoint([FromBody] MyModel model)

model is always null now. Why was the change, it was working fine in v13.10.8.0

thierry-phenix commented 3 years ago

Same error here will it be fixed ? I need to manualy correct my autogenerated code :/

AGlezB commented 3 years ago

Same error here will it be fixed ? I need to manualy correct my autogenerated code :/

I'm still using 13.10.8 to avoid doing that.

mrlux commented 3 years ago

I think this is the change that introduced the issue df18a5d

gmklaus commented 2 years ago

Looking at that PR, I think ConsumesFormUrlEncoded isn't just doing what they think it's doing. It works for issue #3230 because the specification cited in that issue only has one body content type, application/x-www-form-urlencoded. But if there are multiple values in the consumes property ConsumesFormUrlEncoded still returns true. That may be appropriate in the eight other places ConsumesFormUrlEncoded is used, but it isn't here because it ends up adding code that assumes only simple value types in the object to be serialized despite the fact that the ContentType ends up being application/json and the object to be serialized can have nested object types.

I think if we added a ConsumesOnlyFormUrlEncoded method in OperationModelBase and used that instead it would preserve the fix for issue #3230 and also fix this issue. Here's my best guess what that could look like.

/// <summary>Gets a value indicating whether the operation only consumes 'application/x-www-form-urlencoded'.</summary>
public bool ConsumesOnlyFormUrlEncoded =>
    (_operation.ActualConsumes?.Any(c => c == "application/x-www-form-urlencoded") == true && _operation.ActualConsumes?.Count() == 1) ||
    _operation.RequestBody?.Content.Any(mt => mt.Key == "application/x-www-form-urlencoded") == true;

I think the RequestBody line would return true in the case of issue #3230 and the count would prevent it from returning true when the consumes property has multiple values and it should prefer application/json instead.

AronHetLam commented 2 years ago

I Ran into this issue as well in v13.15.10.0

lbras commented 2 years ago

I Ran into this issue as well in v13.15.10.0

Same here, consuming an API using Swashbuckle as spec generator. @vincenzolauretta fix worked for us. Thanks!

But yeah, this doesn't seem right. I would expect the behavior described by @AGlezB:

When both json and x-www-form-urlencoded are present in the spec the template should go for json, which is more flexible, or allow some way configure it as a preference.

At the very least, NSwag should set the right Content-Type. It tells the API that it is sending application/json and actually sends application/x-www-form-urlencoded...

Logic-Bits commented 2 years ago

Same here too. It will send a dictionary instead of JSON. Reverted back to v13.10.8.0 as @igorvorona wrote.

AFract commented 2 years ago

Just got the same problem with a definition file in version "swagger": "2.0" and NSwag studio v13.15.10. The generated client attempts to deserialize the generated json as a System.Collections.Generic.Dictionary<string, string> to make a FormUrlEncodedContent of it. Of course it crashes

var dictionary_ = Newtonsoft.Json.JsonConvert.DeserializeObject<System.Collections.Generic.Dictionary<string, string>>(json_, _settings.Value);
var content_ = new System.Net.Http.FormUrlEncodedContent(dictionary_);

Manual fix :

var json_ = Newtonsoft.Json.JsonConvert.SerializeObject(product, _settings.Value);
var content = new System.Net.Http.StringContent(json_);
content.Headers.ContentType = System.Net.Http.Headers.MediaTypeHeaderValue.Parse("application/json");
request_.Content = content;                    
request_.Method = new System.Net.Http.HttpMethod("POST");
request_.Headers.Accept.Add(System.Net.Http.Headers.MediaTypeWithQualityHeaderValue.Parse("application/json"));

Another solution is to update the service itself to have "consumes" and "produces" only containing JSON and not url-encoded forms or xml old school formats.

simeyla commented 2 years ago

At the very least, NSwag should set the right Content-Type. It tells the API that it is sending application/json and actually sends application/x-www-form-urlencoded...

This is an underrated comment! This definitely still needs to be fixed.

And I second the solution by @vincenzolauretta for just forcing the content-type for .NET framework projects. For me this only affects a couple methods anyway.

hmoratopcs commented 2 years ago

In Swagger 2.0, form and body parameters are mutually exclusive, so an operation can have either form data OR JSON body but not both.

Source: https://stackoverflow.com/a/43352204

So, first of all, Swashbuckle shouldn't include "application/x-www-form-urlencoded" in the consumes property. That said, nswag handles this error poorly, it should stick with the JSON content type, because the parameter is in: body, not in: formData.

PhilippRiegelmann commented 1 year ago

I'm still using 13.10.8 to avoid doing that.

I tried this way as well, but encountered the following problem: My Path is for example /ErrorSolution/{id} but instead of ErrorSolutionGetAsync as its in version 13.18, the methods are generated like ErrorsolutionGetAsync. Is there setting to change the way the methodnames are generated to PascalCase?

Juan4456 commented 1 year ago

Same issue on NSwag 13.18.2, I see this wasn't fixed, is there any workaround for NSwag other than downgrading to NSwag 13.10.8 or editing the generated code manually? Thanks

aratar79 commented 1 year ago

I'm having the same problem, any solution to this? so far I edit the code, but it's not a good solution, when another team member updates it will step on all my code.

hmoratopcs commented 1 year ago

A list of workarounds/solutions.

In the server side:

  1. Remove support for application/x-www-form-urlencoded (example for .NET Framework to leave only the JSON formatter, affects the accepted and the response content-type)
  2. Same of above, but instead of removing support, hide it from Swagger. (https://github.com/RicoSuter/NSwag/issues/3414#issuecomment-826479920)
  3. Instead of Swashbuckle, use something else that generates a Swagger conforming to spec.
  4. Configure Swashbuckle to serve OpenApi 3.0 (not supported if the server is .NET Framework).
  5. Contribute a patch to Swashbuckle.

In the client side:

  1. Downgrade NSwag to 13.10.8.
  2. Contribute a patch for this bug.
  3. Fix the swagger file each time.
  4. Fix the generated code each time.
aratar79 commented 1 year ago

Thanks, I'll stick with 2, it didn't work at first because I was doing something wrong, so I applied the client-side one, but no, it didn't convince me.

Although the best one I think would be 5.

Thank you very much.

DumDumin commented 1 year ago

Is anyone planning to fix this?

The "fix", which crafted this problem is just wrong. As long as the generator only support the first content type in the "consumes" and the "produce" property, it is wrong to check if there is any consume type is FormUrlEncoded "operation.ConsumesFormUrlEncoded" and then to just use "operation.Consumes", which will return the first content-type found...

{%         elseif operation.ConsumesFormUrlEncoded -%}
                var json_ = Newtonsoft.Json.JsonConvert.SerializeObject({{ operation.ContentParameter.VariableName }}, _settings.Value);
                var dictionary_ = Newtonsoft.Json.JsonConvert.DeserializeObject<System.Collections.Generic.Dictionary<string, string>>(json_, _settings.Value);
                var content_ = new System.Net.Http.FormUrlEncodedContent(dictionary_);
                content_.Headers.ContentType = System.Net.Http.Headers.MediaTypeHeaderValue.Parse("{{ operation.Consumes }}");
Strandedpirate commented 9 months ago

Don't fix it, let's see how many years we can keep this obvious MAJOR bug going, and how long it takes to finally kill this project.

amrastanov-nitka commented 3 months ago

any ideas on when the issue going to be fixed?

PromidataAchim commented 2 months ago

I found this when searching for my problem and see, it is still in the newest version not solved. Downgrade NSwag to 13.10.8 has worked, but is not the solution or?