Modify RS256Algorithm to be xplat friendly #118
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Use the base class `RSA` for `RS256Algorithm`, which allows use on platforms like macOS and Linux, which won't have `RSACryptoServiceProvider`.
|
Code looks good to me at first glance. I also don't know why that other code was there. I haven't done much RSA256 crypto myself. |
|
We need few basic (smoke) tests to proceed. Meanwhile is it possible somehow test both platforms implementation? Or AppVeyor will compile and run one or another but not both? |
|
btw by some reason one of AppVeyor checks has stuck and hasn't changed since then |
|
I believe it's possible to build on multiple platforms using AppVeyor's build matrix feature, although I haven't tried it. |
|
It seems here's what we need: Build configuration#Build matrix |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use the base class
RSAforRS256Algorithm, which allows use on platforms like macOS and Linux, which won't haveRSACryptoServiceProvider.This is a very much "works on my machine" build and probably shouldn't be merged as-is. I notably have not tested this on net35.
Particularly, further research is needed as to why the
CspParametersand creating a newRSACryptoServiceProviderwas needed in the old code. I don't know anything about Windows crypto, so I'm not much help here.Fixes #117