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

[Breaking change]: JsonSerializer throws FormatException when deserializing Version types with leading or trailing whitespace #26292

Closed
2 tasks done
layomia opened this issue Sep 28, 2021 · 1 comment · Fixed by #27809
Assignees
Labels
binary incompatible Existing binaries may encounter a breaking change in behavior. breaking-change Indicates a .NET Core breaking change 🏁 Release: .NET 7 Work items for the .NET 7 release doc-idea Indicates issues that are suggestions for new topics [org][type][category] Pri1 High priority, do before Pri2 and Pri3 source incompatible Source code may encounter a breaking change in behavior when targeting the new version.

Comments

@layomia
Copy link
Contributor

layomia commented Sep 28, 2021

Description

JsonSerializer goes from permitting leading and trailing whitespace when deserializing Version types to throwing FormatException. Breaking change introduced in .NET 7 Preview 1.

Version

Other (please put exact version in description textbox)

Previous behavior

Provided in description.

New behavior

Provided in description.

Type of breaking change

  • Binary incompatible: Existing binaries may encounter a breaking change in behavior, such as failure to load/execute or different run-time behavior.
  • Source incompatible: Source code may encounter a breaking change in behavior when targeting the new runtime/component/SDK, such as compile errors or different run-time behavior.

Reason for change

We optimized the implementation of the underlying Version converter. This resulted in the implementation being made to align with the behavior for other primitive types supported by System.Text.Json e.g. DateTime and Guid which also disallow leading and trailing spaces.

Recommended action

To get the old behavior back, add a custom converter for the type which permits whitespace:

internal sealed class VersionConverter : JsonConverter<Version>
{
    public override Version Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        string? versionString = reader.GetString();
        if (Version.TryParse(versionString, out Version? result))
        {
            return result;
        }

        ThrowHelper.ThrowJsonException();
        return null;
    }

    public override void Write(Utf8JsonWriter writer, Version value, JsonSerializerOptions options)
    {
        writer.WriteStringValue(value.ToString());
    }
}

Feature area

Core .NET libraries

Affected APIs

All System.Text.Json.JsonSerializer.Deserialize method overloads.

@layomia layomia added doc-idea Indicates issues that are suggestions for new topics [org][type][category] breaking-change Indicates a .NET Core breaking change Pri1 High priority, do before Pri2 and Pri3 labels Sep 28, 2021
@dotnet-bot dotnet-bot added ⌚ Not Triaged Not triaged binary incompatible Existing binaries may encounter a breaking change in behavior. source incompatible Source code may encounter a breaking change in behavior when targeting the new version. labels Sep 28, 2021
@N0D4N
Copy link
Contributor

N0D4N commented Sep 28, 2021

Perhaps in "Recommended action" section

ThrowHelper.ThrowJsonException();
return null;

can be replaced with

throw new JsonException() { AppendPathInformation = true };

since most likely consumers won't have static class ThrowHelper, and this way proposed solution to described breaking change will work out of the box after importing some usings.
Also in "Recommended action" i think it would be good to have a link to docs showing how to apply/register custom converter, so JsonSerializer will use it instead of built-in one.

@adegeo adegeo removed the ⌚ Not Triaged Not triaged label Sep 29, 2021
@gewarren gewarren added the 🏁 Release: .NET 7 Work items for the .NET 7 release label Oct 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
binary incompatible Existing binaries may encounter a breaking change in behavior. breaking-change Indicates a .NET Core breaking change 🏁 Release: .NET 7 Work items for the .NET 7 release doc-idea Indicates issues that are suggestions for new topics [org][type][category] Pri1 High priority, do before Pri2 and Pri3 source incompatible Source code may encounter a breaking change in behavior when targeting the new version.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants