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

UseNewtonsoft + clientless pattern, default serializer incorrectly used to serialize request body #783

Closed
statler opened this issue Dec 5, 2023 · 18 comments
Labels

Comments

@statler
Copy link

statler commented Dec 5, 2023

Using a complex JSON object - something like the following, the object is incorrectly serialized by PostJsonAsync whether using STJ or Json.Net. However, when serialized outside PostJsonAsync using default settings for Json.Net, the result is as expected.

Using latest 4pre6.

  var _linkItems = new
  {
      Int_Key = 0,
      Int_Box = 1111,
      Int_Link = 2222
  };
  dynamic _linkWrapper = new JObject();
  _linkWrapper["0"] = JObject.FromObject(_linkItems);
  var _params = new
  {
      pageToke = "",
      lotIntKey = 11,
      module = 2,
      lotData = new
      {
          Details = (string)null,
          Custom = (string)null,
          LinkItems = _linkWrapper,
          Security = (string)null,
          BIMLinks = (string)null
      }
  };

E.g this doesn't work:

            var path = lot_Segments.Append("MyMethod");
            var _req = _cookieSession.Request(path.ToArray());
            var _postReq = await _req
                .PostJsonAsync(_params);

However, this does:

            var json = JsonConvert.SerializeObject(_params);
            var content = new CapturedJsonContent(json);
            var _postReq = await _req
                .SendAsync(HttpMethod.Post, content);

The latter correctly sends the serialized string as
{"pageToke":"","lotIntKey":11,"module":2,"lotData":{"Details":null,"Custom":null,"LinkItems":{"0":{"Int_Key":0,"Int_Box":1111,"Int_Link":2222}},"Security":null,"BIMLinks":null}}

The first one mangles the content of linkitems into some strange set of [[[],[],[]][[]]] etc.

@statler statler added the bug label Dec 5, 2023
@tmenier
Copy link
Owner

tmenier commented Dec 7, 2023

I see you're using 4.0. Newtonsoft was removed so JObject is no longer supported. Neither are dynamics. Try the just-released Flurl.Http.Newtonsoft companion lib. I'm in the middle of documentation updates now, but here's the quick & dirty of how to use:

  • Option 1, specific client: flurlClient.Settings.JsonSerializer = new NewtonsoftJsonSerializer();
  • Option 2, global: FlurlHttp.Clients.UseNewtonsoft();

If you're already well versed in #770 (most people probably aren't), UseNewtonsoft() is also available on IFlurlClientBuilder when configuring a specific client at startup.

@statler
Copy link
Author

statler commented Dec 7, 2023

Thanks Todd - this isn't just a case of STJ vs NewtonSoft. This behaviour is observed even if you are using Newtonsoft. There is something about the serialization in the PostJsonAsync that differs from doing it outside as per my workaround above. I am already using FlurlHttp.Clients.UseNewtonsoft()

TLDR; UseNewtonSoft does not fix the observed behaviour.

For reference - per the original report using 4pre6.

@tmenier
Copy link
Owner

tmenier commented Dec 7, 2023

I can't repro this.

The first one mangles the content of linkitems into some strange set of [[[],[],[]][[]]] etc.

Yes, I get that by default.

The latter correctly sends the serialized string as
{"pageToke":"","lotIntKey":11,"module":2,"lotData":{"Details":null,"Custom":null,"LinkItems":{"0":{"Int_Key":0,"Int_Box":1111,"Int_Link":2222}},"Security":null,"BIMLinks":null}}

When I swap out the default serializer for the one defined in Flurl.Http.Newtonsoft, I get exactly that.

How exactly are you determining what Flurl's internals are doing when you reached your conclusions?

@statler
Copy link
Author

statler commented Dec 13, 2023

In the client, I specified the usenewtonsoft method - travelling so can't give the exact details - but if you can't repro it is probably something not working like i expected so I will do some more research my end and get back - feel free to close it for now and I will hit it again if there is an issue.

@tmenier
Copy link
Owner

tmenier commented Dec 14, 2023

I won't keep it open forever, but I'll keep it open a bit longer if you think you'll look at it again soon-ish. ;)

All the symptoms do seem to suggest STJ is still driving the serialization, even when you think Newtonsoft is in control. I suspect it's something with how you have things wired up.

@zhzhwcn
Copy link

zhzhwcn commented Dec 25, 2023

Same here:

#r "nuget: Flurl.Http.Newtonsoft, 0.9.0"
#r "nuget: Flurl.Http, 4.0.0"
using System.Net.Http;
using Flurl;
using Flurl.Http;
using Flurl.Http.Newtonsoft;
using Newtonsoft.Json.Linq;

var url = "http://bin.mailgun.net/5b58c2a";
FlurlHttp.Clients.UseNewtonsoft();
var json = new JObject();
json["test"] = "test";
json["number"] = 1;
json["date"] = DateTime.Today;
await url.PostJsonAsync(json);

Receives:

Parameter Value  
test []
number []
date []

@jassent
Copy link

jassent commented Jan 4, 2024

Same here:

#r "nuget: Flurl.Http.Newtonsoft, 0.9.0"
#r "nuget: Flurl.Http, 4.0.0"
using System.Net.Http;
using Flurl;
using Flurl.Http;
using Flurl.Http.Newtonsoft;
using Newtonsoft.Json.Linq;

var url = "http://bin.mailgun.net/5b58c2a";
FlurlHttp.Clients.UseNewtonsoft();
var json = new JObject();
json["test"] = "test";
json["number"] = 1;
json["date"] = DateTime.Today;
await url.PostJsonAsync(json);

Receives:

Parameter Value  
test []
number []
date []

What happens if you do this:

