MemoryStream should be serializable in .NET Core 3

[koszeggy]

Releated Issue: This one complains about broken WCF services but was closed as will not be fixed.

Now here is another issue if MemoryStream is not serializable: The .NET Framework serializes MemoryStream instances for embedded binary resources. Now the unit tests of my custom ResXResourceReader class are failing in .NET Core because the binary resources cannot be deserialized from .resx files anymore.

Note: Only the embedded resources are affected, the file references are returned as a MemoryStream without any issue.

This could be relevant for .NET Core 3 where the WinForms version of ResXResourceReader will be available again.

Instead of arguing that serializing a byte[] is more reasonable (which I basically agree with) wouldn't it just possible to reintroduce the [Serializable] attribute for MemoryStream? It is essentially a byte[] along with a few integers so nothing as dangerous as a delegate or something.

Note 2: ResourceManager is able to deserialize the affected resource from compiled resources as an UnmanagedMemoryStreamWrapper. Only the direct .resx processing fails
Edit: For embedded resources also the ResourceManager fails in .NET Core saying "Cannot read resources that depend on serialization.". Only the file references are returned as UnmanagedMemoryStreamWrapper (a generic binary file is usually added as byte[] while sound resources are added as MemoryStream).

Of course, I could use a prepared ISurrogateSelector in my deserializer but that would not cure the same issue in the system version. But I prefer compatibility over nasty hacks. Especially when the fix is that simple.

[stephentoub]

cc: @ViktorHofer, @ericstj

[ericstj]

the .NET Framework serializes MemoryStream instances for embedded binary resources.

Not exactly: we never used Binary Serialization for MemoryStream. Streams have a primitive representation in resources that just stores the raw backing data. At runtime we hand back an UnmanagedMemoryStream over the backing embedded bytes in the embedded resources: no copies. https://github.com/dotnet/coreclr/blob/6e1b160523db1c6e7d96aa3c9244a0baf8edb06a/src/System.Private.CoreLib/shared/System/Resources/ResourceReader.cs#L698-L725

We wouldn't want folks to rely more on binary serialization for this. Let alone the security implications of that technology, it would likely result in extra copies of the data to get back to a MemoryStream, at least one from the backing data to a managed array, perhaps more in the process. You'd also need to change designers to write that format to the resx file.

This scenario should be fixed in a source-compatible manner with dotnet/msbuild#4420 / dotnet/msbuild#2221. Have you tried this out?

[koszeggy]

Dear Eric,

Thank you for your reply. I'm afraid I wasn't detailed enough about the issue. I admit it's a bit complicated and even I was a bit confused and corrected my post once.

we never used Binary Serialization for MemoryStream.

Not exactly. We are talking about slightly different scenarios. But now I try to make it as clear as possible:

Scenario 1 (working):

I believe this is what you are talking about. If you add a linked binary resource it will be usually a byte[], which you can change to MemoryStream. Or, if you add a .wav file, the MemoryStream type will be picked by the designer automatically.

When you obtain this resource by GetStream an UnmanagedMemoryStream will be returned, which is essentially a wrapper around a fix memory area so no copy occurs. As this stream is not derived from MemoryStream the GetObject method returns an UnmanagedMemoryStreamWrapper, which can be cast to MemoryStream so the user who believes he/she created a MemoryStream resource can use it seamlessly.

This scenario is handled well both by Microsoft's ResourceManager and my ResXResourceManager/HybridResourceManager classes, which also have this copy prevention for streams.

