[dotnet] [bidi] Make cookie expiry as TimeSpan

[nvborisenko]

User description

Finally Firefox and Chromium are aligned.

💥 What does this PR do?

Expiry means how long cookie lives in milliseconds.

🔧 Implementation Notes

Added new TimeSpan converter.

🔄 Types of changes

• Bug fix (backwards compatible)
• Breaking change (fix or feature that would cause existing functionality to change)

---

PR Type

Bug fix, Enhancement

---

Description

• Change cookie expiry from DateTimeOffset to TimeSpan

• Add TimeSpan JSON converter for BiDi communication

• Align Firefox and Chromium cookie handling

---

Diagram Walkthrough

flowchart LR
  A["Cookie.Expiry Property"] --> B["DateTimeOffset → TimeSpan"]
  B --> C["TimeSpanConverter"]
  C --> D["Broker Registration"]
  D --> E["Aligned Browser Support"]

Loading

File Walkthrough

	Relevant files
Configuration changes	Broker.cs Register TimeSpan converter                                                            dotnet/src/webdriver/BiDi/Communication/Broker.cs Register new TimeSpanConverter in the broker's converter list +1/-0
Enhancement	TimeSpanConverter.cs Add TimeSpan JSON converter                                                            dotnet/src/webdriver/BiDi/Communication/Json/Converters/TimeSpanConverter.cs Create new JSON converter for TimeSpan type Handle milliseconds conversion from/to JSON Support both integer and double value parsing +44/-0
Bug fix	Cookie.cs Update cookie expiry type                                                                dotnet/src/webdriver/BiDi/Network/Cookie.cs Change Expiry property type from DateTimeOffset to TimeSpan +1/-1

---

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Precision Loss Converting a JSON double to milliseconds via Convert.ToInt64 will truncate sub-millisecond precision; verify that protocol values never rely on fractional milliseconds or consider rounding behavior explicitly. public override TimeSpan Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { if (reader.TryGetInt64(out long milliseconds) is false) { var doubleValue = reader.GetDouble(); milliseconds = Convert.ToInt64(doubleValue); } return TimeSpan.FromMilliseconds(milliseconds); Semantics Change Changing cookie expiry from DateTimeOffset? to TimeSpan? alters meaning from absolute time to duration; ensure all serialization, deserialization, and downstream usages (comparisons, logging) are updated accordingly to avoid misinterpretation. public sealed record Cookie(string Name, BytesValue Value, string Domain, string Path, long Size, bool HttpOnly, bool Secure, SameSite SameSite) { [JsonInclude] public TimeSpan? Expiry { get; internal set; } } Read Validation Read assumes numeric token; consider handling unexpected token types or nulls to produce clearer errors and align with protocol robustness. public override TimeSpan Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { if (reader.TryGetInt64(out long milliseconds) is false) { var doubleValue = reader.GetDouble(); milliseconds = Convert.ToInt64(doubleValue); } return TimeSpan.FromMilliseconds(milliseconds);

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Clarify TimeSpan units/precision The new TimeSpanConverter assumes expiry is milliseconds, but silently converts doubles to long, potentially truncating sub-millisecond precision and risking misinterpretation if servers send seconds. Explicitly document and enforce the expected unit (ms) and consider validating or rejecting non-integer/alternative units to avoid cross-browser inconsistencies. Ensure all BiDi message producers/consumers and existing DateTimeOffset usages are migrated to this contract to prevent subtle expiry drift. Examples: dotnet/src/webdriver/BiDi/Communication/Json/Converters/TimeSpanConverter.cs [28-38] public override TimeSpan Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { if (reader.TryGetInt64(out long milliseconds) is false) { var doubleValue = reader.GetDouble(); milliseconds = Convert.ToInt64(doubleValue); } return TimeSpan.FromMilliseconds(milliseconds); ... (clipped 1 lines) Solution Walkthrough: Before: public override TimeSpan Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { if (reader.TryGetInt64(out long milliseconds) is false) { var doubleValue = reader.GetDouble(); milliseconds = Convert.ToInt64(doubleValue); } return TimeSpan.FromMilliseconds(milliseconds); } After: public override TimeSpan Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { // Per spec, expiry is in whole milliseconds. Rejecting floating-point // values prevents silent precision loss and potential unit confusion. if (reader.TryGetInt64(out long milliseconds)) { return TimeSpan.FromMilliseconds(milliseconds); } throw new JsonException("Expected cookie expiry to be an integer representing milliseconds."); } Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies a critical issue where silent truncation of floating-point values in TimeSpanConverter could lead to incorrect cookie expiry times and cross-browser inconsistencies.	Medium
Possible issue	Validate JSON token types Guard against unexpected token types to avoid throwing when the JSON value is not a number. Explicitly validate reader.TokenType and throw a descriptive JsonException if the token isn't an integer or floating-point number. dotnet/src/webdriver/BiDi/Communication/Json/Converters/TimeSpanConverter.cs [28-38] public override TimeSpan Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { - if (reader.TryGetInt64(out long milliseconds) is false) + if (reader.TokenType == JsonTokenType.Number) { - var doubleValue = reader.GetDouble(); + if (reader.TryGetInt64(out long milliseconds)) + { + return TimeSpan.FromMilliseconds(milliseconds); + } - milliseconds = Convert.ToInt64(doubleValue); + if (reader.TryGetDouble(out double doubleValue)) + { + return TimeSpan.FromMilliseconds(doubleValue); + } } - return TimeSpan.FromMilliseconds(milliseconds); + throw new JsonException($"Invalid JSON token for TimeSpan: expected number, got {reader.TokenType}."); } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that the Read method lacks validation for the JSON token type, which could lead to an InvalidOperationException. The proposed change improves robustness and provides more descriptive error messages by checking the token type and throwing a JsonException for unexpected types.	Medium
General	Normalize milliseconds precision Ensure consistent numeric precision by writing integral milliseconds when the value is whole to avoid unnecessary floating-point serialization differences. This reduces potential protocol mismatches where integers are expected. dotnet/src/webdriver/BiDi/Communication/Json/Converters/TimeSpanConverter.cs [40-43] public override void Write(Utf8JsonWriter writer, TimeSpan value, JsonSerializerOptions options) { - writer.WriteNumberValue(value.TotalMilliseconds); + double totalMs = value.TotalMilliseconds; + if (Math.Abs(totalMs % 1) < double.Epsilon) + { + writer.WriteNumberValue((long)totalMs); + } + else + { + writer.WriteNumberValue(totalMs); + } } Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out that serializing whole-number milliseconds as integers instead of doubles can improve consistency with protocols that might expect an integer. This is a good practice for ensuring predictable JSON output, making the implementation more robust.	Low
Update