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

Serialize not filtering properties correctly in .NET Standard #246

Closed
Sten-V opened this issue Mar 27, 2024 · 6 comments · Fixed by #247
Closed

Serialize not filtering properties correctly in .NET Standard #246

Sten-V opened this issue Mar 27, 2024 · 6 comments · Fixed by #247
Labels

Comments

@Sten-V
Copy link

Sten-V commented Mar 27, 2024

YAXLib version: 4.2.0

Framework version: .NET Standard 2.0 (in runtime .NET Framework 4.8) and .NET Standard 2.1 (in Unity version of Mono in Unity 2022.3.22)

This code

var intArray = new[] { 1, 3, 5, 9 };
var serializer = new YAXSerializer<int[]>();
var xml = serializer.Serialize(intArray);
var des = serializer.Deserialize(xml);

First results in XML like

<Array1OfInt32 xmlns:yaxlib="http://www.sinairv.com/yaxlib/">
  <Length>4</Length>
  <LongLength>4</LongLength>
  <Rank>1</Rank>
  <SyncRoot yaxlib:realtype="System.Int32[]" />
  <IsReadOnly>false</IsReadOnly>
  <IsFixedSize>true</IsFixedSize>
  <IsSynchronized>false</IsSynchronized>
  <Int32>1</Int32>
  <Int32>3</Int32>
  <Int32>5</Int32>
  <Int32>9</Int32>
</Array1OfInt32>

and then throws in Deserialize() with
YAXLib.Exceptions.YAXBadlyFormedInput: 'The format of the value specified for the property 'IsReadOnly' is not proper: 'false'.'

Adding the SerializerOptions
ExceptionHandlingPolicies = YAXExceptionHandlingPolicies.DoNotThrow

Keeps Deserialize() from throwing but the deserialized result variable contains seven ints instead of the expected four.

The issue seems to have been introduced in c0bb561

@Sten-V Sten-V added the bug label Mar 27, 2024
@axunonb
Copy link
Collaborator

axunonb commented Mar 27, 2024

@prettyv Could you please have another look at these changes which seam to be the cause for this issue.
@Sten-V Did you try, whether reverting the changes resolves the issue?

@Sten-V
Copy link
Author

Sten-V commented Mar 29, 2024

Yes, reverting that commit resolves the issue.
Before that commit, in the affected method:
assemblyName = "mscorlib.dll"
and after
assemblyName = "mscorlib"

and the pattern searched for after does not match "mscorlib." (dot in the end differs).

@Sten-V
Copy link
Author

Sten-V commented Mar 29, 2024

Note that there's a differences between .NET (Core) and Framework.
My previous comment was executed in .NET Framework 4.8
In .NET 8
Before that commit, in the affected method:
assemblyName = "System.Private.CoreLib.dll"
and after
assemblyName = "System.Private.CoreLib"

@axunonb
Copy link
Collaborator

axunonb commented Mar 29, 2024

The unit tests for the sample code above succeed. So it looks like your environment is different.

But still, the dot after "mscorlib." in

assemblyName.StartsWith("mscorlib.", StringComparison.OrdinalIgnoreCase) ||

should be removed, meaning the comparison should be the same as here:
return assemblyName.Equals("mscorlib", StringComparison.OrdinalIgnoreCase)

Although assembly names may contain dots, this does not apply for "mscorlib.dll".

Never mind your environment: Do I understand correctly that just removing the dot after "mscorlib" would fix the issue?

@Sten-V
Copy link
Author

Sten-V commented Mar 29, 2024

Yes, removing the dot should do the trick.

Given a bit of tweaking you'll probably see this error in a bunch of unit tests too.
I did a POC here Sten-V@8b03c1d

  • Starting the test suite in Visual Studio/ReSharper they flat out refused to run in the configured test-lib target net461 ("Inconclusive: Test has not run"), note that this does not fail the test run.
  • Upgrading NUnit3TestAdapter to latest version had the tests running the net461 test-lib, now with an actual success. I assume that's because the test-lib 4.6.1 will load the yax-lib 4.6.1, since it's a neat match
  • Removing all targets except netstandard2.0 in yax-lib force the 4.6.1 test-run to load the netstandard 2.0 yax-lib, now we see errors, but not the ones we're looking for. As you can read here https://learn.microsoft.com/en-us/dotnet/standard/net-standard?tabs=net-standard-2-0 NET Framework 4.6.1 is not a great match for NET Standard so I bumped test-lib to the recommended version 4.7.2

After this you'll most likely see tests failing in the same manner originally stated.

@axunonb
Copy link
Collaborator

axunonb commented Mar 30, 2024

Okay, very good. There will be a new release in 1-2 days.
Yes, I noticed the NUnit packages must be updated. Not a big deal though. Migration to NUnit 4 will bring a bigger workload.
Thanks for your cooperation.

axunonb added a commit to axunonb/YAXLib that referenced this issue Apr 1, 2024
Correct filtering for properties that are part of the .NET framework, when the runtime framework is not the same as the YAXLib target framework (e.g. reference to NetStandard2.0 from NET481).

Update unit test packages
* Nunit 3.13.3
* NUinit3TestAdapter 4.3.1
* FluentAssertions 6.8.0
* Microsoft.NET.Test.Sdk 17.4.1
axunonb added a commit that referenced this issue Apr 1, 2024
Closes #246: Correct filtering for properties that are part of the .NET framework, when the runtime framework is not the same as the YAXLib target framework (e.g. reference to NetStandard2.0 from NET481).

Update unit test packages
    Nunit 3.13.3
    NUinit3TestAdapter 4.3.1
    FluentAssertions 6.8.0
    Microsoft.NET.Test.Sdk 17.4.1

Convert NUnit Classic Assert to the Constraint model,  Add package reference NUnit.Analyzers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants