From eb08d546fb3609a9367d2d8fa3f474068a89b16f Mon Sep 17 00:00:00 2001 From: Michael Render Date: Mon, 11 Nov 2024 01:33:05 -0500 Subject: [PATCH 1/7] [dotnet] Make `FirefoxProfile` AOT-safe --- .../src/webdriver/Firefox/FirefoxProfile.cs | 18 ++--- dotnet/src/webdriver/Firefox/Preferences.cs | 66 +++++++++++-------- 2 files changed, 46 insertions(+), 38 deletions(-) diff --git a/dotnet/src/webdriver/Firefox/FirefoxProfile.cs b/dotnet/src/webdriver/Firefox/FirefoxProfile.cs index 2e0036a8f2819..d030ed4357115 100644 --- a/dotnet/src/webdriver/Firefox/FirefoxProfile.cs +++ b/dotnet/src/webdriver/Firefox/FirefoxProfile.cs @@ -22,7 +22,7 @@ using System.Collections.Generic; using System.IO; using System.IO.Compression; -using System.Text.Json; +using System.Text.Json.Nodes; namespace OpenQA.Selenium.Firefox { @@ -296,22 +296,16 @@ private void UpdateUserPreferences() private void ReadDefaultPreferences() { - var jsonSerializerOptions = new JsonSerializerOptions - { - Converters = - { - new ResponseValueJsonConverter() - } - }; - using (Stream defaultPrefsStream = ResourceUtilities.GetResourceStream("webdriver_prefs.json", "webdriver_prefs.json")) { using (StreamReader reader = new StreamReader(defaultPrefsStream)) { string defaultPreferences = reader.ReadToEnd(); - Dictionary deserializedPreferences = JsonSerializer.Deserialize>(defaultPreferences, jsonSerializerOptions); - Dictionary immutableDefaultPreferences = deserializedPreferences["frozen"] as Dictionary; - Dictionary editableDefaultPreferences = deserializedPreferences["mutable"] as Dictionary; + JsonObject deserializedPreferences = JsonNode.Parse(defaultPreferences) as JsonObject + ?? throw new WebDriverException("webdriver_prefs.json was not a JSON object"); + + JsonObject immutableDefaultPreferences = deserializedPreferences["frozen"] as JsonObject; + JsonObject editableDefaultPreferences = deserializedPreferences["mutable"] as JsonObject; this.profilePreferences = new Preferences(immutableDefaultPreferences, editableDefaultPreferences); } } diff --git a/dotnet/src/webdriver/Firefox/Preferences.cs b/dotnet/src/webdriver/Firefox/Preferences.cs index fa767ca5a3589..013061d7afbcd 100644 --- a/dotnet/src/webdriver/Firefox/Preferences.cs +++ b/dotnet/src/webdriver/Firefox/Preferences.cs @@ -21,6 +21,9 @@ using System.Collections.Generic; using System.Globalization; using System.IO; +using System.Text.Json.Nodes; + +#nullable enable namespace OpenQA.Selenium.Firefox { @@ -29,28 +32,28 @@ namespace OpenQA.Selenium.Firefox /// internal class Preferences { - private Dictionary preferences = new Dictionary(); - private Dictionary immutablePreferences = new Dictionary(); + private readonly Dictionary preferences = new Dictionary(); + private readonly HashSet immutablePreferences = new HashSet(); /// /// Initializes a new instance of the class. /// /// A set of preferences that cannot be modified once set. /// A set of default preferences. - public Preferences(Dictionary defaultImmutablePreferences, Dictionary defaultPreferences) + public Preferences(JsonObject? defaultImmutablePreferences, JsonObject? defaultPreferences) { if (defaultImmutablePreferences != null) { - foreach (KeyValuePair pref in defaultImmutablePreferences) + foreach (KeyValuePair pref in defaultImmutablePreferences) { this.SetPreferenceValue(pref.Key, pref.Value); - this.immutablePreferences.Add(pref.Key, pref.Value.ToString()); + this.immutablePreferences.Add(pref.Key); } } if (defaultPreferences != null) { - foreach (KeyValuePair pref in defaultPreferences) + foreach (KeyValuePair pref in defaultPreferences) { this.SetPreferenceValue(pref.Key, pref.Value); } @@ -64,6 +67,7 @@ public Preferences(Dictionary defaultImmutablePreferences, Dicti /// A value give the preference. /// If the preference already exists in the currently-set list of preferences, /// the value will be updated. + /// If is null. internal void SetPreference(string key, string value) { this.SetPreferenceValue(key, value); @@ -76,6 +80,7 @@ internal void SetPreference(string key, string value) /// A value give the preference. /// If the preference already exists in the currently-set list of preferences, /// the value will be updated. + /// If is null. internal void SetPreference(string key, int value) { this.SetPreferenceValue(key, value); @@ -88,6 +93,7 @@ internal void SetPreference(string key, int value) /// A value give the preference. /// If the preference already exists in the currently-set list of preferences, /// the value will be updated. + /// If is null. internal void SetPreference(string key, bool value) { this.SetPreferenceValue(key, value); @@ -98,6 +104,7 @@ internal void SetPreference(string key, bool value) /// /// The name of the preference to retrieve. /// The value of the preference, or an empty string if the preference is not set. + /// If is null. internal string GetPreference(string preferenceName) { if (this.preferences.ContainsKey(preferenceName)) @@ -153,39 +160,46 @@ private static bool IsWrappedAsString(string value) private bool IsSettablePreference(string preferenceName) { - return !this.immutablePreferences.ContainsKey(preferenceName); + return !this.immutablePreferences.Contains(preferenceName); } - private void SetPreferenceValue(string key, object value) + private void SetPreferenceValue(string key, JsonNode? value) { if (!this.IsSettablePreference(key)) { - string message = string.Format(CultureInfo.InvariantCulture, "Preference {0} may not be overridden: frozen value={1}, requested value={2}", key, this.immutablePreferences[key], value.ToString()); + string message = string.Format(CultureInfo.InvariantCulture, "Preference {0} may not be overridden: frozen value={1}, requested value={2}", key, this.preferences[key], value?.ToString()); throw new ArgumentException(message); } - string stringValue = value as string; - if (stringValue != null) + if (value is JsonValue jValue) { - if (IsWrappedAsString(stringValue)) + if (jValue.TryGetValue(out string? stringValue)) { - throw new ArgumentException(string.Format(CultureInfo.InvariantCulture, "Preference values must be plain strings: {0}: {1}", key, value)); - } + if (IsWrappedAsString(stringValue)) + { + throw new ArgumentException(string.Format(CultureInfo.InvariantCulture, "Preference values must be plain strings: {0}: {1}", key, value)); + } - this.preferences[key] = string.Format(CultureInfo.InvariantCulture, "\"{0}\"", value); - return; - } + this.preferences[key] = string.Format(CultureInfo.InvariantCulture, "\"{0}\"", value); + return; + } - if (value is bool) - { - this.preferences[key] = Convert.ToBoolean(value, CultureInfo.InvariantCulture).ToString().ToLowerInvariant(); - return; - } + if (jValue.TryGetValue(out bool boolValue)) + { + this.preferences[key] = boolValue ? "true" : "false"; + return; + } - if (value is int || value is long) - { - this.preferences[key] = Convert.ToInt32(value, CultureInfo.InvariantCulture).ToString(CultureInfo.InvariantCulture); - return; + if (jValue.TryGetValue(out int intValue)) + { + this.preferences[key] = intValue.ToString(CultureInfo.InvariantCulture); + return; + } + if (jValue.TryGetValue(out long longValue)) + { + this.preferences[key] = longValue.ToString(CultureInfo.InvariantCulture); + return; + } } throw new WebDriverException("Value must be string, int or boolean"); From 7e07b58fd423c6c5addae0d59c6a53533f10ec48 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Mon, 11 Nov 2024 14:51:56 -0500 Subject: [PATCH 2/7] Use `see` tag for null in XML comments --- dotnet/src/webdriver/Firefox/Preferences.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dotnet/src/webdriver/Firefox/Preferences.cs b/dotnet/src/webdriver/Firefox/Preferences.cs index 013061d7afbcd..dc280daff7a2b 100644 --- a/dotnet/src/webdriver/Firefox/Preferences.cs +++ b/dotnet/src/webdriver/Firefox/Preferences.cs @@ -67,7 +67,7 @@ public Preferences(JsonObject? defaultImmutablePreferences, JsonObject? defaultP /// A value give the preference. /// If the preference already exists in the currently-set list of preferences, /// the value will be updated. - /// If is null. + /// If is . internal void SetPreference(string key, string value) { this.SetPreferenceValue(key, value); @@ -80,7 +80,7 @@ internal void SetPreference(string key, string value) /// A value give the preference. /// If the preference already exists in the currently-set list of preferences, /// the value will be updated. - /// If is null. + /// If is . internal void SetPreference(string key, int value) { this.SetPreferenceValue(key, value); @@ -93,7 +93,7 @@ internal void SetPreference(string key, int value) /// A value give the preference. /// If the preference already exists in the currently-set list of preferences, /// the value will be updated. - /// If is null. + /// If is . internal void SetPreference(string key, bool value) { this.SetPreferenceValue(key, value); @@ -104,7 +104,7 @@ internal void SetPreference(string key, bool value) /// /// The name of the preference to retrieve. /// The value of the preference, or an empty string if the preference is not set. - /// If is null. + /// If is . internal string GetPreference(string preferenceName) { if (this.preferences.ContainsKey(preferenceName)) From a4df272dbfee3f73d60bab8917bbd280183b09d9 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Mon, 11 Nov 2024 15:23:57 -0500 Subject: [PATCH 3/7] Allow decimal values in firefox preferences --- dotnet/src/webdriver/Firefox/Preferences.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dotnet/src/webdriver/Firefox/Preferences.cs b/dotnet/src/webdriver/Firefox/Preferences.cs index dc280daff7a2b..e0b7d5ac5d30b 100644 --- a/dotnet/src/webdriver/Firefox/Preferences.cs +++ b/dotnet/src/webdriver/Firefox/Preferences.cs @@ -200,6 +200,11 @@ private void SetPreferenceValue(string key, JsonNode? value) this.preferences[key] = longValue.ToString(CultureInfo.InvariantCulture); return; } + if (jValue.TryGetValue(out double doubleValue)) + { + this.preferences[key] = doubleValue.ToString(CultureInfo.InvariantCulture); + return; + } } throw new WebDriverException("Value must be string, int or boolean"); From c0905d0f6bfe5db02c0d835786d59135fc7565f1 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Tue, 12 Nov 2024 13:56:30 -0500 Subject: [PATCH 4/7] Update dotnet/src/webdriver/Firefox/Preferences.cs --- dotnet/src/webdriver/Firefox/Preferences.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotnet/src/webdriver/Firefox/Preferences.cs b/dotnet/src/webdriver/Firefox/Preferences.cs index e0b7d5ac5d30b..729bf5c8b0113 100644 --- a/dotnet/src/webdriver/Firefox/Preferences.cs +++ b/dotnet/src/webdriver/Firefox/Preferences.cs @@ -207,7 +207,7 @@ private void SetPreferenceValue(string key, JsonNode? value) } } - throw new WebDriverException("Value must be string, int or boolean"); + throw new WebDriverException("Value must be string, number or boolean"); } } } From 71aea337df31534ab030d90847f36db7a2f2d355 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Tue, 12 Nov 2024 23:52:16 -0500 Subject: [PATCH 5/7] Remove argument validation for internal `webdriver_prefs.json`, simplify --- dotnet/src/webdriver/Firefox/Preferences.cs | 101 ++++++++++---------- 1 file changed, 50 insertions(+), 51 deletions(-) diff --git a/dotnet/src/webdriver/Firefox/Preferences.cs b/dotnet/src/webdriver/Firefox/Preferences.cs index 729bf5c8b0113..8224f620c2e40 100644 --- a/dotnet/src/webdriver/Firefox/Preferences.cs +++ b/dotnet/src/webdriver/Firefox/Preferences.cs @@ -46,7 +46,8 @@ public Preferences(JsonObject? defaultImmutablePreferences, JsonObject? defaultP { foreach (KeyValuePair pref in defaultImmutablePreferences) { - this.SetPreferenceValue(pref.Key, pref.Value); + this.ThrowIfPreferenceIsImmutable(pref.Key, pref.Value); + this.preferences[pref.Key] = pref.Value!.ToJsonString(); this.immutablePreferences.Add(pref.Key); } } @@ -55,7 +56,8 @@ public Preferences(JsonObject? defaultImmutablePreferences, JsonObject? defaultP { foreach (KeyValuePair pref in defaultPreferences) { - this.SetPreferenceValue(pref.Key, pref.Value); + this.ThrowIfPreferenceIsImmutable(pref.Key, pref.Value); + this.preferences[pref.Key] = pref.Value!.ToJsonString(); } } } @@ -67,10 +69,31 @@ public Preferences(JsonObject? defaultImmutablePreferences, JsonObject? defaultP /// A value give the preference. /// If the preference already exists in the currently-set list of preferences, /// the value will be updated. - /// If is . + /// If or are . + /// + /// If is wrapped with double-quotes. + /// -or- + /// If the specified preference is immutable. + /// internal void SetPreference(string key, string value) { - this.SetPreferenceValue(key, value); + if (key is null) + { + throw new ArgumentNullException(nameof(key)); + } + + if (value is null) + { + throw new ArgumentNullException(nameof(value)); + } + + if (IsWrappedAsString(value)) + { + throw new ArgumentException(string.Format(CultureInfo.InvariantCulture, "Preference values must be plain strings: {0}: {1}", key, value)); + } + + this.ThrowIfPreferenceIsImmutable(key, value); + this.preferences[key] = string.Format(CultureInfo.InvariantCulture, "\"{0}\"", value); } /// @@ -81,9 +104,16 @@ internal void SetPreference(string key, string value) /// If the preference already exists in the currently-set list of preferences, /// the value will be updated. /// If is . + /// If the specified preference is immutable. internal void SetPreference(string key, int value) { - this.SetPreferenceValue(key, value); + if (key is null) + { + throw new ArgumentNullException(nameof(key)); + } + + this.ThrowIfPreferenceIsImmutable(key, value); + this.preferences[key] = value.ToString(CultureInfo.InvariantCulture); } /// @@ -94,9 +124,16 @@ internal void SetPreference(string key, int value) /// If the preference already exists in the currently-set list of preferences, /// the value will be updated. /// If is . + /// If the specified preference is immutable. internal void SetPreference(string key, bool value) { - this.SetPreferenceValue(key, value); + if (key is null) + { + throw new ArgumentNullException(nameof(key)); + } + + this.ThrowIfPreferenceIsImmutable(key, value); + this.preferences[key] = value ? "true" : "false"; } /// @@ -158,56 +195,18 @@ private static bool IsWrappedAsString(string value) return value.StartsWith("\"", StringComparison.OrdinalIgnoreCase) && value.EndsWith("\"", StringComparison.OrdinalIgnoreCase); } - private bool IsSettablePreference(string preferenceName) - { - return !this.immutablePreferences.Contains(preferenceName); - } - - private void SetPreferenceValue(string key, JsonNode? value) + private void ThrowIfPreferenceIsImmutable(string preferenceName, TValue value) { - if (!this.IsSettablePreference(key)) + if (this.immutablePreferences.Contains(preferenceName)) { - string message = string.Format(CultureInfo.InvariantCulture, "Preference {0} may not be overridden: frozen value={1}, requested value={2}", key, this.preferences[key], value?.ToString()); + string message = string.Format(CultureInfo.InvariantCulture, "Preference {0} may not be overridden: frozen value={1}, requested value={2}", preferenceName, this.preferences[preferenceName], value?.ToString()); throw new ArgumentException(message); } + } - if (value is JsonValue jValue) - { - if (jValue.TryGetValue(out string? stringValue)) - { - if (IsWrappedAsString(stringValue)) - { - throw new ArgumentException(string.Format(CultureInfo.InvariantCulture, "Preference values must be plain strings: {0}: {1}", key, value)); - } - - this.preferences[key] = string.Format(CultureInfo.InvariantCulture, "\"{0}\"", value); - return; - } - - if (jValue.TryGetValue(out bool boolValue)) - { - this.preferences[key] = boolValue ? "true" : "false"; - return; - } - - if (jValue.TryGetValue(out int intValue)) - { - this.preferences[key] = intValue.ToString(CultureInfo.InvariantCulture); - return; - } - if (jValue.TryGetValue(out long longValue)) - { - this.preferences[key] = longValue.ToString(CultureInfo.InvariantCulture); - return; - } - if (jValue.TryGetValue(out double doubleValue)) - { - this.preferences[key] = doubleValue.ToString(CultureInfo.InvariantCulture); - return; - } - } - - throw new WebDriverException("Value must be string, number or boolean"); + private bool IsSettablePreference(string preferenceName) + { + return !this.immutablePreferences.Contains(preferenceName); } } } From 0bd3e38512a3fe6fee4a85d483a8d4c45dbfbff2 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Wed, 13 Nov 2024 16:35:29 -0500 Subject: [PATCH 6/7] Read firefox preferences using `JsonDocument` instead of Nodes --- .../src/webdriver/Firefox/FirefoxProfile.cs | 16 +++++-------- dotnet/src/webdriver/Firefox/Preferences.cs | 24 +++++++------------ 2 files changed, 15 insertions(+), 25 deletions(-) diff --git a/dotnet/src/webdriver/Firefox/FirefoxProfile.cs b/dotnet/src/webdriver/Firefox/FirefoxProfile.cs index d030ed4357115..1246d430d6c57 100644 --- a/dotnet/src/webdriver/Firefox/FirefoxProfile.cs +++ b/dotnet/src/webdriver/Firefox/FirefoxProfile.cs @@ -22,7 +22,7 @@ using System.Collections.Generic; using System.IO; using System.IO.Compression; -using System.Text.Json.Nodes; +using System.Text.Json; namespace OpenQA.Selenium.Firefox { @@ -298,16 +298,12 @@ private void ReadDefaultPreferences() { using (Stream defaultPrefsStream = ResourceUtilities.GetResourceStream("webdriver_prefs.json", "webdriver_prefs.json")) { - using (StreamReader reader = new StreamReader(defaultPrefsStream)) - { - string defaultPreferences = reader.ReadToEnd(); - JsonObject deserializedPreferences = JsonNode.Parse(defaultPreferences) as JsonObject - ?? throw new WebDriverException("webdriver_prefs.json was not a JSON object"); + JsonDocument defaultPreferences = JsonDocument.Parse(defaultPrefsStream); - JsonObject immutableDefaultPreferences = deserializedPreferences["frozen"] as JsonObject; - JsonObject editableDefaultPreferences = deserializedPreferences["mutable"] as JsonObject; - this.profilePreferences = new Preferences(immutableDefaultPreferences, editableDefaultPreferences); - } + JsonElement immutableDefaultPreferences = defaultPreferences.RootElement.GetProperty("frozen"); + JsonElement editableDefaultPreferences = defaultPreferences.RootElement.GetProperty("mutable"); + + this.profilePreferences = new Preferences(immutableDefaultPreferences, editableDefaultPreferences); } } diff --git a/dotnet/src/webdriver/Firefox/Preferences.cs b/dotnet/src/webdriver/Firefox/Preferences.cs index 8224f620c2e40..708d3a201c045 100644 --- a/dotnet/src/webdriver/Firefox/Preferences.cs +++ b/dotnet/src/webdriver/Firefox/Preferences.cs @@ -21,7 +21,7 @@ using System.Collections.Generic; using System.Globalization; using System.IO; -using System.Text.Json.Nodes; +using System.Text.Json; #nullable enable @@ -40,25 +40,19 @@ internal class Preferences /// /// A set of preferences that cannot be modified once set. /// A set of default preferences. - public Preferences(JsonObject? defaultImmutablePreferences, JsonObject? defaultPreferences) + public Preferences(JsonElement defaultImmutablePreferences, JsonElement defaultPreferences) { - if (defaultImmutablePreferences != null) + foreach (JsonProperty pref in defaultImmutablePreferences.EnumerateObject()) { - foreach (KeyValuePair pref in defaultImmutablePreferences) - { - this.ThrowIfPreferenceIsImmutable(pref.Key, pref.Value); - this.preferences[pref.Key] = pref.Value!.ToJsonString(); - this.immutablePreferences.Add(pref.Key); - } + this.ThrowIfPreferenceIsImmutable(pref.Name, pref.Value); + this.preferences[pref.Name] = pref.Value.GetRawText(); + this.immutablePreferences.Add(pref.Name); } - if (defaultPreferences != null) + foreach (JsonProperty pref in defaultPreferences.EnumerateObject()) { - foreach (KeyValuePair pref in defaultPreferences) - { - this.ThrowIfPreferenceIsImmutable(pref.Key, pref.Value); - this.preferences[pref.Key] = pref.Value!.ToJsonString(); - } + this.ThrowIfPreferenceIsImmutable(pref.Name, pref.Value); + this.preferences[pref.Name] = pref.Value.GetRawText(); } } From d501714d83591ec601844b86cd624edfdabdfd98 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Wed, 13 Nov 2024 23:48:03 -0500 Subject: [PATCH 7/7] Dispose of `JsonDocument` --- dotnet/src/webdriver/Firefox/FirefoxProfile.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotnet/src/webdriver/Firefox/FirefoxProfile.cs b/dotnet/src/webdriver/Firefox/FirefoxProfile.cs index 1246d430d6c57..cf340826db916 100644 --- a/dotnet/src/webdriver/Firefox/FirefoxProfile.cs +++ b/dotnet/src/webdriver/Firefox/FirefoxProfile.cs @@ -298,7 +298,7 @@ private void ReadDefaultPreferences() { using (Stream defaultPrefsStream = ResourceUtilities.GetResourceStream("webdriver_prefs.json", "webdriver_prefs.json")) { - JsonDocument defaultPreferences = JsonDocument.Parse(defaultPrefsStream); + using JsonDocument defaultPreferences = JsonDocument.Parse(defaultPrefsStream); JsonElement immutableDefaultPreferences = defaultPreferences.RootElement.GetProperty("frozen"); JsonElement editableDefaultPreferences = defaultPreferences.RootElement.GetProperty("mutable");