Skip to content

Fix ssl_transport_security_test.cc when built against OpenSSL 1.0.2.#25843

Merged
matthewstevenson88 merged 6 commits intogrpc:masterfrom
matthewstevenson88:matthewstevenson88-openssl-102-fixes-v1
Mar 31, 2021
Merged

Fix ssl_transport_security_test.cc when built against OpenSSL 1.0.2.#25843
matthewstevenson88 merged 6 commits intogrpc:masterfrom
matthewstevenson88:matthewstevenson88-openssl-102-fixes-v1

Conversation

@matthewstevenson88
Copy link
Copy Markdown
Contributor

This PR fixes some of the errors found by PR #25770, namely it enables ssl_transport_security_test.cc to build and have its test pass when built against OpenSSL 1.0.2.

Note that there are still some tests that fail when built against OpenSSL 1.0.2.

Relevant issue: #24960

Copy link
Copy Markdown
Contributor

@veblush veblush left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix!

@veblush
Copy link
Copy Markdown
Contributor

veblush commented Mar 30, 2021

As a side note, //test/core/tsi:ssl_transport_security_test has been flaky for a while log. I don't think this PR changes it but can you take a quick look on that, too? (This has been flaky for months)

@matthewstevenson88
Copy link
Copy Markdown
Contributor Author

As a side note, //test/core/tsi:ssl_transport_security_test has been flaky for a while log. I don't think this PR changes it but can you take a quick look on that, too? (This has been flaky for months)

Sure, I'll take a look before submitting this PR. Is it always a timeout, like in the case of the Bazel RBE TSAN C/C++ failure?

@matthewstevenson88
Copy link
Copy Markdown
Contributor Author

Update: the ssl_tsi_test_do_round_trip_odd_buffer_size test is super slow (because it runs many many times), so we need to decrease the number of test runs when using TSAN. Otherwise, this test will timeout.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/security area/test release notes: no Indicates if PR should not be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants