Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

[release/3.0] Avoid MemoryMarshal.Cast when transcoding from UTF-16 to UTF-8 while escaping in Utf8JsonWriter. #40997

Merged
merged 5 commits into from
Sep 11, 2019

Conversation

ahsonkhan
Copy link
Member

@ahsonkhan ahsonkhan commented Sep 10, 2019

Port of #40996 to fix https://github.com/dotnet/corefx/issues/40979

Description

Instead of using MemoryMarshal to re-interpret cast a span of UTF-16 chars to bytes (to pass them to APIs expecting UTF-8 data), call the JavascriptEncoder API that expects UTF-16 chars instead. Casting char to byte doesn't transode it from UTF-16 to UTF-8 which was the previous intention. Doing so results in certain invariants in the code to break since the resulting index that points to the first character to escape would be incorrect (or even out of the bounds of the original span), which results in a negative value being passed in to stackalloc (and hence a stackoverflow). The issue is in the code-path where a custom encoder is passed in (and wouldn't happen by default).

Customer Impact

The bug was customer-reported where the user observed a stackoverflow in an ASP.NET WebAPI (which uses a custom encoder) when trying to serialize a string that contained non-ascii characters (for example chinese caracters). Generally, any use of the JsonSerializer or Utf8JsonWriter where a custom encoder is involved for writing .NET strings as JSON is affected. It is imperative that the escaping behavior of the serializer is functionally correct.

Regression?

Introduced in .NET Core 3.0 - preview 8

Risk

The risk of this change is around the escaping behavior changing when the user passes in a custom escaper. Be default, the JSON stack uses the default escaper which isn't affected by this change. However, the ASP.NET defaults to a custom escaper so end-users are more likely to be affected by this escaping behavior and fix. There is no easy workaround for the user since passing in a custom/default encoder wouldn't always work either.

Tests run / added

  • Regression test for the serializer was added along with targeted tests of the writer itself (which the serializer uses under the covers).
  • Verified end-to-end in an ASP.NET Web API app that writing non-ascii strings with the custom escaper works as expected.

cc @steveharter, @GrabYourPitchforks, @pranavkm, @ericstj

@ahsonkhan ahsonkhan added this to the 3.0 milestone Sep 10, 2019
@ahsonkhan ahsonkhan self-assigned this Sep 10, 2019
@ahsonkhan
Copy link
Member Author

ahsonkhan commented Sep 11, 2019

MacOS Build x64_Debug test failures are unrelated:
https://dev.azure.com/dnceng/public/_build/results?buildId=348664&view=ms.vss-test-web.build-test-results-tab&runId=10473744&resultId=168822&paneView=debug

System.Security.Cryptography.OpenSsl.Tests on netcoreapp-OSX-Debug-x64-OSX.1014.Amd64.Open

System.Security.Cryptography.OpenSsl.Tests Total: 649, Errors: 0, Failed: 565, Skipped: 14, Time: 1.083s

https://github.com/dotnet/corefx/issues/40262
https://github.com/dotnet/corefx/issues/38498

Example test failure:

