[dotnet] [bidi] Adjust cookie expiry type according spec (unix seconds)

[nvborisenko]

User description

https://w3c.github.io/webdriver/#dfn-cookie-expiry-time

It is not milliseconds, it is seconds.

When the cookie expires, specified in seconds since Unix Epoch. Must not be set if omitted when adding a cookie.

💥 What does this PR do?

Add custom json converter especially for cookie expiry only.

🔄 Types of changes

• Bug fix (backwards compatible)

---

PR Type

Bug fix

---

Description

• Fix cookie expiry time format from milliseconds to seconds per WebDriver spec

• Add new DateTimeOffsetSecondsConverter for proper unix timestamp handling

• Remove unused TimeSpanConverter and update Cookie model

• Update test assertions to validate expiry time correctly

---

Diagram Walkthrough

flowchart LR
  A["Cookie Model"] --> B["DateTimeOffsetSecondsConverter"]
  B --> C["Unix Seconds Format"]
  D["TimeSpanConverter"] --> E["Removed"]
  F["Test Updates"] --> G["Expiry Validation"]

Loading

File Walkthrough

	Relevant files
Configuration changes	Broker.cs Remove unused TimeSpanConverter registration                          dotnet/src/webdriver/BiDi/Communication/Broker.cs Remove TimeSpanConverter from JSON converter list +0/-1
Enhancement	DateTimeOffsetConverter.cs Add seconds-based DateTimeOffset converter                              dotnet/src/webdriver/BiDi/Communication/Json/Converters/DateTimeOffsetConverter.cs Add DateTimeOffsetSecondsConverter class for unix seconds handling Update existing converter to handle double values +14/-1
Miscellaneous	TimeSpanConverter.cs Remove unused TimeSpanConverter class                                        dotnet/src/webdriver/BiDi/Communication/Json/Converters/TimeSpanConverter.cs Delete entire TimeSpanConverter class file +0/-44
Bug fix	Cookie.cs Update Cookie model expiry type                                                    dotnet/src/webdriver/BiDi/Network/Cookie.cs Change Expiry property from TimeSpan? to DateTimeOffset? Add JsonConverter attribute for DateTimeOffsetSecondsConverter +3/-1      SetCookieCommand.cs Apply converter to SetCookie command                                          dotnet/src/webdriver/BiDi/Storage/SetCookieCommand.cs Add JsonConverter attribute to PartialCookie.Expiry property Import required converter namespace +3/-0
Tests	StorageTest.cs Update cookie expiry test validation                                          dotnet/test/common/BiDi/Storage/StorageTest.cs Change expiry variable from DateTime to DateTimeOffset Update assertion to validate expiry within 1 second tolerance +2/-2

