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

PlatformNotSupportedException when attempting to serialize DirectoryInfo instance with Newtonsoft.Json #23261

Closed
Cisien opened this issue Aug 19, 2017 · 21 comments
Labels
area-System.IO bug tenet-compatibility Incompatibility with previous versions or .NET Framework
Milestone

Comments

@Cisien
Copy link

Cisien commented Aug 19, 2017

Cross-posting with JamesNK/Newtonsoft.Json#1404 to demonstrate the pain this can cause with 3rd party libraries that have historically relied on this behavior vs simply removing the API which would bring attention to the problem immediately, instead of at runtime.

Please consider removing these APIs in the future

Source/destination types

System.IO.DirectoryInfo

Source/destination JSON

None/serialization fails.

Expected behavior

Serialization of the DirectoryInfo object to function how it works on .net 4.x

Actual behavior

Message: System.PlatformNotSupportedException : Operation is not supported on this platform.
at System.IO.FileSystemInfo.GetObjectData(SerializationInfo info, StreamingContext context)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeISerializable(JsonWriter writer, ISerializable value, JsonISerializableContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.Serialize(JsonWriter jsonWriter, Object value, Type objectType)
   at Newtonsoft.Json.JsonSerializer.SerializeInternal(JsonWriter jsonWriter, Object value, Type objectType)
   at Newtonsoft.Json.JsonConvert.SerializeObjectInternal(Object value, Type type, JsonSerializer jsonSerializer)
   at CSDiscordService.EvalTests.<JsonConvertOfDirectoryInfoObject>d__15.MoveNext() in C:\src\CSDiscord\CSDiscordService.Tests\EvalTests.cs:line 88
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
Result Message:	System.PlatformNotSupportedException : Operation is not supported on this platform.

Appears to be related to a change made in the .net core implementation to no longer support serializing several types.
dotnet/corefx#20220

Steps to reproduce

PM> dotnet --info
.NET Command Line Tools (2.0.1-servicing-006924)

Product Information:
 Version:            2.0.1-servicing-006924
 Commit SHA-1 hash:  1ed6be56ca

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.16257
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\2.0.1-servicing-006924\

Microsoft .NET Core Shared Framework Host
  Version  : 2.0.0
  Build    : e8b8861ac7faf042c87a5c2f9f2d04c98b69f28d
var di = Directory.CreateDirectory("C:\\this\\doesnt\\exist");
var json = JsonConvert.SerializeObject(di);
Console.WriteLine(json);

Other Observations

SerializerSettings.Error = (s, e) => e.ErrorContext.Handled = true; does not handle this exception
setting SerializerSettings.ContractResolver = new DefaultContractResolver { IgnoreSerializableInterface = true }; does not change this behavior

@ViktorHofer
Copy link
Member

ViktorHofer commented Aug 19, 2017

We brought back binary serialization with .NET Core 2.0 but only for a limited set of types: https://github.com/dotnet/corefx/issues/19119.

The ISerializable interface and the respective GetObjectData method which is called by Newtonsonft.Json is intended to be used by BinaryFormatter for binary serialization.

Any class that might be serialized must be marked with the SerializableAttribute. If a class needs to control its serialization process, it can implement the ISerializable interface. The Formatter calls the GetObjectData at serialization time and populates the supplied SerializationInfo with all the data required to represent the object. The Formatter creates a SerializationInfo with the type of the object in the graph.

https://msdn.microsoft.com/de-de/library/system.runtime.serialization.iserializable(v=vs.110).aspx

We don't expect it to be used by other serialization frameworks. As the GetObjectData method is public we couldn't simply remove it without breaking compatibility but "disabled" it so that casting to ISerializable is still working.

cc @JamesNK @stephentoub

@ViktorHofer
Copy link
Member

ViktorHofer commented Aug 19, 2017

A possible solution could be to change this line:
https://github.com/JamesNK/Newtonsoft.Json/blob/7673a18e55c2b30a69ce0cf972b12d8a7c42d5d3/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalWriter.cs#L863

to

try
{
    value.GetObjectData(serializationInfo, Serializer._context);
}
catch (PlatformNotSupportedException) { }

@danmoseley
Copy link
Member

Anyway is it correct that if this type did not support ISerializable, the issue would be found at compile time? I would have expected a serializer to cast dynamically.

@ViktorHofer
Copy link
Member

ViktorHofer commented Aug 21, 2017

Correct. Newtonsonft.Json casts the object to ISerializable like that:
ISerializable val = (ISerializable)value which would throw at runtime an InvalidCastException.

If other serialization frameworks would cast it with an as or is operator it would either return null or false (same with pattern matching).

@danmoseley
Copy link
Member

@Cisien this probably isn't the info you want to hear, but you see it's a tradeoff- even where we had serialization implemented in earlier builds of .NET Core 2.0 , it wasn't all working (and as you know it wasn't in 1.x at all).

If a type (even DirectoryInfo) that we don't serialize was heavily binary serialized in the .NET Framework world, and we get lots of feedback about that, we would certainly consider whether we should add it. But I recommend not using binary serialization for this -- and 3rd party serializers that do binary serialization probably should catch PNSE and try some other scheme.

@Cisien
Copy link
Author

Cisien commented Aug 23, 2017

Wouldn't it make for a smoother transition to dotnet core if the interface was simply omitted from the types that are no longer supported by binary serialization?

If the interface was removed, then the issue with Newtonsoft.Json wouldn't exist since they do check to see if the type is ISerializable before attempting to cast.

@ViktorHofer
Copy link
Member