try {
    var result = await url.PostJsonAsync(json).ReceiveString();
    Console.WriteLine(result);
}
catch (Exception ex) {
  Console.WriteLine($"{ex}");
}

I have run into empty results due to threading and serializer exceptions. The above would help diagnose.

@zhzhwcn
Copy link

zhzhwcn commented Jan 5, 2024

Same here:

#r "nuget: Flurl.Http.Newtonsoft, 0.9.0"
#r "nuget: Flurl.Http, 4.0.0"
using System.Net.Http;
using Flurl;
using Flurl.Http;
using Flurl.Http.Newtonsoft;
using Newtonsoft.Json.Linq;

var url = "http://bin.mailgun.net/5b58c2a";
FlurlHttp.Clients.UseNewtonsoft();
var json = new JObject();
json["test"] = "test";
json["number"] = 1;
json["date"] = DateTime.Today;
await url.PostJsonAsync(json);

Receives:
Parameter Value  
test []
number []
date []

What happens if you do this:

try {
    var result = await url.PostJsonAsync(json).ReceiveString();
    Console.WriteLine(result);
}
catch (Exception ex) {
  Console.WriteLine($"{ex}");
}

I have run into empty results due to threading and serializer exceptions. The above would help diagnose.

{"message":"Post received. Thanks!"} test with RoslynPad 19.1 and .net 8.0.100, no exception throws.

@jassent
Copy link

jassent commented Jan 5, 2024

This is working for me:

var url = "https://eXXXXXXXXXXXXx.m.pipedream.net";
var a = new { test = "test", number = 1, date = DateTime.Today };
await url.PostJsonAsync(a);

The response received has the correct payload:
example1

This also works with the same payload as above but I don't really know that UseNewtonsoft() is actually doing anything:

var url = "https://eXXXXXXXXXXXXx.m.pipedream.net";
FlurlHttp.Clients.UseNewtonsoft();
var a = new { test = "test", number = 1, date = DateTime.Today };
await url.PostJsonAsync(a);

This does not work:

var url = "https://eXXXXXXXXXXXXx.m.pipedream.net";
FlurlHttp.Clients.UseNewtonsoft();
var json = new JObject();
json["test"] = "test";
json["number"] = 1;
json["date"] = DateTime.Today;
await url.PostJsonAsync(json);

The payload received is empty as others have described.

@tmenier
Copy link
Owner

tmenier commented Jan 7, 2024

Sorry for the delay, this is a priority and I'll look into it as soon as I have some time. In the mean time, you could try using the serializer directly to help determine if it's a problem with that vs the registration shortcuts. It's pretty straightforward:

https://github.com/tmenier/Flurl/blob/dev/src/Flurl.Http.Newtonsoft/NewtonsoftJsonSerializer.cs

Repository owner deleted a comment from astrohart Jan 8, 2024
Repository owner deleted a comment from jassent Jan 8, 2024
Repository owner deleted a comment from astrohart Jan 8, 2024
@tmenier
Copy link
Owner

tmenier commented Jan 8, 2024

Confirmed. Sorry all, better tests would have prevented this little embarrassment. In short, "global" settings as a stand-alone thing were eliminated in 4.0 but moved to a global cache of clients used just for the clientless pattern. But with that pattern, request serialization happens before a client is selected, so changing the default serializer isn't properly applying to request bodies. (It is applying to JSON responses.) I have it proven in a test and will try to get fix released in short order.

@astrohart
Copy link

Who is this "ghost" who keeps deleting my comments? Am I being a bad person or something?

@tmenier
Copy link
Owner

tmenier commented Jan 9, 2024

@astrohart This issue is bug report, not a discussion of the merits of Newtonsoft vs STJ. I've deleted several of your comments recently because they're off-topic noise. Please stop.

@astrohart
Copy link

First of all let me apologize if I've been disrespectful. That was never my intent although I admit I was blowing off some steam.

But there was some frustration there given I'm a contributor on a repo that uses Flurl and I had to do hours of work to use a workaround. But it was my intention to say "me too" on this bug, until I found the workaround.

No hard feelings?

@tmenier tmenier changed the title Json serialization fails for complex object even though it is successfully serialized outside Flurl UseNewtonsoft + clientless pattern, default serializer incorrectly used to serialize request body Jan 10, 2024
@tmenier
Copy link
Owner

tmenier commented Jan 10, 2024

Fixed & released. The core issue was in Flurl.Http; it would have been virtually impossible to fix in Flurl.Http.Newtonsoft alone. Both packages are updated (4.0.1 and 0.9.1 respectively), please update either of them and report back here if it does not resolve the issue. Sorry again for such a blatant bug!

@tmenier tmenier closed this as completed Jan 10, 2024
@amzhang
Copy link

amzhang commented Jul 23, 2024

Thank you @tmenier! You're wonderful!

@amzhang
Copy link

amzhang commented Jul 23, 2024

This might be related. I try to set global default setting in Program.cs to ignore null fields during JSON serialization:

FlurlHttp.Clients.WithDefaults(
    builder =>
        builder.WithSettings(
            settings =>
                settings.JsonSerializer = new DefaultJsonSerializer(
                    new JsonSerializerOptions
                    {
                        DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull
                    }
                )
        )
);

but it seem to have no effect. Null fields are still serialized when calling:

var response = await request.PostMultipartAsync(mp => {
    mp.AddJson("config", config);
});

Versions

<PackageReference Include="Flurl" Version="4.0.0" />
<PackageReference Include="Flurl.Http" Version="4.0.2" />

@tmenier
Copy link
Owner

tmenier commented Jul 23, 2024

@amzhang Could you open a new issue for this? Comments in closed issues don't stay on my radar, and I suspect this isn't directly related anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Released
Development

No branches or pull requests

6 participants