System.Security.Cryptography.Dsa.Tests.DSAKeyFileTests.ReadWriteDsa1024EncryptedPkcs8_PasswordBytes [FAIL]
      System.TypeInitializationException : The type initializer for 'Crypto' threw an exception.
      ---- System.TypeInitializationException : The type initializer for 'CryptoInitializer' threw an exception.
      -------- System.DllNotFoundException : Unable to load shared library 'System.Security.Cryptography.Native.OpenSsl' or one of its dependencies. In order to help diagnose loading problems, consider setting the DYLD_PRINT_LIBRARIES environment variable: dlopen(libSystem.Security.Cryptography.Native.OpenSsl, 1): image not found
      Stack Trace:
           at Interop.Crypto.DsaKeyCreateByExplicitParameters(SafeDsaHandle& dsa, Byte[] p, Int32 pLength, Byte[] q, Int32 qLength, Byte[] g, Int32 gLength, Byte[] y, Int32 yLength, Byte[] x, Int32 xLength)
        /_/src/Common/src/System/Security/Cryptography/DSAOpenSsl.cs(112,0): at System.Security.Cryptography.DSAOpenSsl.ImportParameters(DSAParameters parameters)
        /_/src/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/DSA.cs(349,0): at System.Security.Cryptography.DSA.ImportEncryptedPkcs8PrivateKey(ReadOnlySpan`1 passwordBytes, ReadOnlySpan`1 source, Int32& bytesRead)
        /_/src/Common/src/System/Security/Cryptography/DSAOpenSsl.cs(132,0): at System.Security.Cryptography.DSAOpenSsl.ImportEncryptedPkcs8PrivateKey(ReadOnlySpan`1 passwordBytes, ReadOnlySpan`1 source, Int32& bytesRead)
        /_/src/Common/tests/System/Security/Cryptography/AlgorithmImplementations/DSA/DSAKeyFileTests.cs(593,0): at System.Security.Cryptography.Dsa.Tests.DSAKeyFileTests.<>c__DisplayClass23_0.<ReadBase64EncryptedPkcs8>b__0(DSA dsa, ReadOnlySpan`1 source, Int32& read)
        /_/src/Common/tests/System/Security/Cryptography/AlgorithmImplementations/DSA/DSAKeyFileTests.cs(652,0): at System.Security.Cryptography.Dsa.Tests.DSAKeyFileTests.ReadWriteKey(String base64, DSAParameters& expected, ReadKeyAction readAction, Func`2 writeArrayFunc, WriteKeyToSpanFunc writeSpanFunc, Boolean isEncrypted)
        /_/src/Common/tests/System/Security/Cryptography/AlgorithmImplementations/DSA/DSAKeyFileTests.cs(589,0): at System.Security.Cryptography.Dsa.Tests.DSAKeyFileTests.ReadBase64EncryptedPkcs8(String base64EncPkcs8, Byte[] passwordBytes, PbeParameters pbeParameters, DSAParameters& expected)
        /_/src/Common/tests/System/Security/Cryptography/AlgorithmImplementations/DSA/DSAKeyFileTests.cs(174,0): at System.Security.Cryptography.Dsa.Tests.DSAKeyFileTests.ReadWriteDsa1024EncryptedPkcs8_PasswordBytes()
        ----- Inner Stack Trace -----
        /_/src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Initialization.cs(40,0): at Interop.CryptoInitializer.Initialize()
        /_/src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Initialization.cs(19,0): at Interop.Crypto..cctor()
        ----- Inner Stack Trace -----
           at Interop.CryptoInitializer.EnsureOpenSslInitialized()
        /_/src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Initialization.cs(27,0): at Interop.CryptoInitializer..cctor()

