-
-
Notifications
You must be signed in to change notification settings - Fork 390
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
Custom converters required to persist CookieJar and FlurlCookie with System.Text.Json in .NET 5.0 #572
Comments
I think the issues are as follows:
public sealed class UrlConverter : JsonConverter<Url>
{
public override Url Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
=> Url.Parse(reader.GetString());
public override void Write(Utf8JsonWriter writer, Url value, JsonSerializerOptions options)
=> writer.WriteStringValue(value);
}
...
var options = new JsonSerializerOptions { Converters = { new UrlConverter() }, WriteIndented = true };
[
{
"OriginUrl": "https://boardgamegeek.com/login",
"DateReceived": "2020-11-13T23:22:11.4217197+00:00",
"Name": "bggusername",
"Value": "username",
"Expires": "2020-12-13T23:22:11+00:00",
"MaxAge": 2592000,
"Domain": ".boardgamegeek.com",
"Path": "/",
"Secure": false,
"HttpOnly": false,
"SameSite": null
},
{
"OriginUrl": "https://boardgamegeek.com/login",
"DateReceived": "2020-11-13T23:22:11.4246746+00:00",
"Name": "bggpassword",
"Value": "password",
"Expires": "2020-12-13T23:22:11+00:00",
"MaxAge": 2592000,
"Domain": ".boardgamegeek.com",
"Path": "/",
"Secure": false,
"HttpOnly": false,
"SameSite": null
}
] Note: the cookies seem to be persisted in reverse order to how they were added in the code and I reordered the json. |
The following hypothetical patch to remove From 100ae8f24b68b7e6c9716eb811d8768da96d7f56 Mon Sep 17 00:00:00 2001
From: Sean Fausett <[email protected]>
Date: Mon, 16 Nov 2020 14:40:12 +1300
Subject: [PATCH] Remove FlurlCookie ctor
---
CookieCutter.cs | 2 +-
CookieJar.cs | 16 +++++++++++++---
FlurlCookie.cs | 21 +++------------------
Program.cs | 12 ++++++++++--
4 files changed, 27 insertions(+), 24 deletions(-)
diff --git a/CookieCutter.cs b/CookieCutter.cs
index 0d47195..b2f3586 100644
--- a/CookieCutter.cs
+++ b/CookieCutter.cs
@@ -37,7 +37,7 @@ namespace Flurl.Http
foreach (var pair in GetPairs(headerValue))
{
if (cookie == null)
- cookie = new FlurlCookie(pair.Name, Url.Decode(pair.Value.Trim('"'), false), url, DateTimeOffset.UtcNow);
+ cookie = new FlurlCookie { Name = pair.Name, Value = Url.Decode(pair.Value.Trim('"'), false), OriginUrl = url };
// ordinal string compare is both safest and fastest
// https://docs.microsoft.com/en-us/dotnet/standard/base-types/best-practices-strings#recommendations-for-string-usage
diff --git a/CookieJar.cs b/CookieJar.cs
index d51e563..e881431 100644
--- a/CookieJar.cs
+++ b/CookieJar.cs
@@ -22,9 +22,19 @@ namespace Flurl.Http
/// <param name="name">Name of the cookie.</param>
/// <param name="value">Value of the cookie.</param>
/// <param name="originUrl">URL of request that sent the original Set-Cookie header.</param>
- /// <param name="dateReceived">Date/time that original Set-Cookie header was received. Defaults to current date/time. Important for Max-Age to be enforced correctly.</param>
- public CookieJar AddOrReplace(string name, object value, string originUrl, DateTimeOffset? dateReceived = null) =>
- AddOrReplace(new FlurlCookie(name, value.ToInvariantString(), originUrl, dateReceived));
+ public CookieJar AddOrReplace(string name, object value, string originUrl) =>
+ AddOrReplace(new FlurlCookie { Name = name, Value = value.ToInvariantString(), OriginUrl = originUrl });
+
+ /// <summary>
+ /// Adds a cookie to the jar or replaces one with the same Name/Domain/Path.
+ /// Throws InvalidCookieException if cookie is invalid.
+ /// </summary>
+ /// <param name="name">Name of the cookie.</param>
+ /// <param name="value">Value of the cookie.</param>
+ /// <param name="originUrl">URL of request that sent the original Set-Cookie header.</param>
+ /// <param name="dateReceived">Date/time that original Set-Cookie header was received. Important for Max-Age to be enforced correctly.</param>
+ public CookieJar AddOrReplace(string name, object value, string originUrl, DateTimeOffset dateReceived) =>
+ AddOrReplace(new FlurlCookie { Name = name, Value = value.ToInvariantString(), OriginUrl = originUrl, DateReceived = dateReceived });
/// <summary>
/// Adds a cookie to the jar or replaces one with the same Name/Domain/Path.
diff --git a/FlurlCookie.cs b/FlurlCookie.cs
index 55a32c5..8eb6b1b 100644
--- a/FlurlCookie.cs
+++ b/FlurlCookie.cs
@@ -39,37 +39,22 @@ namespace Flurl.Http
private bool _locked;
- /// <summary>
- /// Creates a new FlurlCookie.
- /// </summary>
- /// <param name="name">Name of the cookie.</param>
- /// <param name="value">Value of the cookie.</param>
- /// <param name="originUrl">URL of request that sent the original Set-Cookie header.</param>
- /// <param name="dateReceived">Date/time that original Set-Cookie header was received. Defaults to current date/time. Important for Max-Age to be enforced correctly.</param>
- public FlurlCookie(string name, string value, string originUrl = null, DateTimeOffset? dateReceived = null)
- {
- Name = name;
- Value = value;
- OriginUrl = originUrl;
- DateReceived = dateReceived ?? DateTimeOffset.UtcNow;
- }
-
/// <summary>
/// The URL that originally sent the Set-Cookie response header. If adding to a CookieJar, this is required unless
/// both Domain AND Path are specified.
/// </summary>
- public Url OriginUrl { get; }
+ public Url OriginUrl { get; init; }
/// <summary>
/// Date and time the cookie was received. Defaults to date/time this FlurlCookie was created.
/// Important for Max-Age to be enforced correctly.
/// </summary>
- public DateTimeOffset DateReceived { get; }
+ public DateTimeOffset DateReceived { get; init ; } = DateTimeOffset.UtcNow;
/// <summary>
/// The cookie name.
/// </summary>
- public string Name { get; }
+ public string Name { get; init; }
/// <summary>
/// The cookie value.
diff --git a/Program.cs b/Program.cs
index f3ff8ca..c2ef31b 100644
--- a/Program.cs
+++ b/Program.cs
@@ -14,8 +14,12 @@ namespace FlurlRepro
private static async Task Main()
{
var cookies = new CookieJar();
- cookies.AddOrReplace(new FlurlCookie("bggusername", "username", "https://boardgamegeek.com/login", DateTimeOffset.Parse("2020-11-13T23:22:11.4217197+00:00"))
+ cookies.AddOrReplace(new FlurlCookie
{
+ Name = "bggusername",
+ Value = "username",
+ OriginUrl = "https://boardgamegeek.com/login",
+ DateReceived = DateTimeOffset.Parse("2020-11-13T23:22:11.4217197+00:00"),
Expires = DateTimeOffset.Parse("2020-12-13T23:22:11+00:00"),
MaxAge = 2592000,
Domain = ".boardgamegeek.com",
@@ -24,8 +28,12 @@ namespace FlurlRepro
HttpOnly = false,
SameSite = null
});
- cookies.AddOrReplace(new FlurlCookie("bggpassword", "password", "https://boardgamegeek.com/login", DateTimeOffset.Parse("2020-11-13T23:22:11.4246746+00:00"))
+ cookies.AddOrReplace(new FlurlCookie
{
+ Name = "bggpassword",
+ Value = "password",
+ OriginUrl = "https://boardgamegeek.com/login",
+ DateReceived = DateTimeOffset.Parse("2020-11-13T23:22:11.4246746+00:00"),
Expires = DateTimeOffset.Parse("2020-12-13T23:22:11+00:00"),
MaxAge = 2592000,
Domain = ".boardgamegeek.com",
--
2.29.2.windows.1 Notes:
using System.Text.Json.Serialization;
[JsonConverter(typeof(UrlConverter))]
public class Url
{
} @tmenier Not sure what to do with |
Doing a little spring cleaning and I see this slipped through the cracks long ago. Looks like you solved it for your case but I imagine it's still an "issue" in .NET 6? I doubt I'll make it a very high priority to be honest but at least I'll get it in the backlog so it's not completely off the radar. |
I haven't managed to test with .NET 6 yet but I'd expect the same issues. |
@gitfool, In hindsight, #758 probably could have been marked a duplicate of this issue (and I sort of forgot about this one to be honest), but in any event, I believe the solution I detailed there makes JSON-serialization problems with CookieJar and FlurlCookie moot. If the end goal is saving and reloading CookieJars, I think it's a cleaner and simpler solution. Would you agree? |
Closing because I'm pretty sure #758 addresses the need here. Feel free to re-open for further discussion if you disagree. |
@tmenier I played with the latest prerelease and it worked well re cookies: gitfool/BoardGameGeek.Dungeon@f2cf3c3 There was one strange side effect of using the latest prerelease though; the type returned by
So I had to change the contract type and convert it to make it work: gitfool/BoardGameGeek.Dungeon@dc01459 If I capture the response as text using {"playid":"76959327","numplays":7,"html":"Plays: <a href=\"\/plays\/thing\/1?userid=12345\">7<\/a>"} Did something else change in this area? 🤔 |
Okay, so what changed in this area was obviously #517, and I'm a bit rusty in this space, sorry. 😊 It turns out |
Excellent. Yeah I can see how the STJ switch may have flown under the radar if this is the first prerelease you've tried, I'll have a loud reminder of it for the full release. 😄 |
Following up from #506 (comment).
Specifically, serializing CookieJar (and FlurlCookie) works without requiring custom converters, although Url is serialized as a nested complex type instead of as a simple string, which is not desirable.
However, deserializing the above types fails due to limited functionality and specific requirements of
System.Text.Json
.FlurlRepro.zip
FlurlRepro output
:CookieJar.json
:The text was updated successfully, but these errors were encountered: