Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dotnet] Tolerate invalid UTF-16 strings in DevTools JSON response #14972

Merged
merged 21 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
d0f063a
[dotnet] Avoid exceptions in cdp logging on file upload
RenderMichael Dec 28, 2024
cce9b94
Revert "[java] JSpecify annotations for wrappers (#14396)"
RenderMichael Jan 8, 2025
5ca2f54
Accept invalid UTF16 strings in DevTools JSON responses
RenderMichael Jan 8, 2025
e0679f4
Merge branch 'trunk' into devtools-file-upload
RenderMichael Jan 8, 2025
43fc198
Reapply "[java] JSpecify annotations for wrappers (#14396)"
RenderMichael Jan 8, 2025
1010456
Revert "[dotnet] Avoid exceptions in cdp logging on file upload"
RenderMichael Jan 8, 2025
4bb41e9
[dotnet] Enable nullability on `DevToolsSessionDomains`
RenderMichael Jan 8, 2025
253e135
Merge branch 'trunk' into devtools-file-upload
RenderMichael Jan 8, 2025
187dcc4
Move DevTools JSON serialization options to base non-generated `DevTo…
RenderMichael Jan 8, 2025
cb16a00
Merge branch 'devtools-file-upload' of https://github.com/RenderMicha…
RenderMichael Jan 8, 2025
5a2bd35
remove accidental push
RenderMichael Jan 8, 2025
442b392
remove unnecessary diff
RenderMichael Jan 8, 2025
1588354
Use explicit type instead of `var`
RenderMichael Jan 8, 2025
6c09888
PR feedback
RenderMichael Jan 8, 2025
0f0ed1b
Apply suggestions from code review
RenderMichael Jan 8, 2025
4ae49b4
Move DevTools JSON options to a separate type
RenderMichael Jan 8, 2025
9fb4159
PR feedback
RenderMichael Jan 8, 2025
c3e257f
PR feedback
RenderMichael Jan 8, 2025
4d11714
Change `DevToolsJsonOptions` instance to `Default`
RenderMichael Jan 8, 2025
55f69d5
Improve comment in `StringConverter`
RenderMichael Jan 8, 2025
f13a6d9
Add file headers
RenderMichael Jan 8, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions dotnet/src/webdriver/DevTools/Json/DevToolsJsonOptions.cs
Original file line number Diff line number Diff line change
@@ -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
RenderMichael marked this conversation as resolved.
Show resolved Hide resolved
internal static class DevToolsJsonOptions
{
public static JsonSerializerOptions DevToolsSerializerOptions { get; } = new JsonSerializerOptions()
nvborisenko marked this conversation as resolved.
Show resolved Hide resolved
{
Converters =
{
new StringConverter(),
}
};
}
35 changes: 35 additions & 0 deletions dotnet/src/webdriver/DevTools/Json/StringConverter.cs
Original file line number Diff line number Diff line change
@@ -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<string>
{
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
nvborisenko marked this conversation as resolved.
Show resolved Hide resolved
// 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));
RenderMichael marked this conversation as resolved.
Show resolved Hide resolved
return sb.ToString();
}
}

public override void Write(Utf8JsonWriter writer, string value, JsonSerializerOptions options) =>
writer.WriteStringValue(value);
}
4 changes: 2 additions & 2 deletions dotnet/src/webdriver/cdp/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ add an entry for version `<N>` 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 `<N>`):

```
```bash
if not exist "%1..\..\..\bazel-bin\dotnet\src\webdriver\cdp\v<N>\DevToolsSessionDomains.cs" (
echo Generating CDP code for version <N>
pushd "%1..\..\.."
Expand All @@ -29,7 +29,7 @@ if not exist "%1..\..\..\bazel-bin\dotnet\src\webdriver\cdp\v<N>\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 `<N>`):
```
```bash
if [[ ! -f "$1../../../bazel-bin/dotnet/src/webdriver/cdp/v<N>/DevToolsSessionDomains.cs" ]]
then
echo "Generating CDP code for version <N>"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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, global::OpenQA.Selenium.DevTools.Json.DevToolsJsonOptions.DevToolsSerializerOptions);
eventData.EventInvoker(eventArgs);
}
}
Expand Down