-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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 platform runtime check for ProtectedData #80158
Conversation
Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones Issue DetailsThis changes S.S.C.ProtectedData to use runtime checks instead of TPI to determine if that platform is Windows or not. This fixes an issue where Since .NET Standard is being supported again, some refactoring that have since occurred need to be undone, or accounted for, now that we are no longer using Lastly, this adds some unit tests to make sure we are correctly throwing PNSE on non-Windows. Contributes to #78875
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks a lot like a change I started before I hibernated through all of December 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot 👍
...System.Security.Cryptography.ProtectedData/src/System/Security/Cryptography/ProtectedData.cs
Show resolved
Hide resolved
...System.Security.Cryptography.ProtectedData/src/System/Security/Cryptography/ProtectedData.cs
Show resolved
Hide resolved
/backport to release/7.0 |
Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3840895989 |
@vcsjones backporting to release/7.0 failed, the patch most likely resulted in conflicts: $ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Use platform runtime check for ProtectedData
Using index info to reconstruct a base tree...
M src/libraries/System.Security.Cryptography.ProtectedData/src/System.Security.Cryptography.ProtectedData.csproj
M src/libraries/System.Security.Cryptography.ProtectedData/src/System/Security/Cryptography/ProtectedData.cs
Falling back to patching base and 3-way merge...
Auto-merging src/libraries/System.Security.Cryptography.ProtectedData/src/System/Security/Cryptography/ProtectedData.cs
CONFLICT (content): Merge conflict in src/libraries/System.Security.Cryptography.ProtectedData/src/System/Security/Cryptography/ProtectedData.cs
Auto-merging src/libraries/System.Security.Cryptography.ProtectedData/src/System.Security.Cryptography.ProtectedData.csproj
CONFLICT (content): Merge conflict in src/libraries/System.Security.Cryptography.ProtectedData/src/System.Security.Cryptography.ProtectedData.csproj
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Use platform runtime check for ProtectedData
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128 Please backport manually! |
@vcsjones an error occurred while backporting to release/7.0, please check the run log for details! Error: git am failed, most likely due to a merge conflict. |
This changes S.S.C.ProtectedData to use runtime checks instead of TPI to determine if that platform is Windows or not.
This fixes an issue where
ProtectedData
does not work on UWP in Windows because UWP is picking up the .NET Standard version. We removed .NET Standard 2.0 platform specific target for Windows, which previously allowed this to work in UWP. Since NS2.0-windows is no longer valid, this moves ProtectedData to use a runtime check for all implementations.Since .NET Standard is being supported again, some refactoring that have since occurred need to be undone, or accounted for, now that we are no longer using
GeneratePlatformNotSupportedAssemblyMessage
for NS2.0. Some of the Common/* sources have been refactored to work off ofSpan<T>
. So for .NETStandard we take a dependency onSystem.Memory
. We can avoid this if we want to create non-span local copies of those common helpers. I have not deeply investigated how feasible this is.Lastly, this adds some unit tests to make sure we are correctly throwing PNSE on non-Windows.
Contributes to #78875