wasphin commented on code in PR #3657:
URL: https://github.com/apache/brpc/pull/3657#discussion_r4436257302


##########
test/brpc_ssl_unittest.cpp:
##########
@@ -88,6 +90,75 @@ class SSLTest : public ::testing::Test{
     virtual void TearDown() {};
 };
 
+#ifndef USE_MESALINK
+static void MarkCertificateFreed(void*, void* ptr, CRYPTO_EX_DATA*,
+                                 int, long, void*) {
+    if (ptr != nullptr) {
+        *static_cast<bool*>(ptr) = true;
+    }
+}
+
+TEST_F(SSLTest, diagnostic_print_releases_peer_certificate) {
+    bool certificate_freed = false;
+    std::unique_ptr<SSL_CTX, decltype(&SSL_CTX_free)> server_ctx(
+        SSL_CTX_new(SSLv23_method()), SSL_CTX_free);
+    std::unique_ptr<SSL_CTX, decltype(&SSL_CTX_free)> client_ctx(
+        SSL_CTX_new(SSLv23_method()), SSL_CTX_free);
+    ASSERT_TRUE(server_ctx);
+    ASSERT_TRUE(client_ctx);
+    ASSERT_EQ(1, SSL_CTX_use_certificate_chain_file(server_ctx.get(), 
"cert1.crt"));
+    ASSERT_EQ(1, SSL_CTX_use_PrivateKey_file(
+        server_ctx.get(), "cert1.key", SSL_FILETYPE_PEM));
+    SSL_CTX_set_session_cache_mode(server_ctx.get(), SSL_SESS_CACHE_OFF);
+    SSL_CTX_set_session_cache_mode(client_ctx.get(), SSL_SESS_CACHE_OFF);
+    std::unique_ptr<SSL, decltype(&SSL_free)> server(
+        SSL_new(server_ctx.get()), SSL_free);
+    std::unique_ptr<SSL, decltype(&SSL_free)> client(
+        SSL_new(client_ctx.get()), SSL_free);
+    ASSERT_TRUE(server);
+    ASSERT_TRUE(client);
+    BIO* server_bio = nullptr;
+    BIO* client_bio = nullptr;
+    ASSERT_EQ(1, BIO_new_bio_pair(&server_bio, 0, &client_bio, 0));
+    SSL_set_bio(server.get(), server_bio, server_bio);
+    SSL_set_bio(client.get(), client_bio, client_bio);
+    SSL_set_accept_state(server.get());
+    SSL_set_connect_state(client.get());
+    for (int i = 0; i < 100 &&
+         (!SSL_is_init_finished(client.get()) ||
+          !SSL_is_init_finished(server.get())); ++i) {
+        for (SSL* ssl : {client.get(), server.get()}) {
+            const int rc = SSL_do_handshake(ssl);
+            if (rc != 1) {
+                const int error = SSL_get_error(ssl, rc);
+                ASSERT_TRUE(error == SSL_ERROR_WANT_READ ||
+                            error == SSL_ERROR_WANT_WRITE) << error;
+            }
+        }
+    }
+    ASSERT_TRUE(SSL_is_init_finished(client.get()));

Review Comment:
   SSL_is_init_finished is already available as a macro in pre-1.1.0 OpenSSL 
headers:
   
   ```c
   #define SSL_is_init_finished(a) (SSL_state(a) == SSL_ST_OK)
   ```
   
   See the official [OpenSSL 1.0.1u 
header](https://github.com/openssl/openssl/blob/OpenSSL_1_0_1u/ssl/ssl.h#L1480) 
and [OpenSSL 1.0.2u 
header](https://github.com/openssl/openssl/blob/OpenSSL_1_0_2u/ssl/ssl.h#L1648).
 The claim that this API is unavailable before 1.1.0 is therefore incorrect; 
these older headers already support the calls used by this test. No version 
guard or handshake-loop change is needed for this concern. Keeping the code 
unchanged and resolving this thread.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to