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

Fixing Issue #8 #9

Merged
merged 3 commits into from
Aug 30, 2018
Merged

Fixing Issue #8 #9

merged 3 commits into from
Aug 30, 2018

Conversation

s-bauer
Copy link
Contributor

@s-bauer s-bauer commented Aug 30, 2018

The Unity.RegistrationByConvention assembly is marked as SecurityTransparent, meaning I cannot add the SecurityCriticalAttribute at the GetObjectData method (or at least it has no effect).

For .NET Framework there is a ISafeSerializationData Interface with which the Serialization can be handled without needing to deal with SecurityPermissions. This is however not supported in .NET Core so I just excluded it using compiler flag.

I also added a Limited PermissionSet test which is going to run the Serialization/Deserialization process with a very limited permission set.

In addition I fixed the test, so they are executed for all .NET versions. Thanks to @GreenKn1ght and his commit GreenKn1ght@538f3e6

Let me know if you have any concerns or questions!

Copy link

@GreenKn1ght GreenKn1ght left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to merge.

@ENikS
Copy link
Contributor

ENikS commented Aug 30, 2018

@s-bauer
You are creating new exception. Would going back and using old exception solve the problem?
It would probably be better legacy support...

@s-bauer
Copy link
Contributor Author

s-bauer commented Aug 30, 2018

@ENikS Sorry, but I don't quite understand what you mean

@ENikS ENikS merged commit 2890346 into unitycontainer:master Aug 30, 2018
@s-bauer
Copy link
Contributor Author

s-bauer commented Aug 30, 2018

@ENikS Ok, so I'm still using the same exception, just a different way of serializing and deserializing the exception. The other solution would be to either remove the GetObjectData completely or remove the SecurityTransparent assembly attribute. I think both of them would cause more trouble than the current solution.

@ENikS
Copy link
Contributor

ENikS commented Aug 30, 2018

Thank you for your help. Perhaps you could take look at other issues?

@s-bauer
Copy link
Contributor Author

s-bauer commented Aug 30, 2018

@ENikS Sure, now I'm already familiar with the project. You can expect several PRs in the future!

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

Successfully merging this pull request may close these issues.

None yet

4 participants