From b142f528be951a71f2624a5749f10dde8aa0b617 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Fri, 28 Mar 2025 00:51:17 -0400 Subject: [PATCH 1/3] [dotnet] Fully annotate `Command` for AOT support --- dotnet/src/webdriver/Command.cs | 62 ++++++++++++++++++++----- dotnet/src/webdriver/HttpCommandInfo.cs | 10 ++-- 2 files changed, 53 insertions(+), 19 deletions(-) diff --git a/dotnet/src/webdriver/Command.cs b/dotnet/src/webdriver/Command.cs index ddf6ed4228995..9bcdccd0157a2 100644 --- a/dotnet/src/webdriver/Command.cs +++ b/dotnet/src/webdriver/Command.cs @@ -20,6 +20,7 @@ using OpenQA.Selenium.Internal; using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Text.Json; using System.Text.Json.Serialization; using System.Text.Json.Serialization.Metadata; @@ -31,15 +32,22 @@ namespace OpenQA.Selenium /// public class Command { - private readonly static JsonSerializerOptions s_jsonSerializerOptions = new() + [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = $"All trimming-unsafe access points to {nameof(JsonSerializerOptions)} are annotated as such")] + [UnconditionalSuppressMessage("AOT", "IL3050", Justification = $"All AOT-unsafe access points to {nameof(JsonSerializerOptions)} are annotated as such")] + private static class JsonOptionsHolder { - TypeInfoResolverChain = + public readonly static JsonSerializerOptions JsonSerializerOptions = new() { - CommandJsonSerializerContext.Default, - new DefaultJsonTypeInfoResolver() - }, - Converters = { new ResponseValueJsonConverter() } - }; + TypeInfoResolverChain = + { + CommandJsonSerializerContext.Default, + new DefaultJsonTypeInfoResolver() + }, + Converters = { new ResponseValueJsonConverter() } + }; + } + + private readonly Dictionary _parameters; /// /// Initializes a new instance of the class using a command name and a JSON-encoded string for the parameters. @@ -47,8 +55,10 @@ public class Command /// Name of the command /// Parameters for the command as a JSON-encoded string. public Command(string name, string jsonParameters) - : this(null, name, ConvertParametersFromJson(jsonParameters)) { + this.SessionId = null; + this._parameters = ConvertParametersFromJson(jsonParameters) ?? new Dictionary(); + this.Name = name ?? throw new ArgumentNullException(nameof(name)); } /// @@ -58,10 +68,12 @@ public Command(string name, string jsonParameters) /// Name of the command /// Parameters for that command /// If is . + [RequiresUnreferencedCode("Adding untyped parameter values for JSON serialization has best-effort AOT support. Ensure only Selenium types and well-known .NET types are added, or use the overload that takes pre-serialized string jsonParameters for guaranteed AOT compatibility.")] + [RequiresDynamicCode("Adding untyped parameter values for JSON serialization has best-effort AOT support. Ensure only Selenium types and well-known .NET types are added, or use the overload that takes pre-serialized string jsonParameters for guaranteed AOT compatibility.")] public Command(SessionId? sessionId, string name, Dictionary? parameters) { this.SessionId = sessionId; - this.Parameters = parameters ?? new Dictionary(); + this._parameters = parameters ?? new Dictionary(); this.Name = name ?? throw new ArgumentNullException(nameof(name)); } @@ -81,18 +93,25 @@ public Command(SessionId? sessionId, string name, Dictionary? p /// Gets the parameters of the command /// [JsonPropertyName("parameters")] - public Dictionary Parameters { get; } + public Dictionary Parameters + { + [RequiresUnreferencedCode("Adding untyped parameter values for JSON serialization has best-effort AOT support. Ensure only Selenium types and well-known .NET types are added.")] + [RequiresDynamicCode("Adding untyped parameter values for JSON serialization has best-effort AOT support. Ensure only Selenium types and well-known .NET types are added.")] + get => _parameters; + } /// /// Gets the parameters of the command as a JSON-encoded string. /// public string ParametersAsJsonString { + [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = $"All trimming-unsafe access points to {nameof(_parameters)} are annotated as such")] + [UnconditionalSuppressMessage("AOT", "IL3050", Justification = $"All AOT-unsafe access points to {nameof(_parameters)} are annotated as such")] get { - if (this.Parameters != null && this.Parameters.Count > 0) + if (HasParameters()) { - return JsonSerializer.Serialize(this.Parameters, s_jsonSerializerOptions); + return JsonSerializer.Serialize(this._parameters, JsonOptionsHolder.JsonSerializerOptions); } else { @@ -101,6 +120,25 @@ public string ParametersAsJsonString } } + internal bool HasParameters() + { + return this._parameters != null && this._parameters.Count > 0; + } + + internal bool TryGetValueAndRemoveIfNotNull(string key, [NotNullWhen(true)] out object? value) + { + if (this._parameters.TryGetValue(key, out value)) + { + if (value is not null) + { + this._parameters.Remove(key); + return true; + } + } + + return false; + } + /// /// Returns a string of the Command object /// diff --git a/dotnet/src/webdriver/HttpCommandInfo.cs b/dotnet/src/webdriver/HttpCommandInfo.cs index ecca593ce55de..8cb4ac3b02fbd 100644 --- a/dotnet/src/webdriver/HttpCommandInfo.cs +++ b/dotnet/src/webdriver/HttpCommandInfo.cs @@ -118,17 +118,13 @@ private static string GetCommandPropertyValue(string propertyName, Command comma propertyValue = commandToExecute.SessionId.ToString(); } } - else if (commandToExecute.Parameters != null && commandToExecute.Parameters.Count > 0) + else if (commandToExecute.HasParameters()) { // Extract the URL parameter, and remove it from the parameters dictionary // so it doesn't get transmitted as a JSON parameter. - if (commandToExecute.Parameters.TryGetValue(propertyName, out var propertyValueObject)) + if (commandToExecute.TryGetValueAndRemoveIfNotNull(propertyName, out var propertyValueObject)) { - if (propertyValueObject != null) - { - propertyValue = propertyValueObject.ToString()!; - commandToExecute.Parameters.Remove(propertyName); - } + propertyValue = propertyValueObject.ToString()!; } } From 8276506d57a13412b2b986260510ecddf1850cb8 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Fri, 28 Mar 2025 02:04:46 -0400 Subject: [PATCH 2/3] Avoid silent failures in AOT when type is missing --- dotnet/src/webdriver/Command.cs | 29 +++++++++++++++++++++-------- dotnet/src/webdriver/WebDriver.cs | 2 +- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/dotnet/src/webdriver/Command.cs b/dotnet/src/webdriver/Command.cs index 9bcdccd0157a2..c09bf5e604af1 100644 --- a/dotnet/src/webdriver/Command.cs +++ b/dotnet/src/webdriver/Command.cs @@ -38,13 +38,20 @@ private static class JsonOptionsHolder { public readonly static JsonSerializerOptions JsonSerializerOptions = new() { - TypeInfoResolverChain = - { - CommandJsonSerializerContext.Default, - new DefaultJsonTypeInfoResolver() - }, + TypeInfoResolver = GetTypeInfoResolver(), Converters = { new ResponseValueJsonConverter() } }; + + private static IJsonTypeInfoResolver GetTypeInfoResolver() + { +#if NET8_0_OR_GREATER + if (!System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported) + { + return CommandJsonSerializerContext.Default; + } +#endif + return JsonTypeInfoResolver.Combine(CommandJsonSerializerContext.Default, new DefaultJsonTypeInfoResolver()); + } } private readonly Dictionary _parameters; @@ -68,8 +75,6 @@ public Command(string name, string jsonParameters) /// Name of the command /// Parameters for that command /// If is . - [RequiresUnreferencedCode("Adding untyped parameter values for JSON serialization has best-effort AOT support. Ensure only Selenium types and well-known .NET types are added, or use the overload that takes pre-serialized string jsonParameters for guaranteed AOT compatibility.")] - [RequiresDynamicCode("Adding untyped parameter values for JSON serialization has best-effort AOT support. Ensure only Selenium types and well-known .NET types are added, or use the overload that takes pre-serialized string jsonParameters for guaranteed AOT compatibility.")] public Command(SessionId? sessionId, string name, Dictionary? parameters) { this.SessionId = sessionId; @@ -111,7 +116,14 @@ public string ParametersAsJsonString { if (HasParameters()) { - return JsonSerializer.Serialize(this._parameters, JsonOptionsHolder.JsonSerializerOptions); + try + { + return JsonSerializer.Serialize(this._parameters, JsonOptionsHolder.JsonSerializerOptions); + } + catch (NotSupportedException ex) + { + throw new WebDriverException("Attempted to serialize an unsupported type. Ensure you are using Selenium types, or well-known .NET types such as Dictionary and object[]", ex); + } } else { @@ -206,6 +218,7 @@ public override string ToString() [JsonSerializable(typeof(Dictionary))] [JsonSerializable(typeof(Dictionary))] [JsonSerializable(typeof(Dictionary))] + [JsonSerializable(typeof(object[]))] [JsonSourceGenerationOptions(Converters = [typeof(ResponseValueJsonConverter)])] internal partial class CommandJsonSerializerContext : JsonSerializerContext; } diff --git a/dotnet/src/webdriver/WebDriver.cs b/dotnet/src/webdriver/WebDriver.cs index c9bfdd4c93f04..d62f08ce278af 100644 --- a/dotnet/src/webdriver/WebDriver.cs +++ b/dotnet/src/webdriver/WebDriver.cs @@ -40,7 +40,7 @@ public class WebDriver : IWebDriver, ISearchContext, IJavaScriptExecutor, IFinds /// protected static readonly TimeSpan DefaultCommandTimeout = TimeSpan.FromSeconds(60); private IFileDetector fileDetector = new DefaultFileDetector(); - private NetworkManager network; + private NetworkManager? network; private WebElementFactory elementFactory; private readonly List registeredCommands = new List(); From 5c6b6ab4047f988639e630bd82c24302edc2ba78 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Fri, 28 Mar 2025 02:25:13 -0400 Subject: [PATCH 3/3] minimize diffs --- dotnet/src/webdriver/Command.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dotnet/src/webdriver/Command.cs b/dotnet/src/webdriver/Command.cs index c09bf5e604af1..c7b863e8bff1c 100644 --- a/dotnet/src/webdriver/Command.cs +++ b/dotnet/src/webdriver/Command.cs @@ -32,11 +32,11 @@ namespace OpenQA.Selenium /// public class Command { - [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = $"All trimming-unsafe access points to {nameof(JsonSerializerOptions)} are annotated as such")] - [UnconditionalSuppressMessage("AOT", "IL3050", Justification = $"All AOT-unsafe access points to {nameof(JsonSerializerOptions)} are annotated as such")] + [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = $"All trimming-unsafe access points to {nameof(s_jsonSerializerOptions)} are annotated as such")] + [UnconditionalSuppressMessage("AOT", "IL3050", Justification = $"All AOT-unsafe access points to {nameof(s_jsonSerializerOptions)} are annotated as such")] private static class JsonOptionsHolder { - public readonly static JsonSerializerOptions JsonSerializerOptions = new() + public readonly static JsonSerializerOptions s_jsonSerializerOptions = new() { TypeInfoResolver = GetTypeInfoResolver(), Converters = { new ResponseValueJsonConverter() } @@ -118,7 +118,7 @@ public string ParametersAsJsonString { try { - return JsonSerializer.Serialize(this._parameters, JsonOptionsHolder.JsonSerializerOptions); + return JsonSerializer.Serialize(this._parameters, JsonOptionsHolder.s_jsonSerializerOptions); } catch (NotSupportedException ex) {