@ahsonkhan ahsonkhan merged commit dbdb299 into dotnet:release/3.0 Sep 11, 2019
@ahsonkhan ahsonkhan deleted the PortFixingEscaping branch September 11, 2019 00:48
ViktorHofer pushed a commit that referenced this pull request Sep 13, 2019
* Disable SDL validation (#40903)

SDL validation is too expensive to run on a per-build basis. Disable for now

* [release/3.0] Update dependencies from dotnet/standard (#40911)

* Update dependencies from https://github.com/dotnet/standard build 20190907.2

- NETStandard.Library - 2.1.0-prerelease.19457.2

* Update dependencies from https://github.com/dotnet/standard build 20190907.1

- NETStandard.Library - 2.1.0-prerelease.19457.1

* [release/3.0] Update dependencies from 3 repositories (#40915)

* Update dependencies from https://github.com/dotnet/core-setup build 20190907.02

- Microsoft.NETCore.App - 3.0.0-rc2-19457-02
- Microsoft.NETCore.DotNetHost - 3.0.0-rc2-19457-02
- Microsoft.NETCore.DotNetHostPolicy - 3.0.0-rc2-19457-02

* Update dependencies from https://github.com/dotnet/arcade build 20190906.10

- Microsoft.DotNet.XUnitExtensions - 2.4.1-beta.19456.10
- Microsoft.DotNet.XUnitConsoleRunner - 2.5.1-beta.19456.10
- Microsoft.DotNet.VersionTools.Tasks - 1.0.0-beta.19456.10
- Microsoft.DotNet.ApiCompat - 1.0.0-beta.19456.10
- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19456.10
- Microsoft.DotNet.Build.Tasks.Configuration - 1.0.0-beta.19456.10
- Microsoft.DotNet.Build.Tasks.Feed - 2.2.0-beta.19456.10
- Microsoft.DotNet.Build.Tasks.Packaging - 1.0.0-beta.19456.10
- Microsoft.DotNet.CodeAnalysis - 1.0.0-beta.19456.10
- Microsoft.DotNet.CoreFxTesting - 1.0.0-beta.19456.10
- Microsoft.DotNet.GenAPI - 1.0.0-beta.19456.10
- Microsoft.DotNet.GenFacades - 1.0.0-beta.19456.10
- Microsoft.DotNet.Helix.Sdk - 2.0.0-beta.19456.10
- Microsoft.DotNet.RemoteExecutor - 1.0.0-beta.19456.10

* Update dependencies from https://github.com/dotnet/standard build 20190907.5

- NETStandard.Library - 2.1.0-prerelease.19457.5

* Disable ToolboxBitmatAttribute test in netfx (#40901) (#40908)

* [release/3.0] Update dependencies from 4 repositories (#40929)

* Update dependencies from https://github.com/dotnet/core-setup build 20190907.15

- Microsoft.NETCore.App - 3.0.0-rc2-19457-15
- Microsoft.NETCore.DotNetHost - 3.0.0-rc2-19457-15
- Microsoft.NETCore.DotNetHostPolicy - 3.0.0-rc2-19457-15

* Update dependencies from https://github.com/dotnet/arcade build 20190907.1

- Microsoft.DotNet.XUnitExtensions - 2.4.1-beta.19457.1
- Microsoft.DotNet.XUnitConsoleRunner - 2.5.1-beta.19457.1
- Microsoft.DotNet.VersionTools.Tasks - 1.0.0-beta.19457.1
- Microsoft.DotNet.ApiCompat - 1.0.0-beta.19457.1
- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19457.1
- Microsoft.DotNet.Build.Tasks.Configuration - 1.0.0-beta.19457.1
- Microsoft.DotNet.Build.Tasks.Feed - 2.2.0-beta.19457.1
- Microsoft.DotNet.Build.Tasks.Packaging - 1.0.0-beta.19457.1
- Microsoft.DotNet.CodeAnalysis - 1.0.0-beta.19457.1
- Microsoft.DotNet.CoreFxTesting - 1.0.0-beta.19457.1
- Microsoft.DotNet.GenAPI - 1.0.0-beta.19457.1
- Microsoft.DotNet.GenFacades - 1.0.0-beta.19457.1
- Microsoft.DotNet.Helix.Sdk - 2.0.0-beta.19457.1
- Microsoft.DotNet.RemoteExecutor - 1.0.0-beta.19457.1

* Update dependencies from https://dev.azure.com/dnceng/internal/_git/dotnet-optimization build 20190908.1

- optimization.windows_nt-x64.IBC.CoreFx - 99.99.99-master-20190908.1

* Update dependencies from https://github.com/dotnet/standard build 20190908.3

- NETStandard.Library - 2.1.0-prerelease.19458.3

* [release/3.0] Update dependencies from 4 repositories (#40940)

* Update dependencies from https://github.com/dotnet/core-setup build 20190908.11

- Microsoft.NETCore.App - 3.0.0-rc2-19458-11
- Microsoft.NETCore.DotNetHost - 3.0.0-rc2-19458-11
- Microsoft.NETCore.DotNetHostPolicy - 3.0.0-rc2-19458-11

* Update dependencies from https://github.com/dotnet/arcade build 20190908.2

- Microsoft.DotNet.XUnitExtensions - 2.4.1-beta.19458.2
- Microsoft.DotNet.XUnitConsoleRunner - 2.5.1-beta.19458.2
- Microsoft.DotNet.VersionTools.Tasks - 1.0.0-beta.19458.2
- Microsoft.DotNet.ApiCompat - 1.0.0-beta.19458.2
- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19458.2
- Microsoft.DotNet.Build.Tasks.Configuration - 1.0.0-beta.19458.2
- Microsoft.DotNet.Build.Tasks.Feed - 2.2.0-beta.19458.2
- Microsoft.DotNet.Build.Tasks.Packaging - 1.0.0-beta.19458.2
- Microsoft.DotNet.CodeAnalysis - 1.0.0-beta.19458.2
- Microsoft.DotNet.CoreFxTesting - 1.0.0-beta.19458.2
- Microsoft.DotNet.GenAPI - 1.0.0-beta.19458.2
- Microsoft.DotNet.GenFacades - 1.0.0-beta.19458.2
- Microsoft.DotNet.Helix.Sdk - 2.0.0-beta.19458.2
- Microsoft.DotNet.RemoteExecutor - 1.0.0-beta.19458.2

* Update dependencies from https://dev.azure.com/dnceng/internal/_git/dotnet-optimization build 20190909.1

- optimization.windows_nt-x64.IBC.CoreFx - 99.99.99-master-20190909.1

* Update dependencies from https://github.com/dotnet/standard build 20190909.3

- NETStandard.Library - 2.1.0-prerelease.19459.3

* Add missing IAsyncDisposable interfaces to System.Data (#40872)

Part of #35012

* Update dependencies from https://github.com/dotnet/coreclr build 20190909.3 (#40956)

- Microsoft.NET.Sdk.IL - 3.0.0-rc2.19459.3
- Microsoft.NETCore.ILAsm - 3.0.0-rc2.19459.3
- Microsoft.NETCore.Runtime.CoreCLR - 3.0.0-rc2.19459.3

* Fix TypeConverter for IComponent (#40837) (#40883)

* .NET Core 3.0 Prev9 Intellisense nupkg version bump (#40963) (#40965)

* [release/3.0] Update dependencies from 4 repositories (#40951)

* Update dependencies from https://github.com/dotnet/standard build 20190909.4

- NETStandard.Library - 2.1.0-prerelease.19459.4

* Update dependencies from https://github.com/dotnet/core-setup build 20190909.40

- Microsoft.NETCore.App - 3.0.0-rc2-19459-40
- Microsoft.NETCore.DotNetHost - 3.0.0-rc2-19459-40
- Microsoft.NETCore.DotNetHostPolicy - 3.0.0-rc2-19459-40

* Update dependencies from https://github.com/dotnet/arcade build 20190909.10

- Microsoft.DotNet.XUnitExtensions - 2.4.1-beta.19459.10
- Microsoft.DotNet.XUnitConsoleRunner - 2.5.1-beta.19459.10
- Microsoft.DotNet.VersionTools.Tasks - 1.0.0-beta.19459.10
- Microsoft.DotNet.ApiCompat - 1.0.0-beta.19459.10
- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19459.10
- Microsoft.DotNet.Build.Tasks.Configuration - 1.0.0-beta.19459.10
- Microsoft.DotNet.Build.Tasks.Feed - 2.2.0-beta.19459.10
- Microsoft.DotNet.Build.Tasks.Packaging - 1.0.0-beta.19459.10
- Microsoft.DotNet.CodeAnalysis - 1.0.0-beta.19459.10
- Microsoft.DotNet.CoreFxTesting - 1.0.0-beta.19459.10
- Microsoft.DotNet.GenAPI - 1.0.0-beta.19459.10
- Microsoft.DotNet.GenFacades - 1.0.0-beta.19459.10
- Microsoft.DotNet.Helix.Sdk - 2.0.0-beta.19459.10
- Microsoft.DotNet.RemoteExecutor - 1.0.0-beta.19459.10

* Update dependencies from https://dev.azure.com/dnceng/internal/_git/dotnet-optimization build 20190910.1

- optimization.windows_nt-x64.IBC.CoreFx - 99.99.99-master-20190910.1

* Add test for IComponent typeconverter register in TypeDescriptor (#40959) (#40977)

* Update dependencies from https://github.com/dotnet/coreclr build 20190910.2 (#40984)

- Microsoft.NET.Sdk.IL - 3.0.0-rc2.19460.2
- Microsoft.NETCore.ILAsm - 3.0.0-rc2.19460.2
- Microsoft.NETCore.Runtime.CoreCLR - 3.0.0-rc2.19460.2

* Update dependencies from https://github.com/dotnet/coreclr build 20190910.4 (#41006)

- Microsoft.NET.Sdk.IL - 3.0.0-rc2.19460.4
- Microsoft.NETCore.ILAsm - 3.0.0-rc2.19460.4
- Microsoft.NETCore.Runtime.CoreCLR - 3.0.0-rc2.19460.4

* [release/3.0] Update dependencies from dotnet/arcade dotnet/standard (#40986)

* Update dependencies from https://github.com/dotnet/arcade build 20190910.3

- Microsoft.DotNet.XUnitExtensions - 2.4.1-beta.19460.3
- Microsoft.DotNet.XUnitConsoleRunner - 2.5.1-beta.19460.3
- Microsoft.DotNet.VersionTools.Tasks - 1.0.0-beta.19460.3
- Microsoft.DotNet.ApiCompat - 1.0.0-beta.19460.3
- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19460.3
- Microsoft.DotNet.Build.Tasks.Configuration - 1.0.0-beta.19460.3
- Microsoft.DotNet.Build.Tasks.Feed - 2.2.0-beta.19460.3
- Microsoft.DotNet.Build.Tasks.Packaging - 1.0.0-beta.19460.3
- Microsoft.DotNet.CodeAnalysis - 1.0.0-beta.19460.3
- Microsoft.DotNet.CoreFxTesting - 1.0.0-beta.19460.3
- Microsoft.DotNet.GenAPI - 1.0.0-beta.19460.3
- Microsoft.DotNet.GenFacades - 1.0.0-beta.19460.3
- Microsoft.DotNet.Helix.Sdk - 2.0.0-beta.19460.3
- Microsoft.DotNet.RemoteExecutor - 1.0.0-beta.19460.3

* Update dependencies from https://github.com/dotnet/standard build 20190910.4

- NETStandard.Library - 2.1.0-prerelease.19460.4

* Update dependencies from https://github.com/dotnet/standard build 20190910.5

- NETStandard.Library - 2.1.0-prerelease.19460.5

* [release/3.0] Avoid MemoryMarshal.Cast when transcoding from UTF-16 to UTF-8 while escaping in Utf8JsonWriter. (#40997)

* Avoid MemoryMarshal.Cast when transcoding from UTF-16 to UTF-8 while
escaping in Utf8JsonWriter.

* Fix a typo in spacing within the test.

* Guard against empty spans where an implementation of JavascriptEncoder
might not handle null ptrs correctly.

* Cleanup tests to avoid some duplication.

* Some more test clean up.

* Update dependencies from https://github.com/dotnet/coreclr build 20190910.8 (#41011)

- Microsoft.NET.Sdk.IL - 3.0.0-rc2.19460.8
- Microsoft.NETCore.ILAsm - 3.0.0-rc2.19460.8
- Microsoft.NETCore.Runtime.CoreCLR - 3.0.0-rc2.19460.8

* Update dependencies from https://github.com/dotnet/coreclr build 20190910.11 (#41014)

- Microsoft.NET.Sdk.IL - 3.0.0-rc2.19460.11
- Microsoft.NETCore.ILAsm - 3.0.0-rc2.19460.11
- Microsoft.NETCore.Runtime.CoreCLR - 3.0.0-rc2.19460.11

* [release/3.0] Update dependencies from 3 repositories (#41022)

* Update dependencies from https://github.com/dotnet/core-setup build 20190910.02

- Microsoft.NETCore.App - 3.0.0-rc2-19460-02
- Microsoft.NETCore.DotNetHost - 3.0.0-rc2-19460-02
- Microsoft.NETCore.DotNetHostPolicy - 3.0.0-rc2-19460-02

* Update dependencies from https://dev.azure.com/dnceng/internal/_git/dotnet-optimization build 20190911.1

- optimization.windows_nt-x64.IBC.CoreFx - 99.99.99-master-20190911.1

* Update dependencies from https://github.com/dotnet/standard build 20190911.3

- NETStandard.Library - 2.1.0-prerelease.19461.3

* Update dependencies from https://github.com/dotnet/coreclr build 20190911.3 (#41035)

- Microsoft.NET.Sdk.IL - 3.0.0-rc2.19461.3
- Microsoft.NETCore.ILAsm - 3.0.0-rc2.19461.3
- Microsoft.NETCore.Runtime.CoreCLR - 3.0.0-rc2.19461.3

* adding version suffix as non empty for building release package versions

* Update dependencies from https://github.com/dotnet/coreclr build 20190911.5 (#41045)

- Microsoft.NET.Sdk.IL - 3.0.0-rc2.19461.5
- Microsoft.NETCore.ILAsm - 3.0.0-rc2.19461.5
- Microsoft.NETCore.Runtime.CoreCLR - 3.0.0-rc2.19461.5

* [release/3.0] Update dependencies from 3 repositories (#41052)

* Update dependencies from https://github.com/dotnet/arcade build 20190911.7

- Microsoft.DotNet.XUnitExtensions - 2.4.1-beta.19461.7
- Microsoft.DotNet.XUnitConsoleRunner - 2.5.1-beta.19461.7
- Microsoft.DotNet.VersionTools.Tasks - 1.0.0-beta.19461.7
- Microsoft.DotNet.ApiCompat - 1.0.0-beta.19461.7
- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19461.7
- Microsoft.DotNet.Build.Tasks.Configuration - 1.0.0-beta.19461.7
- Microsoft.DotNet.Build.Tasks.Feed - 2.2.0-beta.19461.7
- Microsoft.DotNet.Build.Tasks.Packaging - 1.0.0-beta.19461.7
- Microsoft.DotNet.CodeAnalysis - 1.0.0-beta.19461.7
- Microsoft.DotNet.CoreFxTesting - 1.0.0-beta.19461.7
- Microsoft.DotNet.GenAPI - 1.0.0-beta.19461.7
- Microsoft.DotNet.GenFacades - 1.0.0-beta.19461.7
- Microsoft.DotNet.Helix.Sdk - 2.0.0-beta.19461.7
- Microsoft.DotNet.RemoteExecutor - 1.0.0-beta.19461.7

* Update dependencies from https://dev.azure.com/dnceng/internal/_git/dotnet-optimization build 20190912.1

- optimization.windows_nt-x64.IBC.CoreFx - 99.99.99-master-20190912.1

* Update dependencies from https://github.com/dotnet/standard build 20190912.2

- NETStandard.Library - 2.1.0-prerelease.19462.2

* Update dependencies from https://github.com/dotnet/standard build 20190912.4

- NETStandard.Library - 2.1.0-prerelease.19462.4

* Update dependencies from https://github.com/dotnet/coreclr build 20190912.2 (#41062)

- Microsoft.NET.Sdk.IL - 3.0.0-rc2.19462.2
- Microsoft.NETCore.ILAsm - 3.0.0-rc2.19462.2
- Microsoft.NETCore.Runtime.CoreCLR - 3.0.0-rc2.19462.2

* Update dependencies from https://github.com/dotnet/standard build 20190912.5

- NETStandard.Library - 2.1.0

* Stabilize package versions (#41076)

* Update dependencies from https://github.com/dotnet/coreclr build 20190912.5 (#41081)

- Microsoft.NET.Sdk.IL - 3.0.0-rc2.19462.5
- Microsoft.NETCore.ILAsm - 3.0.0-rc2.19462.5
- Microsoft.NETCore.Runtime.CoreCLR - 3.0.0-rc2.19462.5
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants