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

Use a better serialization/deserialization mechanism for data discovery #1462

Open
Evangelink opened this issue Dec 14, 2022 · 6 comments
Open
Milestone

Comments

@Evangelink
Copy link
Member

Evangelink commented Dec 14, 2022

Describe the bug

Today, MSTest uses the DataContractJsonSerializer serializer in order to perform the serialization and deserialization of the data during discovery and execution.

The problem with current solution is that it requires custom objects to be marked with DataContractAttribute/DataMemberAttribute which isn't what user expects to do or to be doing for the sake of test.

At the moment, the BinaryFormatter seems to be the best option but it's not ideal as this formatter has a lot of security concerns associated (see https://learn.microsoft.com/dotnet/api/system.runtime.serialization.formatters.binary.binaryformatter#remarks)

Additional context

Related issues:

@poizan42
Copy link

Is there really any issue with using BinaryFormatter here? The data doesn't cross a security boundary, does it?

@Evangelink
Copy link
Member Author

Technically I don't see much issues but we would be flagged by internal MS compliance checks + there are many discussions toward removing this formatter in some new version of .NET (nothing acted) so I am not sure if best is to be using that and wait for some "break" to change mechanism.

@jnyrup
Copy link
Contributor

jnyrup commented May 29, 2023

Given BinaryFormatter Obsoletion Strategy it doesn't seem that using BinaryFormatter would be a good path forward.

@mungojam
Copy link

mungojam commented Jul 3, 2023

How about switching to System.Text.Json based serialization?

@Evangelink
Copy link
Member Author

I would need to make some tests to see how many issues this would fix. It might also be problematic for .NET fx users.

Having some more experience with mstest, I think that there is no silver bullet solution and I would probably need to check if the data can be serialized (or to be exact if serializing/deserializing data gives me the same data as input) in which case we can "expand" the data otherwise we should not expand and so don't try to serialize/deserialize data.

@mungojam
Copy link

mungojam commented Jul 3, 2023

It might also be problematic for .NET fx users.

To answer that specific thought, it's supported in .net standard 2.0 and v4.6.2

https://www.nuget.org/packages/System.Text.Json/

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants