-
Notifications
You must be signed in to change notification settings - Fork 312
Add SSL_set1_groups to NativeCrypto. #1443
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
Conversation
| } | ||
|
|
||
| @Test | ||
| public void setCurveList_unsupportedCurvesOrInvalid_throwsIllegalArgumentException() |
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.
I'm not sure this is a good test. Does this mean that if BoringSSL adds any of those this test fails? Or should they never be added?
In principle, you shouldn't test someone elses API which could change (though with BoringSSL and Conscrypt this anyhow might be a special case)
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.
You are right. I've removed the "unsupported" test cases, and only left the "invalid" ones.
tholenst
left a comment
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.
As you suggested offline, consider to use SSL_set1_groups instead.
SSL_set1_groups is simpler.
Done. PTAL. |
int and jint may not have the same size, so doing this makes sure that it always works.
This will be needed to add support for SSLParams.setNamedGroups,
which is needed to support Post-quantum algorithms, see https://openjdk.org/jeps/527.