Parcourir la source

Don't wait for the peer's close_notify on OpenSSL shutdown

tls::shutdown() on OpenSSL called SSL_shutdown() a second time to wait
for the peer's close_notify. An idle keep-alive client never sends one,
so closing its connection held the worker thread until the read timeout,
and Server::stop() waited for it. Send close_notify and return, as the
Mbed TLS and wolfSSL backends already do.
yhirose il y a 2 semaines
Parent
commit
52f214bf2e
2 fichiers modifiés avec 47 ajouts et 5 suppressions
  1. 5 5
      httplib.h
  2. 42 0
      test/test.cc

+ 5 - 5
httplib.h

@@ -19341,11 +19341,11 @@ inline void shutdown(session_t session, bool graceful) {
 
   auto ssl = static_cast<SSL *>(session);
   if (graceful) {
-    // First call sends close_notify
-    if (SSL_shutdown(ssl) == 0) {
-      // Second call waits for peer's close_notify
-      SSL_shutdown(ssl);
-    }
+    // Send close_notify without waiting for the peer's. The connection is
+    // closed right after this, so a unidirectional shutdown is enough, and an
+    // idle peer that never answers would otherwise hold this thread until the
+    // read timeout. The other backends do not wait either.
+    SSL_shutdown(ssl);
   }
 }
 

+ 42 - 0
test/test.cc

@@ -11982,6 +11982,48 @@ TEST(KeepAliveTest, ReadTimeoutSSL) {
   EXPECT_EQ(StatusCode::OK_200, resb->status);
   EXPECT_EQ("b", resb->body);
 }
+
+// Closing an idle keep-alive connection sends close_notify and returns. The
+// server must not wait for the client's close_notify: an idle client never
+// sends one, so the worker would be held until the read timeout expires, and
+// stop() would wait for it.
+TEST(KeepAliveTest, SSLIdleCloseDoesNotWaitForPeer) {
+  SSLServer svr(SERVER_CERT_FILE, SERVER_PRIVATE_KEY_FILE);
+  ASSERT_TRUE(svr.is_valid());
+  svr.set_keep_alive_timeout(1);
+  svr.set_read_timeout(10, 0);
+  svr.Get("/", [](const Request &, Response &res) {
+    res.set_content("ok", "text/plain");
+  });
+
+  auto port = svr.bind_to_any_port(HOST);
+  auto listen_thread = std::thread([&svr]() { svr.listen_after_bind(); });
+  auto se = detail::scope_exit([&] {
+    if (listen_thread.joinable()) {
+      svr.stop();
+      listen_thread.join();
+    }
+  });
+  svr.wait_until_ready();
+
+  SSLClient cli(HOST, port);
+  cli.enable_server_certificate_verification(false);
+  cli.set_keep_alive(true);
+  auto res = cli.Get("/");
+  ASSERT_TRUE(res) << "Error: " << to_string(res.error());
+  EXPECT_EQ(StatusCode::OK_200, res->status);
+
+  // Stay idle past the keep-alive timeout so the server closes the connection.
+  std::this_thread::sleep_for(std::chrono::milliseconds(1500));
+
+  auto start = std::chrono::steady_clock::now();
+  svr.stop();
+  listen_thread.join();
+  auto elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(
+                     std::chrono::steady_clock::now() - start)
+                     .count();
+  EXPECT_LT(elapsed, 3000);
+}
 #endif
 
 class ServerTestWithAI_PASSIVE : public ::testing::Test {