[dotnet] Switch DevTools response JSON parsing to JsonElement

[RenderMichael]

User description

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Switches JSON response parsing in DevTools from mutable JsonNode to immutable JsonElement.

Motivation and Context

In general, JsonElement is preferred if the JSON is not being mutated. But the true motivation is from #14972. JsonElement is a lot more forgiving of imperfect json (it allows multiple properties with the same name, it doesn't throw exceptions on invalid UTF-16 before the deserialization converters apply, etc).

This will allow us to Eventually fortify the code against invalid JSON, which is the root cause of #14903.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Bug fix, Enhancement

---

Description

• Switches JSON response parsing in DevTools from mutable JsonNode to immutable JsonElement across multiple files
• Improves JSON handling robustness by using JsonElement which is more forgiving of imperfect JSON
• Updates error handling to use proper JsonElement property access methods
• Fixes file upload issues when using Network.StartMonitoring() by addressing JSON parsing issues
• Adds better handling of invalid UTF-16 and duplicate property names in JSON responses

---

Changes walkthrough 📝

Relevant files

Enhancement

DevToolsCommandData.cs Switch Result property to JsonElement type                              dotnet/src/webdriver/DevTools/DevToolsCommandData.cs Changed Result property type from JsonNode to JsonElement Added System.Text.Json namespace import +2/-1      DevToolsSession.cs Refactor JSON handling to use JsonElement                                dotnet/src/webdriver/DevTools/DevToolsSession.cs Changed JSON parsing to use JsonElement instead of JsonNode Updated error handling to use JsonElement methods Modified message processing to use JsonDocument and JsonElement +20/-16  DevToolsSessionEventReceivedEventArgs.cs Update EventData to use JsonElement                                            dotnet/src/webdriver/DevTools/DevToolsSessionEventReceivedEventArgs.cs Changed EventData property type from JsonNode to JsonElement Updated constructor parameter type to match +3/-2      IDevToolsSession.cs Update SendCommand return type to JsonElement                        dotnet/src/webdriver/DevTools/IDevToolsSession.cs Changed return type of SendCommand from JsonNode to JsonElement? Added System.Text.Json namespace import +2/-1

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 14903 - Partially compliant Compliant requirements: None yet, this PR is a preparatory refactoring step Non-compliant requirements: Fix file upload failures when IWebDriver.Manage().Network.StartMonitoring() is called Support chunked file uploads (files > 4MB) Requires further human verification: Testing with large file uploads (>4MB) to verify the fix works Testing with Chrome browser version 131.0.6778.140

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling The new JsonElement error handling code needs careful review to ensure all error cases are properly handled, especially around null checks and property access var errorMessage = modified.Result.GetProperty("message").GetString(); var errorData = modified.Result.TryGetProperty("data", out var data) ? data.GetString() : null; var exceptionMessage = $"{commandName}: {errorMessage}"; if (!string.IsNullOrWhiteSpace(errorData)) { exceptionMessage = $"{exceptionMessage} - {errorData}"; } LogTrace("Recieved Error Response {0}: {1} {2}", modified.CommandId, message, errorData); throw new CommandResponseException(exceptionMessage) { Code = modified.Result.TryGetProperty("code", out var code) ? code.GetInt64() : -1 }; Memory Management Verify proper disposal of JsonDocument in ProcessMessage method to prevent memory leaks using (var doc = JsonDocument.Parse(message)) { messageObject = doc.RootElement.Clone(); }

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Add defensive programming to handle missing JSON properties safely Add null checks before accessing properties of JsonElement to prevent potential InvalidOperationException when properties don't exist. dotnet/src/webdriver/DevTools/DevToolsSession.cs [301-302] -var errorMessage = modified.Result.GetProperty("message").GetString(); +var errorMessage = modified.Result.TryGetProperty("message", out var messageElement) ? messageElement.GetString() ?? string.Empty : string.Empty; var errorData = modified.Result.TryGetProperty("data", out var data) ? data.GetString() : null; Apply this suggestion Suggestion importance[1-10]: 8 Why: The suggestion improves error handling by using TryGetProperty for the 'message' property, preventing potential runtime exceptions when processing malformed JSON responses. This is critical for maintaining application stability.	8
	Add input validation to prevent processing of invalid JSON messages Add validation for empty or malformed JSON messages before parsing to prevent potential exceptions. dotnet/src/webdriver/DevTools/DevToolsSession.cs [562-563] +if (string.IsNullOrWhiteSpace(message)) +{ + LogError("Received empty or invalid message"); + return; +} JsonElement messageObject; using (var doc = JsonDocument.Parse(message)) Apply this suggestion Suggestion importance[1-10]: 7 Why: Adding validation for empty or malformed messages before parsing is a good defensive programming practice that can prevent crashes and provide better error handling for invalid input.	7
