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

test/sslapitest.c: Add check for SSL_CTX_new #17739

Closed
wants to merge 1 commit into from

Conversation

JiangJias
Copy link
Contributor

As the potential failure of the memory allocation, it should
be better to check the return value of SSL_CTX_new() and return
error if fails, like SSL_CTX_new_ex().

Signed-off-by: Jiasheng Jiang jiasheng@iscas.ac.cn

Checklist
  • documentation is added or updated
  • tests are added or updated

As the potential failure of the memory allocation, it should
be better to check the return value of SSL_CTX_new() and return
error if fails, like SSL_CTX_new_ex().

Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
@paulidale paulidale added approval: review pending This pull request needs review by a committer branch: 3.0 Merge to openssl-3.0 branch branch: master Merge to master branch triaged: bug The issue/pr is/fixes a bug labels Feb 21, 2022
@mattcaswell mattcaswell added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Feb 21, 2022
@openssl-machine openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Feb 22, 2022
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@paulidale
Copy link
Contributor

Merged to both, thanks for the fix.

@paulidale paulidale closed this Feb 22, 2022
openssl-machine pushed a commit that referenced this pull request Feb 22, 2022
As the potential failure of the memory allocation, it should
be better to check the return value of SSL_CTX_new() and return
error if fails, like SSL_CTX_new_ex().

Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #17739)
openssl-machine pushed a commit that referenced this pull request Feb 22, 2022
As the potential failure of the memory allocation, it should
be better to check the return value of SSL_CTX_new() and return
error if fails, like SSL_CTX_new_ex().

Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #17739)

(cherry picked from commit b0317df)
@mattcaswell
Copy link
Member

Somehow we failed to notice the failing test in the CI related to this PR! This broke the build :-(

Fix coming up...

mattcaswell added a commit to mattcaswell/openssl that referenced this pull request Feb 23, 2022
The SNI test in test_cert_cb_int() was always failing because it used
SSL_CTX_new() instead of SSL_CTX_new_ex() and was therefore not using the
correct libctx. PR openssl#17739 amended the test to check the return value from
SSL_CTX_new() which made the failure obvious.

Fixes openssl#17757
openssl-machine pushed a commit that referenced this pull request Feb 23, 2022
The SNI test in test_cert_cb_int() was always failing because it used
SSL_CTX_new() instead of SSL_CTX_new_ex() and was therefore not using the
correct libctx. PR #17739 amended the test to check the return value from
SSL_CTX_new() which made the failure obvious.

Fixes #17757

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from #17758)

(cherry picked from commit 7e1eda4)
openssl-machine pushed a commit that referenced this pull request Feb 23, 2022
The SNI test in test_cert_cb_int() was always failing because it used
SSL_CTX_new() instead of SSL_CTX_new_ex() and was therefore not using the
correct libctx. PR #17739 amended the test to check the return value from
SSL_CTX_new() which made the failure obvious.

Fixes #17757

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from #17758)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch branch: 3.0 Merge to openssl-3.0 branch triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants