From cef8da180587628c268f893550dd55df1b1fd3cd Mon Sep 17 00:00:00 2001 From: Michael Render Date: Fri, 17 Jan 2025 17:36:23 -0500 Subject: [PATCH 1/8] [dotnet] Fully annotate nullability on `HttpCommandExecutor` --- .../webdriver/Remote/HttpCommandExecutor.cs | 94 ++++++++++--------- 1 file changed, 52 insertions(+), 42 deletions(-) diff --git a/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs b/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs index d9c24591fc69a..643c0b490fe1c 100644 --- a/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs +++ b/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs @@ -30,6 +30,8 @@ using System.Threading; using System.Threading.Tasks; +#nullable enable + namespace OpenQA.Selenium.Remote { /// @@ -47,7 +49,8 @@ public class HttpCommandExecutor : ICommandExecutor private readonly TimeSpan serverResponseTimeout; private bool isDisposed; private CommandInfoRepository commandInfoRepository = new W3CWireProtocolCommandInfoRepository(); - private HttpClient client; + private HttpClient? client; + private readonly object _createClientLock = new(); private static readonly ILogger _logger = Log.GetLogger(); @@ -56,6 +59,7 @@ public class HttpCommandExecutor : ICommandExecutor /// /// Address of the WebDriver Server /// The timeout within which the server must respond. + /// If is . public HttpCommandExecutor(Uri addressOfRemoteServer, TimeSpan timeout) : this(addressOfRemoteServer, timeout, true) { @@ -91,14 +95,14 @@ public HttpCommandExecutor(Uri addressOfRemoteServer, TimeSpan timeout, bool ena /// Occurs when the is sending an HTTP /// request to the remote end WebDriver implementation. /// - public event EventHandler SendingRemoteHttpRequest; + public event EventHandler? SendingRemoteHttpRequest; /// /// Gets or sets an object to be used to proxy requests /// between this and the remote end WebDriver /// implementation. /// - public IWebProxy Proxy { get; set; } + public IWebProxy? Proxy { get; set; } /// /// Gets or sets a value indicating whether keep-alive is enabled for HTTP @@ -167,17 +171,12 @@ public virtual async Task ExecuteAsync(Command commandToExecute) _logger.Debug($"Executing command: {commandToExecute}"); } - HttpCommandInfo info = this.commandInfoRepository.GetCommandInfo(commandToExecute.Name); + HttpCommandInfo? info = this.commandInfoRepository.GetCommandInfo(commandToExecute.Name); if (info == null) { throw new NotImplementedException(string.Format("The command you are attempting to execute, {0}, does not exist in the protocol dialect used by the remote end.", commandToExecute.Name)); } - if (this.client == null) - { - this.CreateHttpClient(); - } - HttpRequestInfo requestInfo = new HttpRequestInfo(this.remoteServerUri, commandToExecute, info); HttpResponseInfo responseInfo; try @@ -216,42 +215,55 @@ protected virtual void OnSendingRemoteHttpRequest(SendingRemoteHttpRequestEventA throw new ArgumentNullException(nameof(eventArgs), "eventArgs must not be null"); } - if (this.SendingRemoteHttpRequest != null) - { - this.SendingRemoteHttpRequest(this, eventArgs); - } + this.SendingRemoteHttpRequest?.Invoke(this, eventArgs); } - private void CreateHttpClient() + private HttpClient Client { - HttpClientHandler httpClientHandler = new HttpClientHandler(); - string userInfo = this.remoteServerUri.UserInfo; - if (!string.IsNullOrEmpty(userInfo) && userInfo.Contains(":")) - { - string[] userInfoComponents = this.remoteServerUri.UserInfo.Split(new char[] { ':' }, 2); - httpClientHandler.Credentials = new NetworkCredential(userInfoComponents[0], userInfoComponents[1]); - httpClientHandler.PreAuthenticate = true; - } - - httpClientHandler.Proxy = this.Proxy; - - HttpMessageHandler handler = httpClientHandler; - - if (_logger.IsEnabled(LogEventLevel.Trace)) + get { - handler = new DiagnosticsHttpHandler(httpClientHandler, _logger); - } + if (this.client is null) + { + lock (_createClientLock) + { + if (this.client is null) + { + HttpClientHandler httpClientHandler = new HttpClientHandler(); + string userInfo = this.remoteServerUri.UserInfo; + if (!string.IsNullOrEmpty(userInfo) && userInfo.Contains(":")) + { + string[] userInfoComponents = this.remoteServerUri.UserInfo.Split(new char[] { ':' }, 2); + httpClientHandler.Credentials = new NetworkCredential(userInfoComponents[0], userInfoComponents[1]); + httpClientHandler.PreAuthenticate = true; + } + + httpClientHandler.Proxy = this.Proxy; + + HttpMessageHandler handler = httpClientHandler; + + if (_logger.IsEnabled(LogEventLevel.Trace)) + { + handler = new DiagnosticsHttpHandler(httpClientHandler, _logger); + } + + var client = new HttpClient(handler); + client.DefaultRequestHeaders.UserAgent.ParseAdd(this.UserAgent); + client.DefaultRequestHeaders.Accept.ParseAdd(RequestAcceptHeader); + client.DefaultRequestHeaders.ExpectContinue = false; + if (!this.IsKeepAliveEnabled) + { + client.DefaultRequestHeaders.Connection.ParseAdd("close"); + } + + client.Timeout = this.serverResponseTimeout; + + this.client = client; + } + } + } - this.client = new HttpClient(handler); - this.client.DefaultRequestHeaders.UserAgent.ParseAdd(this.UserAgent); - this.client.DefaultRequestHeaders.Accept.ParseAdd(RequestAcceptHeader); - this.client.DefaultRequestHeaders.ExpectContinue = false; - if (!this.IsKeepAliveEnabled) - { - this.client.DefaultRequestHeaders.Connection.ParseAdd("close"); + return this.client; } - - this.client.Timeout = this.serverResponseTimeout; } private async Task MakeHttpRequest(HttpRequestInfo requestInfo) @@ -288,7 +300,7 @@ private async Task MakeHttpRequest(HttpRequestInfo requestInfo requestMessage.Content.Headers.ContentType = contentTypeHeader; } - using (HttpResponseMessage responseMessage = await this.client.SendAsync(requestMessage).ConfigureAwait(false)) + using (HttpResponseMessage responseMessage = await this.Client.SendAsync(requestMessage).ConfigureAwait(false)) { var responseBody = await responseMessage.Content.ReadAsStringAsync().ConfigureAwait(false); var responseContentType = responseMessage.Content.Headers.ContentType?.ToString(); @@ -331,8 +343,6 @@ private Response CreateResponse(HttpResponseInfo responseInfo) return response; } -#nullable enable - /// /// Releases all resources used by the . /// From a0c3e4dee9dd60b213e52626a49adc2cc7ef9dd5 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Fri, 17 Jan 2025 18:08:18 -0500 Subject: [PATCH 2/8] Convert `HttpCommandExecutor` to `Lazy` --- .../webdriver/Remote/HttpCommandExecutor.cs | 81 ++++++++----------- 1 file changed, 34 insertions(+), 47 deletions(-) diff --git a/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs b/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs index 643c0b490fe1c..05f91e33f646e 100644 --- a/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs +++ b/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs @@ -26,6 +26,7 @@ using System.Net; using System.Net.Http; using System.Net.Http.Headers; +using System.Runtime.CompilerServices; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -49,8 +50,7 @@ public class HttpCommandExecutor : ICommandExecutor private readonly TimeSpan serverResponseTimeout; private bool isDisposed; private CommandInfoRepository commandInfoRepository = new W3CWireProtocolCommandInfoRepository(); - private HttpClient? client; - private readonly object _createClientLock = new(); + private readonly Lazy client; private static readonly ILogger _logger = Log.GetLogger(); @@ -89,6 +89,7 @@ public HttpCommandExecutor(Uri addressOfRemoteServer, TimeSpan timeout, bool ena this.remoteServerUri = addressOfRemoteServer; this.serverResponseTimeout = timeout; this.IsKeepAliveEnabled = enableKeepAlive; + this.client = new Lazy(CreateHttpClient); } /// @@ -218,54 +219,37 @@ protected virtual void OnSendingRemoteHttpRequest(SendingRemoteHttpRequestEventA this.SendingRemoteHttpRequest?.Invoke(this, eventArgs); } - private HttpClient Client + private HttpClient CreateHttpClient() { - get + HttpClientHandler httpClientHandler = new HttpClientHandler(); + string userInfo = this.remoteServerUri.UserInfo; + if (!string.IsNullOrEmpty(userInfo) && userInfo.Contains(":")) { - if (this.client is null) - { - lock (_createClientLock) - { - if (this.client is null) - { - HttpClientHandler httpClientHandler = new HttpClientHandler(); - string userInfo = this.remoteServerUri.UserInfo; - if (!string.IsNullOrEmpty(userInfo) && userInfo.Contains(":")) - { - string[] userInfoComponents = this.remoteServerUri.UserInfo.Split(new char[] { ':' }, 2); - httpClientHandler.Credentials = new NetworkCredential(userInfoComponents[0], userInfoComponents[1]); - httpClientHandler.PreAuthenticate = true; - } - - httpClientHandler.Proxy = this.Proxy; - - HttpMessageHandler handler = httpClientHandler; - - if (_logger.IsEnabled(LogEventLevel.Trace)) - { - handler = new DiagnosticsHttpHandler(httpClientHandler, _logger); - } - - var client = new HttpClient(handler); - client.DefaultRequestHeaders.UserAgent.ParseAdd(this.UserAgent); - client.DefaultRequestHeaders.Accept.ParseAdd(RequestAcceptHeader); - client.DefaultRequestHeaders.ExpectContinue = false; - if (!this.IsKeepAliveEnabled) - { - client.DefaultRequestHeaders.Connection.ParseAdd("close"); - } - - client.Timeout = this.serverResponseTimeout; - - this.client = client; - } - } - } + string[] userInfoComponents = this.remoteServerUri.UserInfo.Split(new char[] { ':' }, 2); + httpClientHandler.Credentials = new NetworkCredential(userInfoComponents[0], userInfoComponents[1]); + httpClientHandler.PreAuthenticate = true; + } + + httpClientHandler.Proxy = this.Proxy; - return this.client; + HttpMessageHandler handler = httpClientHandler; + + if (_logger.IsEnabled(LogEventLevel.Trace)) + { + handler = new DiagnosticsHttpHandler(httpClientHandler, _logger); } - } + var client = new HttpClient(handler); + client.DefaultRequestHeaders.UserAgent.ParseAdd(this.UserAgent); + client.DefaultRequestHeaders.Accept.ParseAdd(RequestAcceptHeader); + client.DefaultRequestHeaders.ExpectContinue = false; + if (!this.IsKeepAliveEnabled) + { + client.DefaultRequestHeaders.Connection.ParseAdd("close"); + } + return client; + + } private async Task MakeHttpRequest(HttpRequestInfo requestInfo) { SendingRemoteHttpRequestEventArgs eventArgs = new SendingRemoteHttpRequestEventArgs(requestInfo.HttpMethod, requestInfo.FullUri.ToString(), requestInfo.RequestBody); @@ -300,7 +284,7 @@ private async Task MakeHttpRequest(HttpRequestInfo requestInfo requestMessage.Content.Headers.ContentType = contentTypeHeader; } - using (HttpResponseMessage responseMessage = await this.Client.SendAsync(requestMessage).ConfigureAwait(false)) + using (HttpResponseMessage responseMessage = await this.client.Value.SendAsync(requestMessage).ConfigureAwait(false)) { var responseBody = await responseMessage.Content.ReadAsStringAsync().ConfigureAwait(false); var responseContentType = responseMessage.Content.Headers.ContentType?.ToString(); @@ -362,7 +346,10 @@ protected virtual void Dispose(bool disposing) { if (!this.isDisposed) { - this.client?.Dispose(); + if (this.client.IsValueCreated) + { + this.client.Value.Dispose(); + } this.isDisposed = true; } From d3e49d5b729b4d67c5e7e82f32a55b212cbdf2d0 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Mon, 20 Jan 2025 00:53:25 -0500 Subject: [PATCH 3/8] Fix missing `HttpClient.Timeout` set, fix whitespace --- dotnet/src/webdriver/Remote/HttpCommandExecutor.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs b/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs index 05f91e33f646e..677dc0aa35bbc 100644 --- a/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs +++ b/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs @@ -247,9 +247,11 @@ private HttpClient CreateHttpClient() { client.DefaultRequestHeaders.Connection.ParseAdd("close"); } - return client; + client.Timeout = this.serverResponseTimeout; + return client; } + private async Task MakeHttpRequest(HttpRequestInfo requestInfo) { SendingRemoteHttpRequestEventArgs eventArgs = new SendingRemoteHttpRequestEventArgs(requestInfo.HttpMethod, requestInfo.FullUri.ToString(), requestInfo.RequestBody); From 2e3b981915b69d641d1da1417b2761f948020ce4 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Mon, 3 Feb 2025 02:49:59 -0500 Subject: [PATCH 4/8] Broaden changes to include HTTP execution in general --- dotnet/src/webdriver/CommandInfo.cs | 12 ++++--- dotnet/src/webdriver/HttpCommandInfo.cs | 35 +++++++------------ dotnet/src/webdriver/ICommandExecutor.cs | 9 +++-- .../webdriver/ICustomDriverCommandExecutor.cs | 7 ++-- .../Remote/DriverServiceCommandExecutor.cs | 5 ++- .../webdriver/Remote/HttpCommandExecutor.cs | 12 ++++--- dotnet/src/webdriver/WebDriver.cs | 29 ++++++++++----- 7 files changed, 64 insertions(+), 45 deletions(-) diff --git a/dotnet/src/webdriver/CommandInfo.cs b/dotnet/src/webdriver/CommandInfo.cs index 0e679e221dcc7..a1bde2f141a20 100644 --- a/dotnet/src/webdriver/CommandInfo.cs +++ b/dotnet/src/webdriver/CommandInfo.cs @@ -19,6 +19,8 @@ using System; +#nullable enable + namespace OpenQA.Selenium { /// @@ -45,7 +47,7 @@ public override int GetHashCode() /// /// The to compare to this instance. /// if is a and its value is the same as this instance; otherwise, . If is , the method returns . - public override bool Equals(object obj) + public override bool Equals(object? obj) { return this.Equals(obj as CommandInfo); } @@ -55,7 +57,7 @@ public override bool Equals(object obj) /// /// The to compare to this instance. /// if the value of the parameter is the same as this instance; otherwise, . If is , the method returns . - public bool Equals(CommandInfo other) + public bool Equals(CommandInfo? other) { if (other is null) { @@ -63,7 +65,7 @@ public bool Equals(CommandInfo other) } // Optimization for a common success case. - if (Object.ReferenceEquals(this, other)) + if (object.ReferenceEquals(this, other)) { return true; } @@ -86,7 +88,7 @@ public bool Equals(CommandInfo other) /// The first object to compare. /// The second object to compare. /// if the value of is the same as the value of ; otherwise, . - public static bool operator ==(CommandInfo left, CommandInfo right) + public static bool operator ==(CommandInfo? left, CommandInfo? right) { if (left is null) { @@ -107,7 +109,7 @@ public bool Equals(CommandInfo other) /// The first object to compare. /// The second object to compare. /// if the value of is different from the value of ; otherwise, . - public static bool operator !=(CommandInfo left, CommandInfo right) + public static bool operator !=(CommandInfo? left, CommandInfo? right) { return !(left == right); } diff --git a/dotnet/src/webdriver/HttpCommandInfo.cs b/dotnet/src/webdriver/HttpCommandInfo.cs index 0841ffcde7524..36965d875cd5b 100644 --- a/dotnet/src/webdriver/HttpCommandInfo.cs +++ b/dotnet/src/webdriver/HttpCommandInfo.cs @@ -20,6 +20,8 @@ using System; using System.Globalization; +#nullable enable + namespace OpenQA.Selenium { /// @@ -44,9 +46,6 @@ public class HttpCommandInfo : CommandInfo private const string SessionIdPropertyName = "sessionId"; - private string resourcePath; - private string method; - /// /// Initializes a new instance of the class /// @@ -54,32 +53,26 @@ public class HttpCommandInfo : CommandInfo /// Relative URL path to the resource used to execute the command public HttpCommandInfo(string method, string resourcePath) { - this.resourcePath = resourcePath; - this.method = method; + this.ResourcePath = resourcePath; + this.Method = method; } /// /// Gets the URL representing the path to the resource. /// - public string ResourcePath - { - get { return this.resourcePath; } - } + public string ResourcePath { get; } /// /// Gets the HTTP method associated with the command. /// - public string Method - { - get { return this.method; } - } + public string Method { get; } /// /// Gets the unique identifier for this command within the scope of its protocol definition /// public override string CommandIdentifier { - get { return string.Format(CultureInfo.InvariantCulture, "{0} {1}", this.method, this.resourcePath); } + get { return string.Format(CultureInfo.InvariantCulture, "{0} {1}", this.Method, this.ResourcePath); } } /// @@ -93,7 +86,7 @@ public override string CommandIdentifier /// substituted for the tokens in the template. public Uri CreateCommandUri(Uri baseUri, Command commandToExecute) { - string[] urlParts = this.resourcePath.Split(new string[] { "/" }, StringSplitOptions.RemoveEmptyEntries); + string[] urlParts = this.ResourcePath.Split(["/"], StringSplitOptions.RemoveEmptyEntries); for (int i = 0; i < urlParts.Length; i++) { string urlPart = urlParts[i]; @@ -103,13 +96,11 @@ public Uri CreateCommandUri(Uri baseUri, Command commandToExecute) } } - Uri fullUri; string relativeUrlString = string.Join("/", urlParts); Uri relativeUri = new Uri(relativeUrlString, UriKind.Relative); - bool uriCreateSucceeded = Uri.TryCreate(baseUri, relativeUri, out fullUri); - if (!uriCreateSucceeded) + if (!Uri.TryCreate(baseUri, relativeUri, out Uri? fullUri)) { - throw new InvalidOperationException(string.Format(CultureInfo.InvariantCulture, "Unable to create URI from base {0} and relative path {1}", baseUri == null ? string.Empty : baseUri.ToString(), relativeUrlString)); + throw new InvalidOperationException(string.Format(CultureInfo.InvariantCulture, "Unable to create URI from base {0} and relative path {1}", baseUri?.ToString(), relativeUrlString)); } return fullUri; @@ -133,11 +124,11 @@ private static string GetCommandPropertyValue(string propertyName, Command comma { // Extract the URL parameter, and remove it from the parameters dictionary // so it doesn't get transmitted as a JSON parameter. - if (commandToExecute.Parameters.ContainsKey(propertyName)) + if (commandToExecute.Parameters.TryGetValue(propertyName, out var propertyValueObject)) { - if (commandToExecute.Parameters[propertyName] != null) + if (propertyValueObject != null) { - propertyValue = commandToExecute.Parameters[propertyName].ToString(); + propertyValue = propertyValueObject.ToString()!; commandToExecute.Parameters.Remove(propertyName); } } diff --git a/dotnet/src/webdriver/ICommandExecutor.cs b/dotnet/src/webdriver/ICommandExecutor.cs index e80d7ea63b10e..6fef4d9bf3ef1 100644 --- a/dotnet/src/webdriver/ICommandExecutor.cs +++ b/dotnet/src/webdriver/ICommandExecutor.cs @@ -18,8 +18,11 @@ // using System; +using System.Diagnostics.CodeAnalysis; using System.Threading.Tasks; +#nullable enable + namespace OpenQA.Selenium { /// @@ -31,15 +34,16 @@ public interface ICommandExecutor : IDisposable /// Attempts to add a command to the repository of commands known to this executor. /// /// The name of the command to attempt to add. - /// The describing the commnd to add. + /// The describing the command to add. /// if the new command has been added successfully; otherwise, . - bool TryAddCommand(string commandName, CommandInfo info); + bool TryAddCommand(string commandName, [NotNullWhen(true)] CommandInfo? info); /// /// Executes a command /// /// The command you wish to execute /// A response from the browser + /// If is . Response Execute(Command commandToExecute); @@ -48,6 +52,7 @@ public interface ICommandExecutor : IDisposable /// /// The command you wish to execute /// A task object representing the asynchronous operation + /// If is . Task ExecuteAsync(Command commandToExecute); } } diff --git a/dotnet/src/webdriver/ICustomDriverCommandExecutor.cs b/dotnet/src/webdriver/ICustomDriverCommandExecutor.cs index 07f2169490391..66a2eec2e0e79 100644 --- a/dotnet/src/webdriver/ICustomDriverCommandExecutor.cs +++ b/dotnet/src/webdriver/ICustomDriverCommandExecutor.cs @@ -18,6 +18,9 @@ // using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; + +#nullable enable namespace OpenQA.Selenium { @@ -32,7 +35,7 @@ public interface ICustomDriverCommandExecutor /// The name of the command to execute. The command name must be registered with the command executor, and must not be a command name known to this driver type. /// A containing the names and values of the parameters of the command. /// An object that contains the value returned by the command, if any. - object ExecuteCustomDriverCommand(string driverCommandToExecute, Dictionary parameters); + object? ExecuteCustomDriverCommand(string driverCommandToExecute, Dictionary parameters); /// /// Registers a set of commands to be executed with this driver instance. @@ -46,6 +49,6 @@ public interface ICustomDriverCommandExecutor /// The unique name of the command to register. /// The object describing the command. /// if the command was registered; otherwise, . - bool RegisterCustomDriverCommand(string commandName, CommandInfo commandInfo); + bool RegisterCustomDriverCommand(string commandName, [NotNullWhen(true)] CommandInfo? commandInfo); } } diff --git a/dotnet/src/webdriver/Remote/DriverServiceCommandExecutor.cs b/dotnet/src/webdriver/Remote/DriverServiceCommandExecutor.cs index b9bb4efcf0084..0f1d341c8c353 100644 --- a/dotnet/src/webdriver/Remote/DriverServiceCommandExecutor.cs +++ b/dotnet/src/webdriver/Remote/DriverServiceCommandExecutor.cs @@ -18,6 +18,7 @@ // using System; +using System.Diagnostics.CodeAnalysis; using System.Threading.Tasks; #nullable enable @@ -78,7 +79,7 @@ public DriverServiceCommandExecutor(DriverService service, HttpCommandExecutor c // get { return this.HttpExecutor.CommandInfoRepository; } //} - public bool TryAddCommand(string commandName, CommandInfo info) + public bool TryAddCommand(string commandName, [NotNullWhen(true)] CommandInfo? info) { return this.HttpExecutor.TryAddCommand(commandName, info); } @@ -94,6 +95,7 @@ public bool TryAddCommand(string commandName, CommandInfo info) /// /// The command you wish to execute /// A response from the browser + /// If is . public Response Execute(Command commandToExecute) { return Task.Run(() => this.ExecuteAsync(commandToExecute)).GetAwaiter().GetResult(); @@ -104,6 +106,7 @@ public Response Execute(Command commandToExecute) /// /// The command you wish to execute /// A task object representing the asynchronous operation + /// If is . public async Task ExecuteAsync(Command commandToExecute) { if (commandToExecute == null) diff --git a/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs b/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs index 677dc0aa35bbc..299064daa7a20 100644 --- a/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs +++ b/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs @@ -21,12 +21,12 @@ using OpenQA.Selenium.Internal.Logging; using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Linq; using System.Net; using System.Net.Http; using System.Net.Http.Headers; -using System.Runtime.CompilerServices; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -114,7 +114,7 @@ public HttpCommandExecutor(Uri addressOfRemoteServer, TimeSpan timeout, bool ena /// /// Gets or sets the user agent string used for HTTP communication - /// batween this and the remote end + /// between this and the remote end /// WebDriver implementation /// public string UserAgent { get; set; } @@ -133,9 +133,9 @@ protected CommandInfoRepository CommandInfoRepository /// Attempts to add a command to the repository of commands known to this executor. /// /// The name of the command to attempt to add. - /// The describing the commnd to add. + /// The describing the command to add. /// if the new command has been added successfully; otherwise, . - public bool TryAddCommand(string commandName, CommandInfo info) + public bool TryAddCommand(string commandName, [NotNullWhen(true)] CommandInfo? info) { if (info is not HttpCommandInfo commandInfo) { @@ -150,6 +150,7 @@ public bool TryAddCommand(string commandName, CommandInfo info) /// /// The command you wish to execute. /// A response from the browser. + /// If is . public virtual Response Execute(Command commandToExecute) { return Task.Run(() => this.ExecuteAsync(commandToExecute)).GetAwaiter().GetResult(); @@ -160,6 +161,7 @@ public virtual Response Execute(Command commandToExecute) /// /// The command you wish to execute. /// A task object representing the asynchronous operation. + /// If is . public virtual async Task ExecuteAsync(Command commandToExecute) { if (commandToExecute == null) @@ -169,7 +171,7 @@ public virtual async Task ExecuteAsync(Command commandToExecute) if (_logger.IsEnabled(LogEventLevel.Debug)) { - _logger.Debug($"Executing command: {commandToExecute}"); + _logger.Debug($"Executing command: [{commandToExecute.SessionId}]: {commandToExecute.Name} {commandToExecute.ParametersAsJsonString}"); } HttpCommandInfo? info = this.commandInfoRepository.GetCommandInfo(commandToExecute.Name); diff --git a/dotnet/src/webdriver/WebDriver.cs b/dotnet/src/webdriver/WebDriver.cs index 974cbf608fcba..b5e8d5b6437b5 100644 --- a/dotnet/src/webdriver/WebDriver.cs +++ b/dotnet/src/webdriver/WebDriver.cs @@ -463,13 +463,16 @@ public INavigation Navigate() return new Navigator(this); } +#nullable enable + /// /// Executes a command with this driver. /// /// The name of the command to execute. The command name must be registered with the command executor, and must not be a command name known to this driver type. /// A containing the names and values of the parameters of the command. /// A containing information about the success or failure of the command and any data returned by the command. - public object ExecuteCustomDriverCommand(string driverCommandToExecute, Dictionary parameters) + /// The command returned an exceptional value. + public object? ExecuteCustomDriverCommand(string driverCommandToExecute, Dictionary parameters) { if (this.registeredCommands.Contains(driverCommandToExecute)) { @@ -497,7 +500,7 @@ public void RegisterCustomDriverCommands(IReadOnlyDictionaryThe unique name of the command to register. /// The object describing the command. /// if the command was registered; otherwise, . - public bool RegisterCustomDriverCommand(string commandName, CommandInfo commandInfo) + public bool RegisterCustomDriverCommand(string commandName, [NotNullWhen(true)] CommandInfo? commandInfo) { return this.RegisterDriverCommand(commandName, commandInfo, false); } @@ -509,17 +512,23 @@ public bool RegisterCustomDriverCommand(string commandName, CommandInfo commandI /// The object describing the command. /// if the registered command is internal to the driver; otherwise . /// if the command was registered; otherwise, . - internal bool RegisterDriverCommand(string commandName, CommandInfo commandInfo, bool isInternalCommand) + internal bool RegisterDriverCommand(string commandName, [NotNullWhen(true)] CommandInfo? commandInfo, bool isInternalCommand) { - bool commandAdded = this.CommandExecutor.TryAddCommand(commandName, commandInfo); - if (commandAdded && isInternalCommand) + if (this.CommandExecutor.TryAddCommand(commandName, commandInfo)) { - this.registeredCommands.Add(commandName); + if (isInternalCommand) + { + this.registeredCommands.Add(commandName); + } + + return true; } - return commandAdded; + return false; } +#nullable restore + /// /// Find the element in the response /// @@ -692,17 +701,21 @@ protected virtual Dictionary GetCapabilitiesDictionary(ICapabili return capabilitiesDictionary; } +#nullable enable + /// /// Registers a command to be executed with this driver instance as an internally known driver command. /// /// The unique name of the command to register. /// The object describing the command. /// if the command was registered; otherwise, . - protected bool RegisterInternalDriverCommand(string commandName, CommandInfo commandInfo) + protected bool RegisterInternalDriverCommand(string commandName, [NotNullWhen(true)] CommandInfo? commandInfo) { return this.RegisterDriverCommand(commandName, commandInfo, true); } +#nullable restore + /// /// Stops the client from running /// From 93e0e92caebd9e536aab4e721c474bf14a88085b Mon Sep 17 00:00:00 2001 From: Michael Render Date: Mon, 3 Feb 2025 02:51:28 -0500 Subject: [PATCH 5/8] add XML doc about exception in `ICustomDriverCommandExecutor.ExecuteCustomDriverCommand` --- dotnet/src/webdriver/ICustomDriverCommandExecutor.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/dotnet/src/webdriver/ICustomDriverCommandExecutor.cs b/dotnet/src/webdriver/ICustomDriverCommandExecutor.cs index 66a2eec2e0e79..145bb8c66013d 100644 --- a/dotnet/src/webdriver/ICustomDriverCommandExecutor.cs +++ b/dotnet/src/webdriver/ICustomDriverCommandExecutor.cs @@ -35,6 +35,7 @@ public interface ICustomDriverCommandExecutor /// The name of the command to execute. The command name must be registered with the command executor, and must not be a command name known to this driver type. /// A containing the names and values of the parameters of the command. /// An object that contains the value returned by the command, if any. + /// The command returned an exceptional value. object? ExecuteCustomDriverCommand(string driverCommandToExecute, Dictionary parameters); /// From d0b9d47c75477d5fbe35400a815331b104b3ba5a Mon Sep 17 00:00:00 2001 From: Michael Render Date: Mon, 3 Feb 2025 12:28:17 -0500 Subject: [PATCH 6/8] Only log parameter commands at the Trace level --- dotnet/src/webdriver/Remote/HttpCommandExecutor.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs b/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs index 299064daa7a20..f93adb8960bd1 100644 --- a/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs +++ b/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs @@ -169,10 +169,14 @@ public virtual async Task ExecuteAsync(Command commandToExecute) throw new ArgumentNullException(nameof(commandToExecute), "commandToExecute cannot be null"); } - if (_logger.IsEnabled(LogEventLevel.Debug)) + if (_logger.IsEnabled(LogEventLevel.Trace)) { _logger.Debug($"Executing command: [{commandToExecute.SessionId}]: {commandToExecute.Name} {commandToExecute.ParametersAsJsonString}"); } + else if (_logger.IsEnabled(LogEventLevel.Debug)) + { + _logger.Debug($"Executing command: [{commandToExecute.SessionId}]: {commandToExecute.Name}"); + } HttpCommandInfo? info = this.commandInfoRepository.GetCommandInfo(commandToExecute.Name); if (info == null) From 15459e7491e68490c0d68aa5a819aa6ae88cdc71 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Mon, 3 Feb 2025 12:31:25 -0500 Subject: [PATCH 7/8] Do not log command parameters in HttpCommandExecutor at all --- dotnet/src/webdriver/Remote/HttpCommandExecutor.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs b/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs index f93adb8960bd1..a85f95f33724e 100644 --- a/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs +++ b/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs @@ -170,10 +170,6 @@ public virtual async Task ExecuteAsync(Command commandToExecute) } if (_logger.IsEnabled(LogEventLevel.Trace)) - { - _logger.Debug($"Executing command: [{commandToExecute.SessionId}]: {commandToExecute.Name} {commandToExecute.ParametersAsJsonString}"); - } - else if (_logger.IsEnabled(LogEventLevel.Debug)) { _logger.Debug($"Executing command: [{commandToExecute.SessionId}]: {commandToExecute.Name}"); } From 0b5703ba4d649831338f668ab6819425a26642b6 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Mon, 3 Feb 2025 12:31:50 -0500 Subject: [PATCH 8/8] fix push --- dotnet/src/webdriver/Remote/HttpCommandExecutor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs b/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs index a85f95f33724e..71cefbb38cd9a 100644 --- a/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs +++ b/dotnet/src/webdriver/Remote/HttpCommandExecutor.cs @@ -169,7 +169,7 @@ public virtual async Task ExecuteAsync(Command commandToExecute) throw new ArgumentNullException(nameof(commandToExecute), "commandToExecute cannot be null"); } - if (_logger.IsEnabled(LogEventLevel.Trace)) + if (_logger.IsEnabled(LogEventLevel.Debug)) { _logger.Debug($"Executing command: [{commandToExecute.SessionId}]: {commandToExecute.Name}"); }