This repository was archived by the owner on Jan 23, 2023. It is now read-only.
Support custom converters that treat non-null input as null#40287
Merged
steveharter merged 1 commit intodotnet:masterfrom Aug 15, 2019
Merged
Support custom converters that treat non-null input as null#40287steveharter merged 1 commit intodotnet:masterfrom
steveharter merged 1 commit intodotnet:masterfrom
Conversation
layomia
approved these changes
Aug 13, 2019
| } | ||
|
|
||
| // Assume a single property. | ||
|
|
Contributor
There was a problem hiding this comment.
nit: assert token type is StartObject to be consistent with below assertions?
Contributor
Author
There was a problem hiding this comment.
The asserts in this converter are unnecessary since the semantics are verified by the calling test. I added the other assert and comment for readability.
steveharter
added a commit
to steveharter/dotnet_corefx
that referenced
this pull request
Aug 15, 2019
What happens in release? |
ahsonkhan
reviewed
Aug 15, 2019
| // Null values were already handled. | ||
| Debug.Assert(value != null); | ||
|
|
||
| Set(state.Current.ReturnValue, (TDeclaredProperty)value); |
There was a problem hiding this comment.
If value could be null here now, could this cast (and subsequent call to Set), null ref somewhere down the call stack?
ahsonkhan
reviewed
Aug 15, 2019
ahsonkhan
left a comment
ahsonkhan
left a comment
There was a problem hiding this comment.
I don't fully understand the underlying issue (and Nullable support) that this is fixing, but the change makes sense.
eerhardt
pushed a commit
that referenced
this pull request
Aug 15, 2019
Anipik
pushed a commit
that referenced
this pull request
Aug 15, 2019
* [release/3.0]Handle `UnparseableExtension` status code when building X509Chain on … (#40117) Ignore `UnparseableExtension` status code when building X509Chain on OSX * [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 * 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 * Support custom converters that treat non-null input as null (#40287) (#40357)
picenka21
pushed a commit
to picenka21/runtime
that referenced
this pull request
Feb 18, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds support to allow a custom converter to return null on deserialize when the input is not null. This applies to both
Nullable<T>and classes.Without this, an
InvalidOperationExceptionis raised for theNullable<T>case and aDebug.Assertfailure for the class case.Fixes https://github.com/dotnet/corefx/issues/40229
I will recommend porting this to 3.0 once in master as the scenario is valid, the fix is low-risk, and common enough to be a blocking issue for many.