General	Ensure proper resource cleanup with structured exception handling Dispose of JsonDocument using a try-finally block to ensure proper cleanup even if an exception occurs during processing. dotnet/src/webdriver/DevTools/DevToolsSession.cs [563-566] -using (var doc = JsonDocument.Parse(message)) -{ +JsonDocument doc = null; +try { + doc = JsonDocument.Parse(message); messageObject = doc.RootElement.Clone(); } +finally { + doc?.Dispose(); +} Apply this suggestion Suggestion importance[1-10]: 4 Why: While the existing using statement already ensures proper disposal, the suggested try-finally block is redundant as the using statement provides the same functionality. The improvement is minimal.	4

[nvborisenko]

This will allow us to Eventually fortify the code against invalid JSON, which is the root cause of #14903.

I still don't see how, can you please elaborate more?

it doesn't throw exceptions on invalid UTF-16 before the deserialization converters apply

00:29:37.899  WARN DevToolsSession: CDP VNT ^^ Unhandled error occured in event handler of 'Fetch.requestPaused' method. System.Text.Json.JsonException: The JSON value could not be converted to System.String. Path: $.request.postData | LineNumber: 0 | BytePositionInLine: 419706.
 ---> System.InvalidOperationException: Cannot read incomplete UTF-16 JSON text as string with missing low surrogate.
   at System.Text.Json.ThrowHelper.ThrowInvalidOperationException_ReadIncompleteUTF16()
   at System.Text.Json.JsonReaderHelper.TryUnescape(ReadOnlySpan`1 source, Span`1 destination, Int32 idx, Int32& written)
   at System.Text.Json.JsonReaderHelper.GetUnescapedString(ReadOnlySpan`1 utf8Source)
   at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.ReadJsonAndSetMember(Object obj, ReadStack& state, Utf8JsonReader& reader)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value, Boolean& isPopulatedValue)
   at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.ReadJsonAndSetMember(Object obj, ReadStack& state, Utf8JsonReader& reader)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value, Boolean& isPopulatedValue)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   --- End of inner exception stack trace ---
   at System.Text.Json.ThrowHelper.ReThrowWithPath(ReadStack& state, Utf8JsonReader& reader, Exception ex)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo`1.DeserializeAsObject(Utf8JsonReader& reader, ReadStack& state)
   at System.Text.Json.JsonSerializer.ReadFromSpanAsObject(ReadOnlySpan`1 utf8Json, JsonTypeInfo jsonTypeInfo, Nullable`1 actualByteCount)
   at System.Text.Json.JsonSerializer.Deserialize(JsonElement element, Type returnType, JsonSerializerOptions options)
   at OpenQA.Selenium.DevTools.V131.Fetch.FetchAdapter.OnDevToolsEventReceived(Object sender, DevToolsEventReceivedEventArgs e) in D:\SeleniumHQ\selenium\bazel-bin\dotnet\src\webdriver\cdp\v131\Fetch\FetchAdapter.cs:line 151
   at OpenQA.Selenium.DevTools.DevToolsSession.OnDevToolsEventReceived(DevToolsEventReceivedEventArgs e) in D:\SeleniumHQ\selenium\dotnet\src\webdriver\DevTools\DevToolsSession.cs:line 641
   at OpenQA.Selenium.DevTools.DevToolsSession.<>c__DisplayClass52_0.<ProcessMessage>b__0() in D:\SeleniumHQ\selenium\dotnet\src\webdriver\DevTools\DevToolsSession.cs:line 618

[RenderMichael]

JsonElement with invalid JSON contents will allow custom converters to run before the exception, JsonNode does not.

This PR is a first step towards handling the invalid JSON which CDP sometimes returns.

[nvborisenko]

When I was migrating from Newtonsoft.Json to System.Text.Json I chose JsonNode as more similar class to avoid big code changes. Seems JsonElement is more fundamental, let's try to use it.

[nvborisenko]

Thx @RenderMichael for your contribution!