ProtectedData: add DataProtectionScope parameter check #30726
Conversation
|
Thanks, but why? This check doesn't exist on the .NET Framework as far as I can tell, so this is introducing an exception that wasn't ever there before. |
|
cc: @bartonjs |
|
During the import ProtectedData type from CoreFX to Mono I faced a couple of failing unit test which check this parameter and expect ArgumentException on wrong value. There is no exactly the same check in .Net Framework but there are similar ones in other places (like here). CoreFX also has similar validations. So I thought it'd be acceptable for you. |
|
It sounds like the bug is actually in Mono, doing extra checks and throwing for something that the .NET Framework doesn't check or throw on. All the .NET Framework (and corefx) care about is whether the value is LocalMachine or not: |
It's definitely in a grey area. If someone wrote If we added more enum values in the future then their code would eventually encounter a behavioral difference. If I had expectation of this API changing within the next 5-10 years then I'd say that adding the "don't give me unmapped states" check was reasonable future-proofing. Since it seems pretty well done and set, I think that adding an exception in CoreFX that isn't present in NetFX is just a source of aggravation during porting. So I don't know if I'd say that the "bug" is in Mono over NetFX, but I think that in this specific case the better answer is that Mono relax their tests instead of us constrain CoreFX. |
|
@MaximLipnin, thanks for the interest in changing this. Given the feedback, I'm going to close this. |
|
@stephentoub @bartonjs Thank you for the clarification! I will resolve it on Mono side. |
As part of #7589 - CoreFX import only for Windows platform (no support for non-Windows platforms, see https://github.com/dotnet/corefx/issues/22510) - left Mono managed implementation for non-Windows platforms - removed DataProtectionScope parameter value check (no such a check in CoreFX and NET Framework, see dotnet/corefx#30726) and couple of related unit tests - added CoreFX xunit tests
Adds DataProtectionScope parameter value check to Protect and Unprotect methods.