From 3694ef6d674381439046667e3ae830b97f5912d7 Mon Sep 17 00:00:00 2001 From: buyaa-n Date: Tue, 13 Aug 2019 15:12:27 -0700 Subject: [PATCH 1/4] =?UTF-8?q?[release/3.0]Handle=20`UnparseableExtension?= =?UTF-8?q?`=20status=20code=20when=20building=20X509Chain=20on=20?= =?UTF-8?q?=E2=80=A6=20(#40117)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ignore `UnparseableExtension` status code when building X509Chain on OSX --- .../System.Security.Cryptography.Native.Apple/pal_x509chain.c | 4 ++++ .../tests/DynamicChainTests.cs | 2 ++ 2 files changed, 6 insertions(+) diff --git a/src/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509chain.c b/src/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509chain.c index 28354226f67c..0944bb196822 100644 --- a/src/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509chain.c +++ b/src/Native/Unix/System.Security.Cryptography.Native.Apple/pal_x509chain.c @@ -174,6 +174,10 @@ static void MergeStatusCodes(CFTypeRef key, CFTypeRef value, void* context) *pStatus |= PAL_X509ChainRevocationStatusUnknown; else if (CFEqual(keyString, CFSTR("MissingIntermediate"))) *pStatus |= PAL_X509ChainPartialChain; + else if (CFEqual(keyString, CFSTR("UnparseableExtension"))) + { + // 10.15 introduced new status code value which is not reported by Windows. Ignoring for now. + } else if (CFEqual(keyString, CFSTR("WeakLeaf")) || CFEqual(keyString, CFSTR("WeakIntermediates")) || CFEqual(keyString, CFSTR("WeakRoot")) || CFEqual(keyString, CFSTR("WeakKeySize"))) { diff --git a/src/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs b/src/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs index c4c0e932c5bb..1f859ddfa916 100644 --- a/src/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs +++ b/src/System.Security.Cryptography.X509Certificates/tests/DynamicChainTests.cs @@ -237,8 +237,10 @@ public static void TestInvalidAia() using (X509Certificate2 ee = certReq.Create(root, notBefore, notAfter, root.GetSerialNumber())) { X509Chain chain = new X509Chain(); + chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; Assert.False(chain.Build(ee)); Assert.Equal(1, chain.ChainElements.Count); + Assert.Equal(X509ChainStatusFlags.PartialChain, chain.AllStatusFlags()); } } } From 46816db48f20ad1294f18408624ec53f5dbe595b Mon Sep 17 00:00:00 2001 From: "dotnet-maestro[bot]" <42748379+dotnet-maestro[bot]@users.noreply.github.com> Date: Wed, 14 Aug 2019 08:16:39 -0700 Subject: [PATCH 2/4] [release/3.0] Update dependencies from 3 repositories (#40308) * Update dependencies from https://github.com/dotnet/core-setup build 20190814.02 - Microsoft.NETCore.App - 3.0.0-preview9-19414-02 - Microsoft.NETCore.DotNetHostPolicy - 3.0.0-preview9-19414-02 - Microsoft.NETCore.DotNetHost - 3.0.0-preview9-19414-02 * Update dependencies from https://github.com/dotnet/arcade build 20190812.7 - Microsoft.DotNet.XUnitExtensions - 2.4.1-beta.19412.7 - Microsoft.DotNet.XUnitConsoleRunner - 2.5.1-beta.19412.7 - Microsoft.DotNet.VersionTools.Tasks - 1.0.0-beta.19412.7 - Microsoft.DotNet.ApiCompat - 1.0.0-beta.19412.7 - Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19412.7 - Microsoft.DotNet.Build.Tasks.Configuration - 1.0.0-beta.19412.7 - Microsoft.DotNet.Build.Tasks.Feed - 2.2.0-beta.19412.7 - Microsoft.DotNet.Build.Tasks.Packaging - 1.0.0-beta.19412.7 - Microsoft.DotNet.CodeAnalysis - 1.0.0-beta.19412.7 - Microsoft.DotNet.CoreFxTesting - 1.0.0-beta.19412.7 - Microsoft.DotNet.GenAPI - 1.0.0-beta.19412.7 - Microsoft.DotNet.GenFacades - 1.0.0-beta.19412.7 - Microsoft.DotNet.Helix.Sdk - 2.0.0-beta.19412.7 - Microsoft.DotNet.RemoteExecutor - 1.0.0-beta.19412.7 * Update dependencies from https://github.com/dotnet/standard build 20190814.3 - NETStandard.Library - 2.1.0-prerelease.19414.3 --- eng/Version.Details.xml | 72 ++++++++++---------- eng/Versions.props | 28 ++++---- eng/common/performance/performance-setup.ps1 | 2 +- eng/common/performance/performance-setup.sh | 36 +++++----- global.json | 8 +-- 5 files changed, 73 insertions(+), 73 deletions(-) diff --git a/eng/Version.Details.xml b/eng/Version.Details.xml index 708f764e573a..f18939cbdbda 100644 --- a/eng/Version.Details.xml +++ b/eng/Version.Details.xml @@ -14,17 +14,17 @@ - + https://github.com/dotnet/core-setup - 16d0c88cff0642a49013048ea24980d65eeed626 + f1de07d0abab3899bd7e1facb7e375d6100a2ac8 - + https://github.com/dotnet/core-setup - 16d0c88cff0642a49013048ea24980d65eeed626 + f1de07d0abab3899bd7e1facb7e375d6100a2ac8 - + https://github.com/dotnet/core-setup - 16d0c88cff0642a49013048ea24980d65eeed626 + f1de07d0abab3899bd7e1facb7e375d6100a2ac8 https://github.com/dotnet/corefx @@ -34,65 +34,65 @@ https://github.com/dotnet/corefx 976b84b4d969ce5d87bc437d811ec8864b47947a - + https://github.com/dotnet/arcade - ec2dd5b3e7d11b88b2ca0688bb1685836cfad20a + 82c822ee7db08f5347e6ac44e3ed465248394a9e - + https://github.com/dotnet/standard - f27dd1491378aa0da5b15aac881fc4de2ea357a8 + 7d56086e753abe6d9a99f67a9b6c021bf7c809ce - + https://github.com/dotnet/arcade - ec2dd5b3e7d11b88b2ca0688bb1685836cfad20a + 82c822ee7db08f5347e6ac44e3ed465248394a9e - + https://github.com/dotnet/arcade - ec2dd5b3e7d11b88b2ca0688bb1685836cfad20a + 82c822ee7db08f5347e6ac44e3ed465248394a9e - + https://github.com/dotnet/arcade - ec2dd5b3e7d11b88b2ca0688bb1685836cfad20a + 82c822ee7db08f5347e6ac44e3ed465248394a9e - + https://github.com/dotnet/arcade - ec2dd5b3e7d11b88b2ca0688bb1685836cfad20a + 82c822ee7db08f5347e6ac44e3ed465248394a9e - + https://github.com/dotnet/arcade - ec2dd5b3e7d11b88b2ca0688bb1685836cfad20a + 82c822ee7db08f5347e6ac44e3ed465248394a9e - + https://github.com/dotnet/arcade - ec2dd5b3e7d11b88b2ca0688bb1685836cfad20a + 82c822ee7db08f5347e6ac44e3ed465248394a9e - + https://github.com/dotnet/arcade - ec2dd5b3e7d11b88b2ca0688bb1685836cfad20a + 82c822ee7db08f5347e6ac44e3ed465248394a9e - + https://github.com/dotnet/arcade - ec2dd5b3e7d11b88b2ca0688bb1685836cfad20a + 82c822ee7db08f5347e6ac44e3ed465248394a9e - + https://github.com/dotnet/arcade - ec2dd5b3e7d11b88b2ca0688bb1685836cfad20a + 82c822ee7db08f5347e6ac44e3ed465248394a9e - + https://github.com/dotnet/arcade - ec2dd5b3e7d11b88b2ca0688bb1685836cfad20a + 82c822ee7db08f5347e6ac44e3ed465248394a9e - + https://github.com/dotnet/arcade - ec2dd5b3e7d11b88b2ca0688bb1685836cfad20a + 82c822ee7db08f5347e6ac44e3ed465248394a9e - + https://github.com/dotnet/arcade - ec2dd5b3e7d11b88b2ca0688bb1685836cfad20a + 82c822ee7db08f5347e6ac44e3ed465248394a9e - + https://github.com/dotnet/arcade - ec2dd5b3e7d11b88b2ca0688bb1685836cfad20a + 82c822ee7db08f5347e6ac44e3ed465248394a9e https://dev.azure.com/dnceng/internal/_git/dotnet-optimization diff --git a/eng/Versions.props b/eng/Versions.props index 6e1368a29280..aa9595f00096 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -29,22 +29,22 @@ - 1.0.0-beta.19412.1 - 1.0.0-beta.19412.1 - 1.0.0-beta.19412.1 - 1.0.0-beta.19412.1 - 2.4.1-beta.19412.1 - 2.5.1-beta.19412.1 - 1.0.0-beta.19412.1 - 1.0.0-beta.19412.1 - 2.2.0-beta.19412.1 - 1.0.0-beta.19412.1 + 1.0.0-beta.19412.7 + 1.0.0-beta.19412.7 + 1.0.0-beta.19412.7 + 1.0.0-beta.19412.7 + 2.4.1-beta.19412.7 + 2.5.1-beta.19412.7 + 1.0.0-beta.19412.7 + 1.0.0-beta.19412.7 + 2.2.0-beta.19412.7 + 1.0.0-beta.19412.7 3.3.0-beta2-19367-02 - 3.0.0-preview9-19412-05 - 3.0.0-preview9-19412-05 - 3.0.0-preview9-19412-05 + 3.0.0-preview9-19414-02 + 3.0.0-preview9-19414-02 + 3.0.0-preview9-19414-02 3.0.0-preview9.19409.2 3.0.0-preview9.19409.2 @@ -52,7 +52,7 @@ 3.0.0-preview9.19409.15 4.6.0-preview9.19409.15 - 2.1.0-prerelease.19413.3 + 2.1.0-prerelease.19414.3 99.99.99-master-20190807.1 diff --git a/eng/common/performance/performance-setup.ps1 b/eng/common/performance/performance-setup.ps1 index 7e5441f79743..ac05256bfd2e 100644 --- a/eng/common/performance/performance-setup.ps1 +++ b/eng/common/performance/performance-setup.ps1 @@ -33,7 +33,7 @@ if ($Framework.StartsWith("netcoreapp")) { } if ($Internal) { - $Queue = "Windows.10.Amd64.ClientRS5.Perf" + $Queue = "Windows.10.Amd64.19H1.Tiger.Perf" $PerfLabArguments = "--upload-to-perflab-container" $ExtraBenchmarkDotNetArguments = "" $Creator = "" diff --git a/eng/common/performance/performance-setup.sh b/eng/common/performance/performance-setup.sh index 126da5f76d43..dc6fd218717b 100755 --- a/eng/common/performance/performance-setup.sh +++ b/eng/common/performance/performance-setup.sh @@ -132,7 +132,7 @@ if [[ "$internal" == true ]]; then if [[ "$architecture" = "arm64" ]]; then queue=Ubuntu.1804.Arm64.Perf else - queue=Ubuntu.1804.Amd64.Perf + queue=Ubuntu.1804.Amd64.Tiger.Perf fi fi @@ -157,20 +157,20 @@ if [[ "$use_core_run" = true ]]; then fi # Make sure all of our variables are available for future steps -echo "##vso[task.setvariable variable=UseCoreRun]$use_core_run" -echo "##vso[task.setvariable variable=Architecture]$architecture" -echo "##vso[task.setvariable variable=PayloadDirectory]$payload_directory" -echo "##vso[task.setvariable variable=PerformanceDirectory]$performance_directory" -echo "##vso[task.setvariable variable=WorkItemDirectory]$workitem_directory" -echo "##vso[task.setvariable variable=Queue]$queue" -echo "##vso[task.setvariable variable=SetupArguments]$setup_arguments" -echo "##vso[task.setvariable variable=Python]python3" -echo "##vso[task.setvariable variable=PerfLabArguments]$perflab_arguments" -echo "##vso[task.setvariable variable=ExtraBenchmarkDotNetArguments]$extra_benchmark_dotnet_arguments" -echo "##vso[task.setvariable variable=BDNCategories]$run_categories" -echo "##vso[task.setvariable variable=TargetCsproj]$csproj" -echo "##vso[task.setvariable variable=RunFromPerfRepo]$run_from_perf_repo" -echo "##vso[task.setvariable variable=Creator]$creator" -echo "##vso[task.setvariable variable=HelixSourcePrefix]$helix_source_prefix" -echo "##vso[task.setvariable variable=Kind]$kind" -echo "##vso[task.setvariable variable=_BuildConfig]$architecture.$kind.$framework" \ No newline at end of file +Write-PipelineSetVariable -name "UseCoreRun" -value "$use_core_run" -is_multi_job_variable false +Write-PipelineSetVariable -name "Architecture" -value "$architecture" -is_multi_job_variable false +Write-PipelineSetVariable -name "PayloadDirectory" -value "$payload_directory" -is_multi_job_variable false +Write-PipelineSetVariable -name "PerformanceDirectory" -value "$performance_directory" -is_multi_job_variable false +Write-PipelineSetVariable -name "WorkItemDirectory" -value "$workitem_directory" -is_multi_job_variable false +Write-PipelineSetVariable -name "Queue" -value "$queue" -is_multi_job_variable false +Write-PipelineSetVariable -name "SetupArguments" -value "$setup_arguments" -is_multi_job_variable false +Write-PipelineSetVariable -name "Python" -value "$python3" -is_multi_job_variable false +Write-PipelineSetVariable -name "PerfLabArguments" -value "$perflab_arguments" -is_multi_job_variable false +Write-PipelineSetVariable -name "ExtraBenchmarkDotNetArguments" -value "$extra_benchmark_dotnet_arguments" -is_multi_job_variable false +Write-PipelineSetVariable -name "BDNCategories" -value "$run_categories" -is_multi_job_variable false +Write-PipelineSetVariable -name "TargetCsproj" -value "$csproj" -is_multi_job_variable false +Write-PipelineSetVariable -name "RunFromPerfRepo" -value "$run_from_perf_repo" -is_multi_job_variable false +Write-PipelineSetVariable -name "Creator" -value "$creator" -is_multi_job_variable false +Write-PipelineSetVariable -name "HelixSourcePrefix" -value "$helix_source_prefix" -is_multi_job_variable false +Write-PipelineSetVariable -name "Kind" -value "$kind" -is_multi_job_variable false +Write-PipelineSetVariable -name "_BuildConfig" -value "$architecture.$kind.$framework" -is_multi_job_variable false \ No newline at end of file diff --git a/global.json b/global.json index 04dd9f5798c3..c6e2e64ddf38 100644 --- a/global.json +++ b/global.json @@ -7,10 +7,10 @@ "dotnet": "3.0.100-preview7-012630" }, "msbuild-sdks": { - "Microsoft.DotNet.Arcade.Sdk": "1.0.0-beta.19412.1", - "Microsoft.DotNet.Helix.Sdk": "2.0.0-beta.19412.1", - "Microsoft.DotNet.Build.Tasks.Configuration": "1.0.0-beta.19412.1", - "Microsoft.DotNet.CoreFxTesting": "1.0.0-beta.19412.1", + "Microsoft.DotNet.Arcade.Sdk": "1.0.0-beta.19412.7", + "Microsoft.DotNet.Helix.Sdk": "2.0.0-beta.19412.7", + "Microsoft.DotNet.Build.Tasks.Configuration": "1.0.0-beta.19412.7", + "Microsoft.DotNet.CoreFxTesting": "1.0.0-beta.19412.7", "FIX-85B6-MERGE-9C38-CONFLICT": "1.0.0", "Microsoft.NET.Sdk.IL": "3.0.0-preview9.19409.2" } From 75ec1b3df476ec48bb6e31ff694d3a8fcf430264 Mon Sep 17 00:00:00 2001 From: Steven Kirbach Date: Thu, 15 Aug 2019 13:32:37 -0700 Subject: [PATCH 3/4] fixing ZipPackagePart.GetStreamCore crashes with NotSupportedException (#40355) ZipArchiveEntry only ever supports opening once when the backing archive is in Create mode, and the backing stream is non-seekable, so we shouldn't call SetLength in that case. You could still open an archive in Update mode then call part.GetStream(FileMode.Create), in which case we'll want this call to SetLength, so we only avoid this call when the backing Archive is in Create mode. updating test to explicitly test the Update path for ZipPackage skip UAP since we don't have access to the file system to create the .zip undo accidental change to existing test removing unnecessary variable --- .../src/System/IO/Packaging/ZipPackagePart.cs | 7 +- src/System.IO.Packaging/tests/Tests.cs | 104 ++++++++++++++---- 2 files changed, 87 insertions(+), 24 deletions(-) diff --git a/src/System.IO.Packaging/src/System/IO/Packaging/ZipPackagePart.cs b/src/System.IO.Packaging/src/System/IO/Packaging/ZipPackagePart.cs index 66bca4d8962d..b13b961901ae 100644 --- a/src/System.IO.Packaging/src/System/IO/Packaging/ZipPackagePart.cs +++ b/src/System.IO.Packaging/src/System/IO/Packaging/ZipPackagePart.cs @@ -27,7 +27,12 @@ protected override Stream GetStreamCore(FileMode streamFileMode, FileAccess stre { if (_zipArchiveEntry != null) { - if (streamFileMode == FileMode.Create) + // Reset the stream when FileMode.Create is specified. Since ZipArchiveEntry only + // ever supports opening once when the backing archive is in Create mode, we'll avoid + // calling SetLength since the stream returned won't be seekable. You could still open + // an archive in Update mode then call part.GetStream(FileMode.Create), in which case + // we'll want this call to SetLength. + if (streamFileMode == FileMode.Create && _zipArchiveEntry.Archive.Mode != ZipArchiveMode.Create) { using (var tempStream = _zipStreamManager.Open(_zipArchiveEntry, streamFileMode, streamFileAccess)) { diff --git a/src/System.IO.Packaging/tests/Tests.cs b/src/System.IO.Packaging/tests/Tests.cs index db8429610247..20f78cc7752f 100644 --- a/src/System.IO.Packaging/tests/Tests.cs +++ b/src/System.IO.Packaging/tests/Tests.cs @@ -3644,53 +3644,111 @@ public void SetEmptyPropertyToNull() [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "Desktop doesn't support Package.Open with FileAccess.Write")] public void CreateWithFileAccessWrite() { - string[] fileNames = new [] { "file1.txt", "file2.txt", "file3.txt" }; - const string RelationshipType = "http://schemas.microsoft.com/relationships/contains"; - const string PartRelationshipType = "http://schemas.microsoft.com/relationships/self"; - using (Stream stream = new MemoryStream()) { using (Package package = Package.Open(stream, FileMode.Create, FileAccess.Write)) { - foreach (string fileName in fileNames) + ForEachPartWithFileName(package, (part, fileName) => { - Uri partUri = PackUriHelper.CreatePartUri(new Uri(fileName, UriKind.Relative)); - PackagePart part = package.CreatePart(partUri, - System.Net.Mime.MediaTypeNames.Text.Plain, - CompressionOption.Fast); using (StreamWriter writer = new StreamWriter(part.GetStream(), Encoding.ASCII)) { // just write the filename as content writer.Write(fileName); } - part.CreateRelationship(part.Uri, TargetMode.Internal, PartRelationshipType); - package.CreateRelationship(part.Uri, TargetMode.Internal, RelationshipType); - } + }); } // reopen for read and validate the content stream.Seek(0, SeekOrigin.Begin); using (Package readPackage = Package.Open(stream)) { - PackageRelationshipCollection packageRelationships = readPackage.GetRelationships(); - Assert.All(packageRelationships, relationship => Assert.Equal(RelationshipType, relationship.RelationshipType)); - foreach (string fileName in fileNames) + ForEachPartWithFileName(readPackage, (part, fileName) => { - PackagePart part = readPackage.GetPart(PackUriHelper.CreatePartUri(new Uri(fileName, UriKind.Relative))); - using (Stream partStream = part.GetStream()) using (StreamReader reader = new StreamReader(partStream, Encoding.ASCII)) { Assert.Equal(fileName.Length, partStream.Length); Assert.Equal(fileName, reader.ReadToEnd()); } - - PackageRelationshipCollection partRelationships = part.GetRelationshipsByType(PartRelationshipType); - Assert.Single(partRelationships); - Assert.All(partRelationships, relationship => Assert.Equal(PartRelationshipType, relationship.RelationshipType)); + }); + } + } + } - Assert.Single(packageRelationships, relationship => relationship.TargetUri == part.Uri); + [Fact] + [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "Desktop doesn't support Package.Open with FileAccess.Write")] + [SkipOnTargetFramework(TargetFrameworkMonikers.Uap, "Can't write to FileSystem in UAP")] + public void ZipPackage_CreateWithFileAccessWrite() + { + string packageName = "test.zip"; + + using (Package package = Package.Open(packageName, FileMode.Create, FileAccess.Write)) + { + ForEachPartWithFileName(package, (part, fileName) => + { + using (StreamWriter writer = new StreamWriter(part.GetStream(FileMode.Create), Encoding.ASCII)) + { + // just write the filename as content + writer.Write(fileName); } + }); + } + + // reopen for read and validate the content + using (Package readPackage = Package.Open(packageName)) + { + ForEachPartWithFileName(readPackage, (part, fileName) => + { + using (Stream partStream = part.GetStream()) + using (StreamReader reader = new StreamReader(partStream, Encoding.ASCII)) + { + Assert.Equal(fileName.Length, partStream.Length); + Assert.Equal(fileName, reader.ReadToEnd()); + } + + using (Stream partStream = part.GetStream(FileMode.Create)) + { + // Assert that the stream was reset because we opened the stream in Create mode + Assert.Equal(0, partStream.Length); + } + }); + } + } + + // Helper method for performing an action on every part in the package. All parts are simple + // text files. If the part didn't exist, it will be created before invoking the action, + // otherwise the existing part is retrieved and passed to the action. + private void ForEachPartWithFileName(Package package, Action action) + { + string[] fileNames = new[] { "file1.txt", "file2.txt", "file3.txt" }; + + const string RelationshipType = "http://schemas.microsoft.com/relationships/contains"; + const string PartRelationshipType = "http://schemas.microsoft.com/relationships/self"; + foreach (string fileName in fileNames) + { + Uri partUri = PackUriHelper.CreatePartUri(new Uri(fileName, UriKind.Relative)); + PackagePart part = package.PartExists(partUri) ? + package.GetPart(partUri) : + package.CreatePart(partUri, System.Net.Mime.MediaTypeNames.Text.Plain); + action(part, fileName); + + // Part didn't exist previously so create relationships + if (package.FileOpenAccess == FileAccess.Write) + { + part.CreateRelationship(part.Uri, TargetMode.Internal, PartRelationshipType); + package.CreateRelationship(part.Uri, TargetMode.Internal, RelationshipType); + } + else + { + // Validate the relationship + PackageRelationshipCollection packageRelationships = package.GetRelationships(); + Assert.All(packageRelationships, relationship => Assert.Equal(RelationshipType, relationship.RelationshipType)); + + PackageRelationshipCollection partRelationships = part.GetRelationshipsByType(PartRelationshipType); + Assert.Single(partRelationships); + Assert.All(partRelationships, relationship => Assert.Equal(PartRelationshipType, relationship.RelationshipType)); + + Assert.Single(packageRelationships, relationship => relationship.TargetUri == part.Uri); } } } @@ -3892,7 +3950,7 @@ public void ComparePackUriDifferentPack() } [Fact] - void CreatePackUriWithFragment() + public void CreatePackUriWithFragment() { Uri partUri = new Uri("/idontexist.xml", UriKind.Relative); Uri packageUri = new Uri("application://"); From f8fddfef5be3a3ea7946a9bb6a86f8a28006f645 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Thu, 15 Aug 2019 15:38:45 -0500 Subject: [PATCH 4/4] Support custom converters that treat non-null input as null (#40287) (#40357) --- .../JsonClassInfo.AddProperty.cs | 21 ++- .../JsonPropertyInfoNotNullable.cs | 3 - ...sonPropertyInfoNotNullableContravariant.cs | 3 - .../CustomConverterTests.NullValueType.cs | 163 ++++++++++++++++++ 4 files changed, 180 insertions(+), 10 deletions(-) diff --git a/src/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.AddProperty.cs b/src/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.AddProperty.cs index d1094002bac2..539af2428921 100644 --- a/src/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.AddProperty.cs +++ b/src/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.AddProperty.cs @@ -112,10 +112,23 @@ internal static JsonPropertyInfo CreateProperty( Type propertyInfoClassType; if (runtimePropertyType.IsGenericType && runtimePropertyType.GetGenericTypeDefinition() == typeof(Nullable<>)) { - // For Nullable, use the underlying type. - Type underlyingPropertyType = Nullable.GetUnderlyingType(runtimePropertyType); - propertyInfoClassType = typeof(JsonPropertyInfoNullable<,>).MakeGenericType(parentClassType, underlyingPropertyType); - converter = options.DetermineConverterForProperty(parentClassType, underlyingPropertyType, propertyInfo); + // First try to find a converter for the Nullable, then if not found use the underlying type. + // This supports custom converters that want to (de)serialize as null when the value is not null. + converter = options.DetermineConverterForProperty(parentClassType, runtimePropertyType, propertyInfo); + if (converter != null) + { + propertyInfoClassType = typeof(JsonPropertyInfoNotNullable<,,,>).MakeGenericType( + parentClassType, + declaredPropertyType, + runtimePropertyType, + runtimePropertyType); + } + else + { + Type typeToConvert = Nullable.GetUnderlyingType(runtimePropertyType); + converter = options.DetermineConverterForProperty(parentClassType, typeToConvert, propertyInfo); + propertyInfoClassType = typeof(JsonPropertyInfoNullable<,>).MakeGenericType(parentClassType, typeToConvert); + } } else { diff --git a/src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoNotNullable.cs b/src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoNotNullable.cs index 079c0a864d6b..59a2fd0e05f8 100644 --- a/src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoNotNullable.cs +++ b/src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoNotNullable.cs @@ -30,9 +30,6 @@ protected override void OnRead(JsonTokenType tokenType, ref ReadStack state, ref } else { - // Null values were already handled. - Debug.Assert(value != null); - Set(state.Current.ReturnValue, value); } } diff --git a/src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoNotNullableContravariant.cs b/src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoNotNullableContravariant.cs index c71ba603a961..d939a00f7bf8 100644 --- a/src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoNotNullableContravariant.cs +++ b/src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoNotNullableContravariant.cs @@ -29,9 +29,6 @@ protected override void OnRead(JsonTokenType tokenType, ref ReadStack state, ref } else { - // Null values were already handled. - Debug.Assert(value != null); - Set(state.Current.ReturnValue, (TDeclaredProperty)value); } diff --git a/src/System.Text.Json/tests/Serialization/CustomConverterTests.NullValueType.cs b/src/System.Text.Json/tests/Serialization/CustomConverterTests.NullValueType.cs index 864f87a88543..50ecaaef31ee 100644 --- a/src/System.Text.Json/tests/Serialization/CustomConverterTests.NullValueType.cs +++ b/src/System.Text.Json/tests/Serialization/CustomConverterTests.NullValueType.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Collections.Generic; +using System.Globalization; using Xunit; namespace System.Text.Json.Serialization.Tests @@ -59,5 +61,166 @@ public static void ValueTypeConverterForNullWithArray() Assert.Equal(1, arr[1]); Assert.Equal(0, arr[2]); } + + /// + /// Allow a conversion of empty string to a null DateTimeOffset?. + /// + public class JsonNullableDateTimeOffsetConverter : JsonConverter + { + public override DateTimeOffset? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + if (reader.TokenType == JsonTokenType.Null) + { + return default; + } + + string value = reader.GetString(); + if (value == string.Empty) + { + return default; + } + + return DateTimeOffset.ParseExact(value, "yyyy/MM/dd HH:mm:ss", CultureInfo.InvariantCulture); + } + + public override void Write(Utf8JsonWriter writer, DateTimeOffset? value, JsonSerializerOptions options) + { + if (!value.HasValue) + { + writer.WriteNullValue(); + } + else + { + writer.WriteStringValue(value.Value.ToString("yyyy/MM/dd HH:mm:ss")); + } + } + } + + private class ClassWithNullableAndJsonConverterAttribute + { + [JsonConverter(typeof(JsonNullableDateTimeOffsetConverter))] + public DateTimeOffset? NullableValue { get; set; } + } + + [Fact] + public static void ValueConverterForNullableWithJsonConverterAttribute() + { + ClassWithNullableAndJsonConverterAttribute obj; + + const string BaselineJson = @"{""NullableValue"":""1989/01/01 11:22:33""}"; + obj = JsonSerializer.Deserialize(BaselineJson); + Assert.NotNull(obj.NullableValue); + + const string Json = @"{""NullableValue"":""""}"; + obj = JsonSerializer.Deserialize(Json); + Assert.Null(obj.NullableValue); + + string json = JsonSerializer.Serialize(obj); + Assert.Contains(@"""NullableValue"":null", json); + } + + private class ClassWithNullableAndWithoutJsonConverterAttribute + { + public DateTimeOffset? NullableValue { get; set; } + public List NullableValues { get; set; } + } + + [Fact] + public static void ValueConverterForNullableWithoutJsonConverterAttribute() + { + const string Json = @"{""NullableValue"":"""", ""NullableValues"":[""""]}"; + ClassWithNullableAndWithoutJsonConverterAttribute obj; + + // The json is not valid with the default converter. + Assert.Throws(() => JsonSerializer.Deserialize(Json)); + + JsonSerializerOptions options = new JsonSerializerOptions(); + options.Converters.Add(new JsonNullableDateTimeOffsetConverter()); + + obj = JsonSerializer.Deserialize(Json, options); + Assert.Null(obj.NullableValue); + Assert.Null(obj.NullableValues[0]); + + string json = JsonSerializer.Serialize(obj); + Assert.Contains(@"""NullableValue"":null", json); + Assert.Contains(@"""NullableValues"":[null]", json); + } + + [JsonConverter(typeof(ClassThatCanBeNullDependingOnContentConverter))] + private class ClassThatCanBeNullDependingOnContent + { + public int MyInt { get; set; } + } + + /// + /// Allow a conversion of ClassThatCanBeNullDependingOnContent to null when its MyInt property is 0. + /// + private class ClassThatCanBeNullDependingOnContentConverter : JsonConverter + { + public override ClassThatCanBeNullDependingOnContent Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + if (reader.TokenType == JsonTokenType.Null) + { + return null; + } + + // Assume a single property. + + reader.Read(); + Assert.Equal(JsonTokenType.PropertyName, reader.TokenType); + + reader.Read(); + int myInt = reader.GetInt16(); + + reader.Read(); + Assert.Equal(JsonTokenType.EndObject, reader.TokenType); + + if (myInt == 0) + { + return null; + } + + return new ClassThatCanBeNullDependingOnContent + { + MyInt = myInt + }; + } + + public override void Write(Utf8JsonWriter writer, ClassThatCanBeNullDependingOnContent value, JsonSerializerOptions options) + { + writer.WriteStartObject(); + + if (value.MyInt == 0) + { + writer.WriteNull("MyInt"); + } + else + { + writer.WriteNumber("MyInt", value.MyInt); + } + + writer.WriteEndObject(); + } + } + + [Fact] + public static void ConverterForClassThatCanBeNullDependingOnContent() + { + ClassThatCanBeNullDependingOnContent obj; + + obj = JsonSerializer.Deserialize(@"{""MyInt"":5}"); + Assert.Equal(5, obj.MyInt); + + string json; + json = JsonSerializer.Serialize(obj); + Assert.Contains(@"""MyInt"":5", json); + + obj.MyInt = 0; + json = JsonSerializer.Serialize(obj); + Assert.Contains(@"""MyInt"":null", json); + + obj = JsonSerializer.Deserialize(@"{""MyInt"":0}"); + Assert.Null(obj); + } } }