---

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Precision/Type Handling The converters cast reader.GetDouble() to long for both milliseconds and seconds. If the JSON number exceeds Int64 range or contains fractional seconds, this could truncate/overflow silently. Consider validating range, rejecting NaN/Infinity, and handling integral tokens via TryGetInt64 first to avoid precision loss. public override DateTimeOffset Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { return DateTimeOffset.FromUnixTimeMilliseconds((long)reader.GetDouble()); // still might get a double } public override void Write(Utf8JsonWriter writer, DateTimeOffset value, JsonSerializerOptions options) { writer.WriteNumberValue(value.ToUnixTimeMilliseconds()); } } internal class DateTimeOffsetSecondsConverter : JsonConverter<DateTimeOffset> { public override DateTimeOffset Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { return DateTimeOffset.FromUnixTimeSeconds((long)reader.GetDouble()); // still might get a double } public override void Write(Utf8JsonWriter writer, DateTimeOffset value, JsonSerializerOptions options) { writer.WriteNumberValue(value.ToUnixTimeSeconds()); } Converter Registration Only DateTimeOffsetConverter is registered globally; the new DateTimeOffsetSecondsConverter relies on property-level attributes. Confirm all cookie-related payloads (requests and responses) use the attribute-backed model; otherwise, seconds-based values may be misinterpreted elsewhere. Consider adding targeted options or context-aware converters if any path omits the attribute. new RequestConverter(), new ChannelConverter(), new HandleConverter(_bidi), new InternalIdConverter(_bidi), new PreloadScriptConverter(_bidi), new RealmConverter(_bidi), new RealmTypeConverter(), new DateTimeOffsetConverter(), new PrintPageRangeConverter(), new InputOriginConverter(), new WebExtensionConverter(_bidi), new SubscriptionConverter(),

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Converter misuse risk DateTimeOffsetConverter now reads with GetDouble but still writes milliseconds, while DateTimeOffsetSecondsConverter writes seconds; ensure the correct converter is consistently applied per field to avoid mixed units. Verify that all BiDi messages using DateTimeOffset still expecting milliseconds haven’t inadvertently switched or conflicted with the new seconds-based handling, preventing subtle timestamp bugs across the protocol. Examples: dotnet/src/webdriver/BiDi/Communication/Json/Converters/DateTimeOffsetConverter.cs [26-50] internal class DateTimeOffsetConverter : JsonConverter<DateTimeOffset> { public override DateTimeOffset Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { return DateTimeOffset.FromUnixTimeMilliseconds((long)reader.GetDouble()); // still might get a double } public override void Write(Utf8JsonWriter writer, DateTimeOffset value, JsonSerializerOptions options) { writer.WriteNumberValue(value.ToUnixTimeMilliseconds()); ... (clipped 15 lines) dotnet/src/webdriver/BiDi/Network/Cookie.cs [26] public sealed record Cookie(string Name, BytesValue Value, string Domain, string Path, long Size, bool HttpOnly, bool Secure, SameSite SameSite, [property: JsonConverter(typeof(DateTimeOffsetSecondsConverter))] DateTimeOffset? Expiry); Solution Walkthrough: Before: // Default converter handles milliseconds and is registered globally. // Broker.cs ... new DateTimeOffsetConverter(), // Uses milliseconds ... // Cookie expiry correctly uses a specific converter for seconds. // Cookie.cs public sealed record Cookie( ..., [property: JsonConverter(typeof(DateTimeOffsetSecondsConverter))] DateTimeOffset? Expiry ); // A hypothetical other message uses the default. Is this correct? public sealed record OtherMessage(DateTimeOffset? Timestamp); // Implicitly uses millisecond converter. After: // Default converter handles milliseconds and is registered globally. // Broker.cs ... new DateTimeOffsetConverter(), // Uses milliseconds ... // All timestamp fields are audited and have the correct converter applied explicitly. // Cookie.cs public sealed record Cookie( ..., [property: JsonConverter(typeof(DateTimeOffsetSecondsConverter))] DateTimeOffset? Expiry ); // Other message is verified against the spec and the correct converter is chosen. public sealed record OtherMessage([JsonConverter(typeof(DateTimeOffsetSecondsConverter))] DateTimeOffset? Timestamp); Suggestion importance[1-10]: 9 __ Why: This is a critical suggestion that highlights a potential architectural risk of inconsistent data serialization, as mixing timestamp units (seconds vs. milliseconds) can introduce subtle, hard-to-debug bugs.	High
Possible issue	Safely parse unix milliseconds Casting a potentially large double to long can overflow or silently truncate fractional milliseconds. Handle both integer and floating-point tokens explicitly and validate range before conversion to avoid incorrect dates and exceptions. dotnet/src/webdriver/BiDi/Communication/Json/Converters/DateTimeOffsetConverter.cs [28-31] public override DateTimeOffset Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { - return DateTimeOffset.FromUnixTimeMilliseconds((long)reader.GetDouble()); // still might get a double + // Handle both integer and floating-point number tokens safely + long millis; + if (reader.TokenType == JsonTokenType.Number && reader.TryGetInt64(out var intVal)) + { + millis = intVal; + } + else + { + var doubleVal = reader.GetDouble(); + if (doubleVal > long.MaxValue || doubleVal < long.MinValue) + { + throw new JsonException("Unix time milliseconds out of range."); + } + millis = Convert.ToInt64(Math.Truncate(doubleVal)); + } + return DateTimeOffset.FromUnixTimeMilliseconds(millis); } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that casting a double to a long can cause an overflow and improves robustness by adding range checks and explicitly handling integer and floating-point tokens.	Medium
	Robust unix seconds parsing Similar to milliseconds, direct cast from double risks precision loss and overflow. Support both integer and floating-point tokens with bounds checking and truncation to whole seconds before converting. dotnet/src/webdriver/BiDi/Communication/Json/Converters/DateTimeOffsetConverter.cs [41-44] public override DateTimeOffset Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { - return DateTimeOffset.FromUnixTimeSeconds((long)reader.GetDouble()); // still might get a double + long seconds; + if (reader.TokenType == JsonTokenType.Number && reader.TryGetInt64(out var intVal)) + { + seconds = intVal; + } + else + { + var doubleVal = reader.GetDouble(); + if (doubleVal > long.MaxValue || doubleVal < long.MinValue) + { + throw new JsonException("Unix time seconds out of range."); + } + seconds = Convert.ToInt64(Math.Truncate(doubleVal)); + } + return DateTimeOffset.FromUnixTimeSeconds(seconds); } Apply / Chat Suggestion importance[1-10]: 7 __ Why: Similar to the previous suggestion, this one correctly identifies a potential overflow when casting a double to a long and provides a more robust implementation with proper checks.	Medium
More