If the interface was removed, then the issue with Newtonsoft.Json wouldn't exist since they do check to see if the type is ISerializable before attempting to cast.

I'm curious, can you show me where that happens in Newtonsoft.Json?

@Cisien
Copy link
Author

Cisien commented Aug 23, 2017

@ViktorHofer
Copy link
Member

Thanks!

@JamesNK
Copy link
Member

JamesNK commented Aug 23, 2017

No one has ever said ISerializable is only for binary serialization. DataContractSerializer uses it I believe. And it was the ASP.NET team that asked for Json.NET to support it many years ago.

Catching PlatformNotSupportedException like below will not work because by that point the JSON has already been read into the serializationInfo object and can't be used using the regular serialization code path. Also using exceptions for flow control will kill performance.

try
{
    value.GetObjectData(serializationInfo, Serializer._context);
}
catch (PlatformNotSupportedException) { }

@danmoseley
Copy link
Member

@Cisien I'm not familiar with the Newtonsoft.json code, but are you saying it walks the hierarchy of objects testing every object implements ISerializable and if any do not then it does not use binary serialization? Even if it does, testing for ISerializable is not sufficient to know whether a type is binary serializable.
I think it is necessary to control the set of types one is serializing, and it is a more limited set on .NET Core - or specify a different serialization method.

@Cisien
Copy link
Author

Cisien commented Aug 24, 2017

I'm not very familiar with newtonsoft.json either -- only what I've researched while tracking down this issue. @JamesNK would be the best person to answer that question.

@ViktorHofer
Copy link
Member

@Jameskn I talked offline with @stephentoub and it seems you are right.

// Stephen:

ISerializable is meant to only be meaningful if a type is marked as [Serializable]; it should not be relied on for types that aren’t. Other serializers can use [Serializable], ISerializable, etc., so it’s not just BinaryFormatter, but any such formatter needs to follow the same patterns, and should only be using an instance’s ISerializable implementation if that type is [Serializable]. The same goes for the [On*] attributes; they’re only meant to be meaningful if a type is marked [Serializable]… if it’s not, such attributes should be ignored by any formatter.
The MSDN docs even allude to this, though don’t explicitly state it as concretely as I did:
https://msdn.microsoft.com/en-us/library/system.runtime.serialization.iserializable(v=vs.110).aspx
“Any class that might be serialized must be marked with the SerializableAttribute. If a class needs to control its serialization process, it can implement the ISerializable interface.

@JamesNK
Copy link
Member

JamesNK commented Aug 29, 2017

So the issue is Json.NET is checking that a type implements ISerializable but not also checking for the SerialiazableAttribute?

@ViktorHofer
Copy link
Member

Correct :)

@ViktorHofer
Copy link
Member

I will close the issue as it seems James will implement the code suggestion on his side. If you disagree feel free to reopen.

@JamesNK
Copy link
Member

JamesNK commented Sep 6, 2017

I think I may do it. Unfortunately it is a breaking change so I'll put some thought into it first.

@BorisMisic
Copy link

I created test Asp.Net Core 2.0 project and added following line action inside Values controller
// GET api/values/5
[HttpGet("{id}")]
public string Get(int id)
{
IList<IDictionary<string, object>> l1 = new List<IDictionary<string, object>>();
IDictionary<string, object> d1 = new Dictionary<string, object>();
d1.Add("k1", id);
d1.Add("k2", DBNull.Value);
d1.Add("k3", "abcd");
d1.Add("K4", null);
l1.Add(d1);
string jsonRes = JsonConvert.SerializeObject(l1);
return jsonRes;
}

It raised following exception
System.PlatformNotSupportedException occurred
HResult=0x80131539
Message=Operation is not supported on this platform.
Source=System.Private.CoreLib
StackTrace:
at System.DBNull.GetObjectData(SerializationInfo info, StreamingContext context)
at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeISerializable(JsonWriter writer, ISerializable value, JsonISerializableContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty)
at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeDictionary(JsonWriter writer, IDictionary values, JsonDictionaryContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty)
at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeList(JsonWriter writer, IEnumerable values, JsonArrayContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty)
at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.Serialize(JsonWriter jsonWriter, Object value, Type objectType)
at Newtonsoft.Json.JsonSerializer.SerializeInternal(JsonWriter jsonWriter, Object value, Type objectType)
at Newtonsoft.Json.JsonConvert.SerializeObjectInternal(Object value, Type type, JsonSerializer jsonSerializer)
at AspNetCore20Test1.Controllers.ValuesController.Get(Int32 id) in c:\users\boris\documents\visual studio 2017\Projects\AspNetCore20Test1\AspNetCore20Test1\Controllers\ValuesController.cs:line 47
at Microsoft.Extensions.Internal.ObjectMethodExecutor.Execute(Object target, Object[] parameters)
at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.d__12.MoveNext()

I know that it worked in previous releases of hybrid .NET Framework and Core 1.0 and 1.1 projects. Could you fix this annoying issue? Thanks.

@ViktorHofer
Copy link
Member

Hi Boris,

DBNull serialization was added in the last servicing release of .NET Core 2.0. Please update to the latest version.

@BorisMisic
Copy link

Hi Viktor,

Where can I find the last servicing release of .NET Core 2.0?
I normally look at these 2 locations
https://www.microsoft.com/net/download/core https://www.nuget.org/packages/Microsoft.NETCore.App

@ViktorHofer
Copy link
Member

cc @danmosemsft how to obtain the latest servicing release?

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 2.1.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.IO bug tenet-compatibility Incompatibility with previous versions or .NET Framework
Projects
None yet
Development

No branches or pull requests

6 participants