[dotnet] Fully annotate nullability on HttpCommandExecutor

[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

Finishes annotating HttpCommandExecutor for nullability. Refactor the client creation to play nicer with nullability analysis.

Motivation and Context

Contributes to #14640

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

Enhancement, Bug fix

---

Description

• Fully annotated nullability in HttpCommandExecutor class.

• Refactored HttpClient creation with thread-safe lazy initialization.

• Improved nullability handling for properties and events.

• Replaced manual null checks with null-coalescing operators.

---

Changes walkthrough 📝

Relevant files

Enhancement

HttpCommandExecutor.cs Annotate nullability and refactor `HttpCommandExecutor`    dotnet/src/webdriver/Remote/HttpCommandExecutor.cs Enabled nullable reference types for the file. Annotated fields, properties, and events with nullable types. Refactored HttpClient creation to use a thread-safe lazy initialization pattern. Updated methods to handle nullability more robustly. Simplified event invocation using null-conditional operator. +52/-42

---

Need help?

Type /help how to ... in the comments thread for any question about Qodo Merge usage.

Check out the documentation for more information.

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

🧪 No relevant tests

🔒 Security concerns Credentials handling: The code processes user credentials from URI user info which is generally not recommended for security reasons as credentials in URLs can be logged or exposed. Consider using more secure credential passing mechanisms.

⚡ Recommended focus areas for review Thread Safety While the double-checked locking pattern is used for lazy initialization of HttpClient, verify that all other shared state access (like commandInfoRepository) is properly synchronized. private CommandInfoRepository commandInfoRepository = new W3CWireProtocolCommandInfoRepository(); private HttpClient? client; private readonly object _createClientLock = new(); Resource Cleanup The HttpClient instance is stored in a field but there's no explicit disposal in the Dispose method. Consider adding proper cleanup of the HttpClient instance. private HttpClient? client;

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	✅ Properly dispose of HTTP client resources to prevent memory leaks Suggestion Impact: The commit implements proper disposal of HttpClient resources, but uses a different approach with Lazy and checks IsValueCreated before disposal code diff: @@ -362,7 +346,10 @@ { if (!this.isDisposed) { - this.client?.Dispose(); + if (this.client.IsValueCreated) + { + this.client.Value.Dispose(); + } this.isDisposed = true; } Dispose of the HttpClient instance in the Dispose method to prevent resource leaks. The current implementation might leave the HttpClient hanging. dotnet/src/webdriver/Remote/HttpCommandExecutor.cs [52] private HttpClient? client; +public void Dispose() +{ + if (!isDisposed) + { + client?.Dispose(); + client = null; + isDisposed = true; + } +} + Apply this suggestion Suggestion importance[1-10]: 9 Why: The suggestion addresses a critical resource management issue by ensuring proper disposal of HttpClient instances. Not disposing of HttpClient can lead to memory leaks and socket exhaustion in long-running applications.	9
	Add defensive null checks when accessing HTTP response content to prevent runtime exceptions Add null check for responseMessage.Content before accessing it to prevent potential NullReferenceException. The HTTP response might have no content in certain scenarios. dotnet/src/webdriver/Remote/HttpCommandExecutor.cs [305-306] -var responseBody = await responseMessage.Content.ReadAsStringAsync().ConfigureAwait(false); -var responseContentType = responseMessage.Content.Headers.ContentType?.ToString(); +var responseBody = responseMessage.Content != null ? await responseMessage.Content.ReadAsStringAsync().ConfigureAwait(false) : string.Empty; +var responseContentType = responseMessage.Content?.Headers.ContentType?.ToString(); Apply this suggestion Suggestion importance[1-10]: 7 Why: The suggestion addresses a potential null reference exception by adding defensive null checks for HTTP response content. This is important for robustness as HTTP responses may not always contain content.	7

[RenderMichael]

Changes look a lot simpler with whitespace turned off.

[nvborisenko]

Private properties are confusing. Take into account: https://github.com/SeleniumHQ/selenium/pull/15110/files#r1920848814

[RenderMichael]

Nullability analysis was a lot simpler when changing CreateHttpClient() into a lazy-loaded Client.

[nvborisenko]

Please wait me here.

[nvborisenko]

Looks we can introduce Lazy<HttpClient> as a field. Already thread-safe for initialization.