From d0f063ae0690bfbef0f70b33a2b2352259bf2bca Mon Sep 17 00:00:00 2001 From: Michael Render Date: Sat, 28 Dec 2024 16:46:05 -0500 Subject: [PATCH 01/18] [dotnet] Avoid exceptions in cdp logging on file upload --- dotnet/src/webdriver/DevTools/DevToolsSession.cs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/dotnet/src/webdriver/DevTools/DevToolsSession.cs b/dotnet/src/webdriver/DevTools/DevToolsSession.cs index 4ec62289873db..968a564880cd1 100644 --- a/dotnet/src/webdriver/DevTools/DevToolsSession.cs +++ b/dotnet/src/webdriver/DevTools/DevToolsSession.cs @@ -600,7 +600,19 @@ private void ProcessMessage(string message) var methodParts = method.Split(new char[] { '.' }, 2); var eventData = messageObject["params"]; - LogTrace("Recieved Event {0}: {1}", method, eventData.ToString()); + if (eventData.AsObject().TryGetPropertyValue("request", out var requestNode) + && requestNode.AsObject().ContainsKey("postData")) + { + var loggableEventData = eventData.DeepClone(); + var loggableRequest = loggableEventData["request"]!; + loggableRequest["postData"] = "*RAW POST DATA REMOVED FROM LOGS*"; + loggableRequest["postDataEntries"] = new JsonArray(); + LogTrace("Recieved Event {0}: {1}", method, loggableEventData.ToString()); + } + else + { + LogTrace("Recieved Event {0}: {1}", method, eventData.ToString()); + } // Dispatch the event on a new thread so that any event handlers // responding to the event will not block this thread from processing From cce9b9428c6cfe79d0c37f6d64cfbfed4054bcd8 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Tue, 7 Jan 2025 21:55:29 -0500 Subject: [PATCH 02/18] Revert "[java] JSpecify annotations for wrappers (#14396)" This reverts commit 045ce40464dd9020c26b66519cbb44d69a222755. --- java/src/org/openqa/selenium/WrapsDriver.java | 3 --- java/src/org/openqa/selenium/WrapsElement.java | 3 --- 2 files changed, 6 deletions(-) diff --git a/java/src/org/openqa/selenium/WrapsDriver.java b/java/src/org/openqa/selenium/WrapsDriver.java index 683c4de6a050f..0817c8053a13a 100644 --- a/java/src/org/openqa/selenium/WrapsDriver.java +++ b/java/src/org/openqa/selenium/WrapsDriver.java @@ -17,13 +17,10 @@ package org.openqa.selenium; -import org.jspecify.annotations.NullMarked; - /** * This interface indicates that the implementing class knows about the driver that contains it and * can export it. */ -@NullMarked @FunctionalInterface public interface WrapsDriver { /** diff --git a/java/src/org/openqa/selenium/WrapsElement.java b/java/src/org/openqa/selenium/WrapsElement.java index fccbc67bb8f8b..89dccf3497723 100644 --- a/java/src/org/openqa/selenium/WrapsElement.java +++ b/java/src/org/openqa/selenium/WrapsElement.java @@ -17,10 +17,7 @@ package org.openqa.selenium; -import org.jspecify.annotations.NullMarked; - /** Indicates that there is an underlying element that can be used */ -@NullMarked @FunctionalInterface public interface WrapsElement { WebElement getWrappedElement(); From 5ca2f54555398ea0c352be0d8928640821d6a2ed Mon Sep 17 00:00:00 2001 From: Michael Render Date: Tue, 7 Jan 2025 21:56:08 -0500 Subject: [PATCH 03/18] Accept invalid UTF16 strings in DevTools JSON responses --- dotnet/src/webdriver/cdp/README.md | 4 +-- .../Templates/DevToolsSessionDomains.hbs | 33 +++++++++++++++++++ .../src/generator/Templates/domain.hbs | 2 +- 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/dotnet/src/webdriver/cdp/README.md b/dotnet/src/webdriver/cdp/README.md index f78d0b3d04ba4..20c23edd23a22 100644 --- a/dotnet/src/webdriver/cdp/README.md +++ b/dotnet/src/webdriver/cdp/README.md @@ -17,7 +17,7 @@ add an entry for version `` to the `SupportedDevToolsVersions` dictionary ini 6. In [`//dotnet/src/webdriver:WebDriver.csproj.prebuild.cmd`](https://github.com/SeleniumHQ/selenium/blob/trunk/dotnet/src/webdriver/WebDriver.csproj.prebuild.cmd), add the following block (substituting the proper value for ``): -``` +```bash if not exist "%1..\..\..\bazel-bin\dotnet\src\webdriver\cdp\v\DevToolsSessionDomains.cs" ( echo Generating CDP code for version pushd "%1..\..\.." @@ -29,7 +29,7 @@ if not exist "%1..\..\..\bazel-bin\dotnet\src\webdriver\cdp\v\DevToolsSessio 7. In [`//dotnet/src/webdriver:WebDriver.csproj.prebuild.sh`](https://github.com/SeleniumHQ/selenium/blob/trunk/dotnet/src/webdriver/WebDriver.csproj.prebuild.sh), add the following block (substituting the proper value for ``): -``` +```bash if [[ ! -f "$1../../../bazel-bin/dotnet/src/webdriver/cdp/v/DevToolsSessionDomains.cs" ]] then echo "Generating CDP code for version " diff --git a/third_party/dotnet/devtools/src/generator/Templates/DevToolsSessionDomains.hbs b/third_party/dotnet/devtools/src/generator/Templates/DevToolsSessionDomains.hbs index fc88ce6ec2241..d39d83b83cc2c 100644 --- a/third_party/dotnet/devtools/src/generator/Templates/DevToolsSessionDomains.hbs +++ b/third_party/dotnet/devtools/src/generator/Templates/DevToolsSessionDomains.hbs @@ -12,6 +12,15 @@ namespace {{rootNamespace}} private Lazy<{{dehumanize Name}}.{{dehumanize Name}}Adapter> m_{{dehumanize Name}}; {{/each}} + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] // Generated code use only + internal static global::System.Text.Json.JsonSerializerOptions DevToolsSerializerOptions { get; } = new global::System.Text.Json.JsonSerializerOptions() + { + Converters = + { + new InvalidUtf16Converter(), + } + }; + /// /// Initializes a new instance of the DevToolsSessionDomains class. /// @@ -42,5 +51,29 @@ namespace {{rootNamespace}} ResponseTypeMap.AddCommandResponseType(typeof({{FullTypeName}}), typeof({{FullResponseTypeName}})); {{/each}} } + + private sealed class InvalidUtf16Converter : global::System.Text.Json.Serialization.JsonConverter + { + public override bool HandleNull => true; + + public override string? Read(ref global::System.Text.Json.Utf8JsonReader reader, global::System.Type typeToConvert, global::System.Text.Json.JsonSerializerOptions options) + { + try + { + return reader.GetString(); + } + catch (global::System.InvalidOperationException) + { + var bytes = reader.ValueSpan; + var sb = new global::System.Text.StringBuilder(bytes.Length); + foreach (var b in bytes) + sb.Append(global::System.Convert.ToChar(b)); + return sb.ToString(); + } + } + + public override void Write(global::System.Text.Json.Utf8JsonWriter writer, string value, global::System.Text.Json.JsonSerializerOptions options) => + writer.WriteStringValue(value); + } } } diff --git a/third_party/dotnet/devtools/src/generator/Templates/domain.hbs b/third_party/dotnet/devtools/src/generator/Templates/domain.hbs index 1fc6271239b39..b0b653662d8a8 100644 --- a/third_party/dotnet/devtools/src/generator/Templates/domain.hbs +++ b/third_party/dotnet/devtools/src/generator/Templates/domain.hbs @@ -61,7 +61,7 @@ namespace {{rootNamespace}}.{{domain.Name}} if (m_eventMap.ContainsKey(e.EventName)) { var eventData = m_eventMap[e.EventName]; - var eventArgs = e.EventData.Deserialize(eventData.EventArgsType); + var eventArgs = e.EventData.Deserialize(eventData.EventArgsType, {{rootNamespace}}.DevToolsSessionDomains.DevToolsSerializerOptions); eventData.EventInvoker(eventArgs); } } From 43fc198ade448a365ec578b87cc470b2117d66b0 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Tue, 7 Jan 2025 22:00:09 -0500 Subject: [PATCH 04/18] Reapply "[java] JSpecify annotations for wrappers (#14396)" This reverts commit cce9b9428c6cfe79d0c37f6d64cfbfed4054bcd8. --- java/src/org/openqa/selenium/WrapsDriver.java | 3 +++ java/src/org/openqa/selenium/WrapsElement.java | 3 +++ 2 files changed, 6 insertions(+) diff --git a/java/src/org/openqa/selenium/WrapsDriver.java b/java/src/org/openqa/selenium/WrapsDriver.java index 0817c8053a13a..683c4de6a050f 100644 --- a/java/src/org/openqa/selenium/WrapsDriver.java +++ b/java/src/org/openqa/selenium/WrapsDriver.java @@ -17,10 +17,13 @@ package org.openqa.selenium; +import org.jspecify.annotations.NullMarked; + /** * This interface indicates that the implementing class knows about the driver that contains it and * can export it. */ +@NullMarked @FunctionalInterface public interface WrapsDriver { /** diff --git a/java/src/org/openqa/selenium/WrapsElement.java b/java/src/org/openqa/selenium/WrapsElement.java index 89dccf3497723..fccbc67bb8f8b 100644 --- a/java/src/org/openqa/selenium/WrapsElement.java +++ b/java/src/org/openqa/selenium/WrapsElement.java @@ -17,7 +17,10 @@ package org.openqa.selenium; +import org.jspecify.annotations.NullMarked; + /** Indicates that there is an underlying element that can be used */ +@NullMarked @FunctionalInterface public interface WrapsElement { WebElement getWrappedElement(); From 10104564547a7e7be4a76e84e89d8d8903a0c0c9 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Tue, 7 Jan 2025 22:00:28 -0500 Subject: [PATCH 05/18] Revert "[dotnet] Avoid exceptions in cdp logging on file upload" This reverts commit d0f063ae0690bfbef0f70b33a2b2352259bf2bca. --- dotnet/src/webdriver/DevTools/DevToolsSession.cs | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/dotnet/src/webdriver/DevTools/DevToolsSession.cs b/dotnet/src/webdriver/DevTools/DevToolsSession.cs index e190054d549e1..a11f4757776a3 100644 --- a/dotnet/src/webdriver/DevTools/DevToolsSession.cs +++ b/dotnet/src/webdriver/DevTools/DevToolsSession.cs @@ -604,19 +604,7 @@ private void ProcessMessage(string message) var methodParts = method.Split(new char[] { '.' }, 2); var eventData = messageObject.GetProperty("params"); - if (eventData.AsObject().TryGetPropertyValue("request", out var requestNode) - && requestNode.AsObject().ContainsKey("postData")) - { - var loggableEventData = eventData.DeepClone(); - var loggableRequest = loggableEventData["request"]!; - loggableRequest["postData"] = "*RAW POST DATA REMOVED FROM LOGS*"; - loggableRequest["postDataEntries"] = new JsonArray(); - LogTrace("Recieved Event {0}: {1}", method, loggableEventData.ToString()); - } - else - { - LogTrace("Recieved Event {0}: {1}", method, eventData.ToString()); - } + LogTrace("Recieved Event {0}: {1}", method, eventData.ToString()); // Dispatch the event on a new thread so that any event handlers // responding to the event will not block this thread from processing From 4bb41e90c237ce44b0797c7a07e8376278a317a1 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Tue, 7 Jan 2025 22:06:18 -0500 Subject: [PATCH 06/18] [dotnet] Enable nullability on `DevToolsSessionDomains` --- .../src/generator/Templates/DevToolsSessionDomains.hbs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/third_party/dotnet/devtools/src/generator/Templates/DevToolsSessionDomains.hbs b/third_party/dotnet/devtools/src/generator/Templates/DevToolsSessionDomains.hbs index d39d83b83cc2c..116f73dd3fade 100644 --- a/third_party/dotnet/devtools/src/generator/Templates/DevToolsSessionDomains.hbs +++ b/third_party/dotnet/devtools/src/generator/Templates/DevToolsSessionDomains.hbs @@ -1,4 +1,8 @@ // + +#nullable enable annotations +#nullable disable warnings + namespace {{rootNamespace}} { using System; From 187dcc40bffcfa3818466e72e94b731c369b5468 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Wed, 8 Jan 2025 10:49:20 -0500 Subject: [PATCH 07/18] Move DevTools JSON serialization options to base non-generated `DevToolsSessionDomains` --- .../DevTools/DevToolsSessionDomains.cs | 41 +++++++++++++++++++ dotnet/test/firefox/FirefoxDriverTest.cs | 12 ++++++ .../Templates/DevToolsSessionDomains.hbs | 36 ---------------- .../src/generator/Templates/domain.hbs | 2 +- 4 files changed, 54 insertions(+), 37 deletions(-) diff --git a/dotnet/src/webdriver/DevTools/DevToolsSessionDomains.cs b/dotnet/src/webdriver/DevTools/DevToolsSessionDomains.cs index 96d1521f92500..721b0c88e92df 100644 --- a/dotnet/src/webdriver/DevTools/DevToolsSessionDomains.cs +++ b/dotnet/src/webdriver/DevTools/DevToolsSessionDomains.cs @@ -17,6 +17,14 @@ // under the License. // +using System; +using System.ComponentModel; +using System.Text; +using System.Text.Json; +using System.Text.Json.Serialization; + +#nullable enable + namespace OpenQA.Selenium.DevTools { /// @@ -26,6 +34,15 @@ public abstract class DevToolsSessionDomains { private CommandResponseTypeMap responseTypeMap = new CommandResponseTypeMap(); + [EditorBrowsable(EditorBrowsableState.Never)] // Generated code use only + internal static JsonSerializerOptions DevToolsSerializerOptions { get; } = new JsonSerializerOptions() + { + Converters = + { + new InvalidUtf16Converter(), + } + }; + /// /// Initializes a new instance of the class. /// @@ -43,5 +60,29 @@ protected DevToolsSessionDomains() /// Populates the command response type map. /// protected abstract void PopulateCommandResponseTypeMap(); + + private sealed class InvalidUtf16Converter : JsonConverter + { + public override bool HandleNull => true; + + public override string? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + try + { + return reader.GetString(); + } + catch (InvalidOperationException) + { + var bytes = reader.ValueSpan; + var sb = new StringBuilder(bytes.Length); + foreach (var b in bytes) + sb.Append(Convert.ToChar(b)); + return sb.ToString(); + } + } + + public override void Write(Utf8JsonWriter writer, string value, JsonSerializerOptions options) => + writer.WriteStringValue(value); + } } } diff --git a/dotnet/test/firefox/FirefoxDriverTest.cs b/dotnet/test/firefox/FirefoxDriverTest.cs index 0de306815de96..3fc79668561b6 100644 --- a/dotnet/test/firefox/FirefoxDriverTest.cs +++ b/dotnet/test/firefox/FirefoxDriverTest.cs @@ -360,6 +360,18 @@ public void ShouldInstallAndUninstallUnSignedDirAddon() Assert.That(driver.FindElements(By.Id("webextensions-selenium-example")).Count, Is.Zero); } + [Test] + public void AAA() + { + driver.Manage().Network.StartMonitoring().GetAwaiter().GetResult(); // this enables serialization + driver.Navigate().GoToUrl("https://tus.io/demo"); + + Thread.Sleep(10_000); + driver.FindElement(By.Id("P0-0")).SendKeys("C:\\Users\\Owner\\Downloads\\Firefox Installer.exe"); + + Thread.Sleep(60_000); + } + private string GetPath(string name) { string sCurrentDirectory = AppDomain.CurrentDomain.BaseDirectory; diff --git a/third_party/dotnet/devtools/src/generator/Templates/DevToolsSessionDomains.hbs b/third_party/dotnet/devtools/src/generator/Templates/DevToolsSessionDomains.hbs index 116f73dd3fade..7c5c4e490c180 100644 --- a/third_party/dotnet/devtools/src/generator/Templates/DevToolsSessionDomains.hbs +++ b/third_party/dotnet/devtools/src/generator/Templates/DevToolsSessionDomains.hbs @@ -1,8 +1,5 @@ // -#nullable enable annotations -#nullable disable warnings - namespace {{rootNamespace}} { using System; @@ -16,15 +13,6 @@ namespace {{rootNamespace}} private Lazy<{{dehumanize Name}}.{{dehumanize Name}}Adapter> m_{{dehumanize Name}}; {{/each}} - [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] // Generated code use only - internal static global::System.Text.Json.JsonSerializerOptions DevToolsSerializerOptions { get; } = new global::System.Text.Json.JsonSerializerOptions() - { - Converters = - { - new InvalidUtf16Converter(), - } - }; - /// /// Initializes a new instance of the DevToolsSessionDomains class. /// @@ -55,29 +43,5 @@ namespace {{rootNamespace}} ResponseTypeMap.AddCommandResponseType(typeof({{FullTypeName}}), typeof({{FullResponseTypeName}})); {{/each}} } - - private sealed class InvalidUtf16Converter : global::System.Text.Json.Serialization.JsonConverter - { - public override bool HandleNull => true; - - public override string? Read(ref global::System.Text.Json.Utf8JsonReader reader, global::System.Type typeToConvert, global::System.Text.Json.JsonSerializerOptions options) - { - try - { - return reader.GetString(); - } - catch (global::System.InvalidOperationException) - { - var bytes = reader.ValueSpan; - var sb = new global::System.Text.StringBuilder(bytes.Length); - foreach (var b in bytes) - sb.Append(global::System.Convert.ToChar(b)); - return sb.ToString(); - } - } - - public override void Write(global::System.Text.Json.Utf8JsonWriter writer, string value, global::System.Text.Json.JsonSerializerOptions options) => - writer.WriteStringValue(value); - } } } diff --git a/third_party/dotnet/devtools/src/generator/Templates/domain.hbs b/third_party/dotnet/devtools/src/generator/Templates/domain.hbs index b0b653662d8a8..3f4fa6efadfcd 100644 --- a/third_party/dotnet/devtools/src/generator/Templates/domain.hbs +++ b/third_party/dotnet/devtools/src/generator/Templates/domain.hbs @@ -61,7 +61,7 @@ namespace {{rootNamespace}}.{{domain.Name}} if (m_eventMap.ContainsKey(e.EventName)) { var eventData = m_eventMap[e.EventName]; - var eventArgs = e.EventData.Deserialize(eventData.EventArgsType, {{rootNamespace}}.DevToolsSessionDomains.DevToolsSerializerOptions); + var eventArgs = e.EventData.Deserialize(eventData.EventArgsType, OpenQA.Selenium.DevTools.DevToolsSessionDomains.DevToolsSerializerOptions); eventData.EventInvoker(eventArgs); } } From 5a2bd357eb4e8f122b3268a80d63366308c0c02d Mon Sep 17 00:00:00 2001 From: Michael Render Date: Wed, 8 Jan 2025 10:50:00 -0500 Subject: [PATCH 08/18] remove accidental push --- dotnet/test/firefox/FirefoxDriverTest.cs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/dotnet/test/firefox/FirefoxDriverTest.cs b/dotnet/test/firefox/FirefoxDriverTest.cs index 3fc79668561b6..0de306815de96 100644 --- a/dotnet/test/firefox/FirefoxDriverTest.cs +++ b/dotnet/test/firefox/FirefoxDriverTest.cs @@ -360,18 +360,6 @@ public void ShouldInstallAndUninstallUnSignedDirAddon() Assert.That(driver.FindElements(By.Id("webextensions-selenium-example")).Count, Is.Zero); } - [Test] - public void AAA() - { - driver.Manage().Network.StartMonitoring().GetAwaiter().GetResult(); // this enables serialization - driver.Navigate().GoToUrl("https://tus.io/demo"); - - Thread.Sleep(10_000); - driver.FindElement(By.Id("P0-0")).SendKeys("C:\\Users\\Owner\\Downloads\\Firefox Installer.exe"); - - Thread.Sleep(60_000); - } - private string GetPath(string name) { string sCurrentDirectory = AppDomain.CurrentDomain.BaseDirectory; From 442b392053fdf5189ad6fb3c957c9591dd25ffd2 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Wed, 8 Jan 2025 10:50:24 -0500 Subject: [PATCH 09/18] remove unnecessary diff --- .../devtools/src/generator/Templates/DevToolsSessionDomains.hbs | 1 - 1 file changed, 1 deletion(-) diff --git a/third_party/dotnet/devtools/src/generator/Templates/DevToolsSessionDomains.hbs b/third_party/dotnet/devtools/src/generator/Templates/DevToolsSessionDomains.hbs index 7c5c4e490c180..fc88ce6ec2241 100644 --- a/third_party/dotnet/devtools/src/generator/Templates/DevToolsSessionDomains.hbs +++ b/third_party/dotnet/devtools/src/generator/Templates/DevToolsSessionDomains.hbs @@ -1,5 +1,4 @@ // - namespace {{rootNamespace}} { using System; From 15883542c0cf6433d073ce6bbb77ac58ed437b79 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Wed, 8 Jan 2025 11:03:44 -0500 Subject: [PATCH 10/18] Use explicit type instead of `var` --- dotnet/src/webdriver/DevTools/DevToolsSessionDomains.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotnet/src/webdriver/DevTools/DevToolsSessionDomains.cs b/dotnet/src/webdriver/DevTools/DevToolsSessionDomains.cs index 721b0c88e92df..d6396ceac5e9c 100644 --- a/dotnet/src/webdriver/DevTools/DevToolsSessionDomains.cs +++ b/dotnet/src/webdriver/DevTools/DevToolsSessionDomains.cs @@ -75,7 +75,7 @@ private sealed class InvalidUtf16Converter : JsonConverter { var bytes = reader.ValueSpan; var sb = new StringBuilder(bytes.Length); - foreach (var b in bytes) + foreach (byte b in bytes) sb.Append(Convert.ToChar(b)); return sb.ToString(); } From 6c098881912d8a4ff1055e57d2b3dee1a513c722 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Wed, 8 Jan 2025 13:24:09 -0500 Subject: [PATCH 11/18] PR feedback --- .../DevTools/DevToolsSessionDomains.cs | 29 +-------------- .../DevTools/Json/StringConverter.cs | 35 +++++++++++++++++++ 2 files changed, 36 insertions(+), 28 deletions(-) create mode 100644 dotnet/src/webdriver/DevTools/Json/StringConverter.cs diff --git a/dotnet/src/webdriver/DevTools/DevToolsSessionDomains.cs b/dotnet/src/webdriver/DevTools/DevToolsSessionDomains.cs index d6396ceac5e9c..ca53dab49b8c1 100644 --- a/dotnet/src/webdriver/DevTools/DevToolsSessionDomains.cs +++ b/dotnet/src/webdriver/DevTools/DevToolsSessionDomains.cs @@ -17,11 +17,8 @@ // under the License. // -using System; using System.ComponentModel; -using System.Text; using System.Text.Json; -using System.Text.Json.Serialization; #nullable enable @@ -39,7 +36,7 @@ public abstract class DevToolsSessionDomains { Converters = { - new InvalidUtf16Converter(), + new Json.StringConverter(), } }; @@ -60,29 +57,5 @@ protected DevToolsSessionDomains() /// Populates the command response type map. /// protected abstract void PopulateCommandResponseTypeMap(); - - private sealed class InvalidUtf16Converter : JsonConverter - { - public override bool HandleNull => true; - - public override string? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) - { - try - { - return reader.GetString(); - } - catch (InvalidOperationException) - { - var bytes = reader.ValueSpan; - var sb = new StringBuilder(bytes.Length); - foreach (byte b in bytes) - sb.Append(Convert.ToChar(b)); - return sb.ToString(); - } - } - - public override void Write(Utf8JsonWriter writer, string value, JsonSerializerOptions options) => - writer.WriteStringValue(value); - } } } diff --git a/dotnet/src/webdriver/DevTools/Json/StringConverter.cs b/dotnet/src/webdriver/DevTools/Json/StringConverter.cs new file mode 100644 index 0000000000000..508abf088f16b --- /dev/null +++ b/dotnet/src/webdriver/DevTools/Json/StringConverter.cs @@ -0,0 +1,35 @@ +using System; +using System.Text; +using System.Text.Json; +using System.Text.Json.Serialization; + +#nullable enable + +namespace OpenQA.Selenium.DevTools.Json; + +internal sealed class StringConverter : JsonConverter +{ + public override bool HandleNull => true; + + public override string? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + try + { + return reader.GetString(); + } + catch (InvalidOperationException) + { + // Backwards compatibility with Newtonsoft tolerating invalid UTF-16 sequences + // CDP sometimes sends invalid surrogate pairs on file upload + + var bytes = reader.ValueSpan; + var sb = new StringBuilder(bytes.Length); + foreach (byte b in bytes) + sb.Append(Convert.ToChar(b)); + return sb.ToString(); + } + } + + public override void Write(Utf8JsonWriter writer, string value, JsonSerializerOptions options) => + writer.WriteStringValue(value); +} \ No newline at end of file From 0f0ed1b9f140f16dba7b8cae052ea4ea6f8e65a8 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Wed, 8 Jan 2025 13:24:55 -0500 Subject: [PATCH 12/18] Apply suggestions from code review Co-authored-by: Nikolay Borisenko <22616990+nvborisenko@users.noreply.github.com> --- third_party/dotnet/devtools/src/generator/Templates/domain.hbs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/third_party/dotnet/devtools/src/generator/Templates/domain.hbs b/third_party/dotnet/devtools/src/generator/Templates/domain.hbs index 3f4fa6efadfcd..f92556b2b4ccf 100644 --- a/third_party/dotnet/devtools/src/generator/Templates/domain.hbs +++ b/third_party/dotnet/devtools/src/generator/Templates/domain.hbs @@ -61,7 +61,7 @@ namespace {{rootNamespace}}.{{domain.Name}} if (m_eventMap.ContainsKey(e.EventName)) { var eventData = m_eventMap[e.EventName]; - var eventArgs = e.EventData.Deserialize(eventData.EventArgsType, OpenQA.Selenium.DevTools.DevToolsSessionDomains.DevToolsSerializerOptions); + var eventArgs = e.EventData.Deserialize(eventData.EventArgsType, global::OpenQA.Selenium.DevTools.DevToolsSessionDomains.DevToolsSerializerOptions); eventData.EventInvoker(eventArgs); } } From 4ae49b45ea77a194155f528d7f9b42e08a24f62d Mon Sep 17 00:00:00 2001 From: Michael Render Date: Wed, 8 Jan 2025 13:29:28 -0500 Subject: [PATCH 13/18] Move DevTools JSON options to a separate type --- .../DevTools/DevToolsSessionDomains.cs | 14 -------------- .../DevTools/Json/DevToolsJsonOptions.cs | 18 ++++++++++++++++++ .../src/generator/Templates/domain.hbs | 2 +- 3 files changed, 19 insertions(+), 15 deletions(-) create mode 100644 dotnet/src/webdriver/DevTools/Json/DevToolsJsonOptions.cs diff --git a/dotnet/src/webdriver/DevTools/DevToolsSessionDomains.cs b/dotnet/src/webdriver/DevTools/DevToolsSessionDomains.cs index ca53dab49b8c1..96d1521f92500 100644 --- a/dotnet/src/webdriver/DevTools/DevToolsSessionDomains.cs +++ b/dotnet/src/webdriver/DevTools/DevToolsSessionDomains.cs @@ -17,11 +17,6 @@ // under the License. // -using System.ComponentModel; -using System.Text.Json; - -#nullable enable - namespace OpenQA.Selenium.DevTools { /// @@ -31,15 +26,6 @@ public abstract class DevToolsSessionDomains { private CommandResponseTypeMap responseTypeMap = new CommandResponseTypeMap(); - [EditorBrowsable(EditorBrowsableState.Never)] // Generated code use only - internal static JsonSerializerOptions DevToolsSerializerOptions { get; } = new JsonSerializerOptions() - { - Converters = - { - new Json.StringConverter(), - } - }; - /// /// Initializes a new instance of the class. /// diff --git a/dotnet/src/webdriver/DevTools/Json/DevToolsJsonOptions.cs b/dotnet/src/webdriver/DevTools/Json/DevToolsJsonOptions.cs new file mode 100644 index 0000000000000..025af4a5e280f --- /dev/null +++ b/dotnet/src/webdriver/DevTools/Json/DevToolsJsonOptions.cs @@ -0,0 +1,18 @@ +using System.ComponentModel; +using System.Text.Json; + +#nullable enable + +namespace OpenQA.Selenium.DevTools.Json; + +[EditorBrowsable(EditorBrowsableState.Never)] // Generated code use only +internal static class DevToolsJsonOptions +{ + public static JsonSerializerOptions DevToolsSerializerOptions { get; } = new JsonSerializerOptions() + { + Converters = + { + new StringConverter(), + } + }; +} diff --git a/third_party/dotnet/devtools/src/generator/Templates/domain.hbs b/third_party/dotnet/devtools/src/generator/Templates/domain.hbs index f92556b2b4ccf..6a72eb62a1fd3 100644 --- a/third_party/dotnet/devtools/src/generator/Templates/domain.hbs +++ b/third_party/dotnet/devtools/src/generator/Templates/domain.hbs @@ -61,7 +61,7 @@ namespace {{rootNamespace}}.{{domain.Name}} if (m_eventMap.ContainsKey(e.EventName)) { var eventData = m_eventMap[e.EventName]; - var eventArgs = e.EventData.Deserialize(eventData.EventArgsType, global::OpenQA.Selenium.DevTools.DevToolsSessionDomains.DevToolsSerializerOptions); + var eventArgs = e.EventData.Deserialize(eventData.EventArgsType, global::OpenQA.Selenium.DevTools.Json.DevToolsJsonOptions.DevToolsSerializerOptions); eventData.EventInvoker(eventArgs); } } From 9fb41593dedd7adb514fa9034cd9a7f4eb1ef029 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Wed, 8 Jan 2025 13:40:23 -0500 Subject: [PATCH 14/18] PR feedback --- dotnet/src/webdriver/DevTools/Json/DevToolsJsonOptions.cs | 2 -- dotnet/src/webdriver/DevTools/Json/StringConverter.cs | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/dotnet/src/webdriver/DevTools/Json/DevToolsJsonOptions.cs b/dotnet/src/webdriver/DevTools/Json/DevToolsJsonOptions.cs index 025af4a5e280f..a3526e19b403c 100644 --- a/dotnet/src/webdriver/DevTools/Json/DevToolsJsonOptions.cs +++ b/dotnet/src/webdriver/DevTools/Json/DevToolsJsonOptions.cs @@ -1,11 +1,9 @@ -using System.ComponentModel; using System.Text.Json; #nullable enable namespace OpenQA.Selenium.DevTools.Json; -[EditorBrowsable(EditorBrowsableState.Never)] // Generated code use only internal static class DevToolsJsonOptions { public static JsonSerializerOptions DevToolsSerializerOptions { get; } = new JsonSerializerOptions() diff --git a/dotnet/src/webdriver/DevTools/Json/StringConverter.cs b/dotnet/src/webdriver/DevTools/Json/StringConverter.cs index 508abf088f16b..b1eb8b1107045 100644 --- a/dotnet/src/webdriver/DevTools/Json/StringConverter.cs +++ b/dotnet/src/webdriver/DevTools/Json/StringConverter.cs @@ -25,7 +25,10 @@ internal sealed class StringConverter : JsonConverter var bytes = reader.ValueSpan; var sb = new StringBuilder(bytes.Length); foreach (byte b in bytes) + { sb.Append(Convert.ToChar(b)); + } + return sb.ToString(); } } From c3e257f8c3f913bfad7470a1147fba783e09ed0b Mon Sep 17 00:00:00 2001 From: Michael Render Date: Wed, 8 Jan 2025 13:40:53 -0500 Subject: [PATCH 15/18] PR feedback --- dotnet/src/webdriver/DevTools/Json/StringConverter.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dotnet/src/webdriver/DevTools/Json/StringConverter.cs b/dotnet/src/webdriver/DevTools/Json/StringConverter.cs index b1eb8b1107045..e48f6ed8c531e 100644 --- a/dotnet/src/webdriver/DevTools/Json/StringConverter.cs +++ b/dotnet/src/webdriver/DevTools/Json/StringConverter.cs @@ -19,7 +19,6 @@ internal sealed class StringConverter : JsonConverter } catch (InvalidOperationException) { - // Backwards compatibility with Newtonsoft tolerating invalid UTF-16 sequences // CDP sometimes sends invalid surrogate pairs on file upload var bytes = reader.ValueSpan; @@ -28,7 +27,7 @@ internal sealed class StringConverter : JsonConverter { sb.Append(Convert.ToChar(b)); } - + return sb.ToString(); } } From 4d1171462a6be0f280896b113ec0bdf40486c87b Mon Sep 17 00:00:00 2001 From: Michael Render Date: Wed, 8 Jan 2025 14:35:20 -0500 Subject: [PATCH 16/18] Change `DevToolsJsonOptions` instance to `Default` --- dotnet/src/webdriver/DevTools/Json/DevToolsJsonOptions.cs | 2 +- third_party/dotnet/devtools/src/generator/Templates/domain.hbs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dotnet/src/webdriver/DevTools/Json/DevToolsJsonOptions.cs b/dotnet/src/webdriver/DevTools/Json/DevToolsJsonOptions.cs index a3526e19b403c..4b91d4ed852c2 100644 --- a/dotnet/src/webdriver/DevTools/Json/DevToolsJsonOptions.cs +++ b/dotnet/src/webdriver/DevTools/Json/DevToolsJsonOptions.cs @@ -6,7 +6,7 @@ namespace OpenQA.Selenium.DevTools.Json; internal static class DevToolsJsonOptions { - public static JsonSerializerOptions DevToolsSerializerOptions { get; } = new JsonSerializerOptions() + public static JsonSerializerOptions Default { get; } = new JsonSerializerOptions() { Converters = { diff --git a/third_party/dotnet/devtools/src/generator/Templates/domain.hbs b/third_party/dotnet/devtools/src/generator/Templates/domain.hbs index 6a72eb62a1fd3..8dc01ba4fb8ea 100644 --- a/third_party/dotnet/devtools/src/generator/Templates/domain.hbs +++ b/third_party/dotnet/devtools/src/generator/Templates/domain.hbs @@ -61,7 +61,7 @@ namespace {{rootNamespace}}.{{domain.Name}} if (m_eventMap.ContainsKey(e.EventName)) { var eventData = m_eventMap[e.EventName]; - var eventArgs = e.EventData.Deserialize(eventData.EventArgsType, global::OpenQA.Selenium.DevTools.Json.DevToolsJsonOptions.DevToolsSerializerOptions); + var eventArgs = e.EventData.Deserialize(eventData.EventArgsType, global::OpenQA.Selenium.DevTools.Json.DevToolsJsonOptions.Default); eventData.EventInvoker(eventArgs); } } From 55f69d540585a2a978dcbf0768959da142f09f7b Mon Sep 17 00:00:00 2001 From: Michael Render Date: Wed, 8 Jan 2025 16:15:42 -0500 Subject: [PATCH 17/18] Improve comment in `StringConverter` --- dotnet/src/webdriver/DevTools/Json/StringConverter.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/dotnet/src/webdriver/DevTools/Json/StringConverter.cs b/dotnet/src/webdriver/DevTools/Json/StringConverter.cs index e48f6ed8c531e..e5d1b780fbb43 100644 --- a/dotnet/src/webdriver/DevTools/Json/StringConverter.cs +++ b/dotnet/src/webdriver/DevTools/Json/StringConverter.cs @@ -19,7 +19,12 @@ internal sealed class StringConverter : JsonConverter } catch (InvalidOperationException) { - // CDP sometimes sends invalid surrogate pairs on file upload + // Fallback to read the value as bytes instead of string. + // System.Text.Json library throws exception when CDP remote end sends non-encoded string as binary data. + // Using JavaScriptEncoder.UnsafeRelaxedJsonEscaping doesn't help because the string actually is byte[]. + // https://chromedevtools.github.io/devtools-protocol/tot/Network/#type-Request - here "postData" property + // is a string, which we cannot deserialize properly. This property is marked as deprecated, and new "postDataEntries" + // is suggested for using, where most likely it is base64 encoded. var bytes = reader.ValueSpan; var sb = new StringBuilder(bytes.Length); From f13a6d9d68802f3b727409703d66badb985d27d8 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Wed, 8 Jan 2025 16:38:53 -0500 Subject: [PATCH 18/18] Add file headers --- .../DevTools/Json/DevToolsJsonOptions.cs | 19 +++++++++++++++++ .../DevTools/Json/StringConverter.cs | 21 ++++++++++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/dotnet/src/webdriver/DevTools/Json/DevToolsJsonOptions.cs b/dotnet/src/webdriver/DevTools/Json/DevToolsJsonOptions.cs index 4b91d4ed852c2..4b624a9f6b43e 100644 --- a/dotnet/src/webdriver/DevTools/Json/DevToolsJsonOptions.cs +++ b/dotnet/src/webdriver/DevTools/Json/DevToolsJsonOptions.cs @@ -1,3 +1,22 @@ +// +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + using System.Text.Json; #nullable enable diff --git a/dotnet/src/webdriver/DevTools/Json/StringConverter.cs b/dotnet/src/webdriver/DevTools/Json/StringConverter.cs index e5d1b780fbb43..0ba9a9627fc7f 100644 --- a/dotnet/src/webdriver/DevTools/Json/StringConverter.cs +++ b/dotnet/src/webdriver/DevTools/Json/StringConverter.cs @@ -1,3 +1,22 @@ +// +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + using System; using System.Text; using System.Text.Json; @@ -39,4 +58,4 @@ internal sealed class StringConverter : JsonConverter public override void Write(Utf8JsonWriter writer, string value, JsonSerializerOptions options) => writer.WriteStringValue(value); -} \ No newline at end of file +}