Scenario 2 (the issue I'm talking about):

On the other hand, if you create an embedded MemoryStream resource, then a MemoryStream instance will be serialized by BinaryFormatter, as it is indicated by the used mimetype.

In .NET Core this resource cannot be deserialized anymore.

• Neither from compiled resources by Microsoft's ResourceManager: "Cannot read resources that depend on serialization."
• Nor from .resx file by MS ResXResourceReader in .NET Core 3 (or by my ResXResourceReader in .NET Core 2 or later) just because SerializableAttribute has been removed from MemoryStream. And though I can inject a special ISurrogateSelector for MemoryStreams as a workaround, this is still an issue for the MS resource readers/managers.

[ericstj]

Sure, if you're explicitly storing something as a MemoryStream type then that will no longer deserialize.

Neither from compiled resources by Microsoft's ResourceManager: "Cannot read resources that depend on serialization."

You can get further if you use ResourceManager with an assembly-based constructor. In .NETCore we minimize the use of BinaryFormatter except for cases where you've already trusted the code you're loading (eg: by loading an assembly). Even so, if you did that and got ResourceManager to deserialize your embedded type, you'd still fail on deserialization in the same way you mentioned for ResXResourceReader, since the MemoryStream type itself is not serializable.

In most cases it will be more efficient to store it as the stream primitive and only convert it to a MemoryStream if you require that type specifically. Even so, it might even be more efficient to just go from the UMS to an array so that you don't walk through growing a MemoryStream up to final size. If you own the code consuming the MemoryStream, I'd recommend this rather than storing as binary-formatted MemoryStream.

Do you have a user scenario that actually serializes something as a MemoryStream with binary formatted mimetype in ResX? Your sample showed a test case that did this, but how did you create that ResX file? I know @dreddy-work and @rainersigwald tried to examine all the cases where the resx and winforms designer would write common types and ensure that we had support for those types in .NETCore. There's an open-ended case where a user control might expose a localizable property of any type, but I don't recall any cases we saw of folks using MemoryStream. It's possible we missed something here.

@GrabYourPitchforks can probably better comment on the implications of adding [Serializable] to MemoryStream. In general we've opted to not bring back Serializable on types unless we had a strong compelling use case for keeping them. It's not only due to security implications, but also compatibility (since it limits future refactoring of the types). For example, suppose we wanted to fix MemoryStream to use a chunked/linked-list approach instead of a contiguous giant managed arrray? Doing so while maintaining binary-format compatibility would be quite costly, and in some cases might not be possible.

[koszeggy]

Do you have a user scenario that actually serializes something as a MemoryStream with binary formatted mimetype in ResX?

Apart from my test cases, no. And unless compatible format is enforced on saving I use my own binary serialization rather than BinaryFormatter. However, when both compatibleFormat and forceEmbeddedResources are true on saving, then file references will turn to embedded resources using BinaryFormatter if there are no applicable TypeConverters to use. I don't think it's a common use case for the users of my libraries, though.

Your sample showed a test case that did this, but how did you create that ResX file?

1. Add a new resource file to your project.
2. In the designer click Add Resource/Add Existing File... and select a .wav file
3. Under the Audio tab select the added file and in the Properties window set the Persistence property to Embedded in .resx
   

It's not only due to security implications, but also compatibility. [...] For example, suppose we wanted to fix MemoryStream to use a chunked/linked-list approach instead of a contiguous giant managed arrray?

Ok, let's go philosophical. :) I think that is not a problem either. That's why BinaryFormatter is sensitive to assembly versions by default (which can be either a blessing or a curse, matter of opinion). In this case an exception should be thrown that MemoryStream of the past could not be resolved indicating you should either use the same framework version, a custom ISurrogateSelector, or a SerializationBinder that maps to an IObjectReference that is able to instantiate the new version from old format, for example.

Btw, I basically agree that binary serialization is not the best idea for transferring data between different platforms and versions (that's the case also for resources).

But I also believe that removing the SerializableAttribute from almost every type is the other extreme and is basically a bad idea. Binary serialization is (was) the only option to create a bitwise snapshot of any object, or to create deep clones. For in-proc data transfer it is still nothing better. Yes, people often used it where an XML/JSON serialization of public members would have been better but I think the correct response to this is education and not to cripple an important tool. Why would we pour out the baby along with the bath water? Or more importantly: why the breaking changes if it does not solve anything in return?

[ericstj]

Thank you for the repro scenario. I'm able to produce this using the resource editor. I'll follow up with the folks who own the resource editor to see if we can get this written in a slightly different manner in the future that will avoid writing a serialized MemoryStream. Instead the resource editor should be writing as a byte-array and indicating it's a stream type, that way it gets stored using the stream primitive same as it would if it were an external file.

That won't fix existing usage. For assemblies out there that already contain a serialized a MemoryStream they'll fail to deserialize on .NETCore. So far the data we've gathered from assemblies we've analyzed hasn't shown instances of this, but we're open to more feedback if folks are blocked.

[koszeggy]

A little addendum for the newly released .NET Core 3.0, which works differently in some cases compared to .NET Core 2.x:

1. MemoryStream as file reference:

• In .NET Core 2.x retrieving a MemoryStream resource (which was not embedded but added as a ResXFileRef) worked without a problem: an UnmanagedMemoryStream instance was returned.
• In .NET Core 3.0 this causes an exception:

System.MissingMethodException : Constructor on type 'System.IO.MemoryStream' not found.
   at System.RuntimeType.CreateInstanceImpl(BindingFlags bindingAttr, Binder binder, Object[] args, CultureInfo culture)
   at System.Activator.CreateInstance(Type type, BindingFlags bindingAttr, Binder binder, Object[] args, CultureInfo culture, Object[] activationAttributes)
   at System.Activator.CreateInstance(Type type, Object[] args)
   at System.Resources.Extensions.DeserializingResourceReader.DeserializeObject(Int32 typeIndex)
   at System.Resources.Extensions.DeserializingResourceReader._LoadObjectV2(Int32 pos, ResourceTypeCode& typeCode)
   at System.Resources.Extensions.DeserializingResourceReader.LoadObjectV2(Int32 pos, ResourceTypeCode& typeCode)
   at System.Resources.Extensions.DeserializingResourceReader.LoadObject(Int32 pos, ResourceTypeCode& typeCode)
   at System.Resources.Extensions.RuntimeResourceSet.GetObject(String key, Boolean ignoreCase, Boolean isString)
   at System.Resources.Extensions.RuntimeResourceSet.GetObject(String key, Boolean ignoreCase)
   at System.Resources.ResourceManager.GetObject(String name, CultureInfo culture, Boolean wrapUnmanagedMemStream)
   at System.Resources.ResourceManager.GetStream(String name, CultureInfo culture)

2. Binary serialized resources:

• In .NET Core 2.x these resources could not be deserialized: System.NotSupportedException : Cannot read resources that depend on serialization.
• In .NET Core 3.0 retrieveng these resources works again, which is a positive thing. To be more precise, they almost always work. Common types (such as Color, Bitmap, Icon) and even less common ones (such as the obscure ImageListStreamer of WinForms) work perfectly, except that poor MemoryStream (as an embedded resource) because it is still not marked as serializable.

3. A small incompatibility notice:

• In .NET Core 2.x the System.Private.CoreLib assembly was decorated by the NeutralResourcesLanguage("en-US") attribute (just like the mscorlib.dll in the .NET Framework)
• However, in .NET 3.0 this attribute has been removed so from now on the default language of the neutral resources is represented by the invariant culture. This is just a subtle difference but still broke my test cases because ResourceManager.GetNeutralResourcesLanguage(typeof(object).Assembly) returns a different result now.

[ericstj]

Can you share a snippet of your resx for issue 1? I tested this previously and it was working, but your callstack indicates that MSBuild is writing this intead as an activator resource instead of stream primitive. @rainersigwald that might be a regression.

Issue 2 is clear and what we've been discussing here. I think for this we'd really need the security & IO folks to weigh in on security risk of adding back serializable to MemoryStream, and IO folks to weigh in on the compat burden of doing so. @GrabYourPitchforks and @JeremyKuhne can you share your opinion?

For issue 3 I opened https://github.com/dotnet/coreclr/issues/26951.

[rainersigwald]

Can you share a snippet of your resx for issue 1? I tested this previously and it was working, but your callstack indicates that MSBuild is writing this intead as an activator resource instead of stream primitive. @rainersigwald that might be a regression.

@koszeggy Also please include a description of your build process--are you using dotnet build or msbuild.exe, and so on.

[jkotas]

adding back serializable to MemoryStream,

This was discussed in https://github.com/dotnet/coreclr/issues/17460

[GrabYourPitchforks]

From a security perspective, if we added [Serializable] back on MemoryStream we'd want a custom deserialization ctor so that the serialized payload can't blindly splat data all over the private fields of the type. This also allows us to continue to modify the private implementation details of MemoryStream as we see fit, but it has a few compat consequences:

• Instances won't round-trip fully. For example, if you serialized a non-writeable MemoryStream instance, you'd probably get back a writeable instance after deserialization because we'd ignore fields that aren't directly related to carrying the byte[] that represents the data.

• If a user has serialized a custom MemoryStream-derived type, that type wouldn't properly deserialize because its ctor wouldn't call the MemoryStream deserialization ctor.

[koszeggy]

@ericstj, @rainersigwald:
In a minimal repro project I was not able to reproduce the issue so I couldn't determine under what constellation the error occurs. Hence I created a GetStreamError branch in my repo. Only the netcoreapp3.0 target is enabled now, projects are built by dotnet build.

Just execute dotnet test from the KGySoft.CoreLibraries.UnitTest folder to see that GetStream test fails:

Test run for D:\Nyelvek\CSharp\KGySoft\KGySoft.CoreLibraries\KGySoft.CoreLibraries.UnitTest\bin\Debug\netcoreapp3.0\KGySoft.CoreLi
braries.UnitTest.dll(.NETCoreApp,Version=v3.0)
Microsoft (R) Test Execution Command Line Tool Version 16.3.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...

A total of 1 test files matched the specified pattern.

  X GetStream [4ms]
  Error Message:
   System.MissingMethodException : Constructor on type 'System.IO.MemoryStream' not found.
  Stack Trace:
     at System.RuntimeType.CreateInstanceImpl(BindingFlags bindingAttr, Binder binder, Object[] args, CultureInfo culture)
   at System.Activator.CreateInstance(Type type, BindingFlags bindingAttr, Binder binder, Object[] args, CultureInfo culture, Obje
ct[] activationAttributes)
   at System.Activator.CreateInstance(Type type, Object[] args)
   at System.Resources.Extensions.DeserializingResourceReader.DeserializeObject(Int32 typeIndex)
   at System.Resources.Extensions.DeserializingResourceReader._LoadObjectV2(Int32 pos, ResourceTypeCode& typeCode)
   at System.Resources.Extensions.DeserializingResourceReader.LoadObjectV2(Int32 pos, ResourceTypeCode& typeCode)
   at System.Resources.Extensions.DeserializingResourceReader.LoadObject(Int32 pos, ResourceTypeCode& typeCode)
   at System.Resources.Extensions.RuntimeResourceSet.GetObject(String key, Boolean ignoreCase, Boolean isString)
   at System.Resources.Extensions.RuntimeResourceSet.GetObject(String key, Boolean ignoreCase)
   at System.Resources.ResourceManager.GetObject(String name, CultureInfo culture, Boolean wrapUnmanagedMemStream)
   at System.Resources.ResourceManager.GetStream(String name, CultureInfo culture)
   at KGySoft.CoreLibraries.UnitTests.Resources.ResXResourceManagerTest.GetStream() in D:\Nyelvek\CSharp\KGySoft\KGySoft.CoreLibra
ries\KGySoft.CoreLibraries.UnitTest\UnitTests\Resources\ResXResourceManagerTest.cs:line 232


Test Run Failed.
Total tests: 305
     Passed: 304
     Failed: 1
 Total time: 3,8651 Seconds

[koszeggy]

@jkotas: I also referenced #10106 in my opening post. I opened this issue because I found another issue where this is a problem. The main reasoning in #10106 was that serializing a MemoryStream is simply not reasonable as it is just an expensive way of serializing a byte[].

While this is true please also take the compatibility issues into consideration. That's why I've written in the OP:

Instead of arguing that serializing a byte[] is more reasonable (which I basically agree with) wouldn't it just possible to reintroduce the [Serializable] attribute for MemoryStream? It is essentially a byte[] along with a few integers so nothing as dangerous as a delegate or something.

[koszeggy]

@GrabYourPitchforks:
I don't really get why adding serialization constructor would be necessary. Now irrelevant fields, such as _lastReadTask are marked by [NonSerialized] (at least in the Framework version). And now a read-only instance remains read-only after cloning.

And if someone wants to create a serializable derived type without including the byte array buffer and other things from the base, then they still can add ISerialiable and the special constructor as per their preferences. But I quickly checked the derived types in the .NET Framework and none of them are serializable, so no existing types are affected.

[jkotas]

please also take the compatibility issues into consideration

@koszeggy We are taking multiple factors into consideration:

• MemoryStream should not need to be serializable to make your scenario work. Thank you for providing a repro! It will help us to get to the bottom of this.
• We are done with .NET Framework compatibility project. There are always going edge cases where .NET Core could have been more compatible with .NET Framework, but the value of fixing these is low and decreasing over time.
• Binary serialization is a legacy feature that we are not planning to evolve.
• Binary serialization is full of non-obvious security gotchas. @GrabYourPitchforks explored what we might want to do to mitigate the risk for serialiable MemoryStream.

[ericstj]

@koszeggy, I really appreciate the link and the repro, but I'm uncertain about the license on the repository you linked. Do you think it would be possible to share an isolated repro? I did try to ad-hoc test the scenario you mentioned and found it was working for me. From the callstack you shared it really looks like you're hitting dotnet/msbuild#4581 which was fixed with dotnet/msbuild#4607 which should be in the GA release for .NETCore 3.0. Make sure you have updated the SDK and do a clean build to see the fix. Here's my adhoc testing of the scenario: https://github.com/ericstj/repros/tree/master/testMemStreamResXFileRef

[GrabYourPitchforks]

I don't really get why adding serialization constructor would be necessary.

If you want to read a payload serialized by .NET Framework, that payload will contain a bunch of irrelevant fields aside from just the byte[] itself. The deserialization ctor would validate the offset and length against the incoming array, throwing away all other unnecessary fields. Additionally, .NET Core's MemoryStream would need to implement ISerializable.GetObjectData if the requirement is to produce data that can be consumed by .NET Framework, as it would need to provide default values for those fields.

[koszeggy]

@ericstj,

• License: The CC BY-ND 4.0 license allows you everything but distributing a modified material as your product. Feel free to use, share and even modify it.
• Isolated repro: As I mentioned a similar minimal repro project worked also for me. I tried many things that came to my mind without any clue so far. I will try to go for it again as soon as I can isolate some time for it, though. Until then I would appreciate if you could confirm if you get the error when you clone the linked branch of the repo. If dotnet test is green for you, then it's obviously something with my environment (and I will do apologize for the false alarm).
• SDK version: See the dotnet --info dump below. I use the latest SDK and the minimal repro attempt worked also for me.

.NET Core SDK (reflecting any global.json):
 Version:   3.0.100
 Commit:    04339c3a26

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.17763
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\3.0.100\

Host (useful for support):
  Version: 3.0.0
  Commit:  7d57652f33

.NET Core SDKs installed:
  2.1.801 [C:\Program Files\dotnet\sdk]
  2.2.401 [C:\Program Files\dotnet\sdk]
  3.0.100 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.All 2.1.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 1.0.16 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

[koszeggy]

@GrabYourPitchforks,

To be frank, I still not get it. Here are the fields of the MemoryStream:

I believe all of them are needed (except the one marked by [NonSerialized]) for retrieving an exact and functionally equivalent duplicate. And all of the base Stream fields are also decorated by [NonSerialized] (both in .NET Framework and .NET Core 2.x).

• Which field(s) do you consider unnecessary?
• Why would you break compatibility by storing a different object graph in SerializationInfo? That would not solve the issue we are brainstorming about.
• Since when is it so important to validate binary deserialized fields? Btw, you can add this functionality to an [OnDeserialized] method or by implementing IDeserializationCallback. IMHO, this could be a relevant question if we designed this class now but this is more like reinventing the wheel.

Of course, these fields can be unnecessary in a derived class (eg. UnmanagedMemoryStreamWrapper) but

• I haven't found any serializable derived type in the framework
• The author of a derived serializable class still can add ISerializable implementation and the constructor to produce a more stripped payload.

[ericstj]

I was able to reproduce the problem you were seeing in your unit test. Here's an isolated repro: https://github.com/ericstj/repros/tree/master/testMemStreamResXFileRef
Filed: dotnet/msbuild#4773

Let this issue only track the discussion around binary serialization.

[GrabYourPitchforks]

I am not interested in preserving full fidelity of private fields being serialized between .NET Core and .NET Framework, both for forward-compatibility purposes and for security purposes. I'm willing to allow just the raw byte[] data itself to be serialized, but anything beyond that I'm forcibly going to pushing back on.

[mageshsankar]

Please suggest solution for this
https://social.msdn.microsoft.com/Forums/vstudio/en-US/080878ce-82ef-46cd-bffb-815210fd704f/alternate-way-to-serialize-data-using-resource-manager?forum=netfxbcl

[GrabYourPitchforks]

The current plan of record is to not mark any more inbox .NET types as [Serializable], including types that were marked as such in Full Framework.

@ericstj Should this work item be instead used for tracking an arbitrary binary blob storage mechanism in .resx files? Otherwise I'll close the issue as a dupe of #10106.

[koszeggy]

From my side the issue can be closed.

In the meantime I implemented the needed workarounds to provide compatibility between the Framework and .NET Core. It's just a bit odd that one needs 3rd party solutions for it.

For anyone who struggles with such issues:

• The ResXResourceManager can be used to access resources directly from .resx files.
• If you have a stream serialized by BinaryFormatter in .NET Framework, which cannot be deserialized in .NET Core, the CustomSerializerSurrogateSelector, WeakAssemblySerializationBinder or ForwardedTypesSerializationBinder types may help you. Though using a binary serialization is typically not recommended across platforms...
• If you really need to use binary serialization in a new project for some reason, then instead of BinaryFormatter you can use the BinarySerializationFormatter, which is more robust and produces a much more compact stream.

The project is open source and is available on GitHub