From 812875bc89cfe380e2d9dc7ff75803a36b0810fa Mon Sep 17 00:00:00 2001 From: DexesTTP Date: Tue, 18 May 2021 00:28:06 +0200 Subject: [PATCH] LibWebSocket: Fixed occasional infinite loop with TLS sockets This was caused by a double notifier on the TLS socket, which caused the TLS code to freak out about not being able to read properly. In addition, the existing loop inside of drain_read() has been replaced by code that actually works, and which includes new warnings when the drain method is called before initialization is done or after the websocket gets closed. --- .../Impl/TLSv12WebSocketConnectionImpl.cpp | 8 ++--- .../Impl/TLSv12WebSocketConnectionImpl.h | 1 - Userland/Libraries/LibWebSocket/WebSocket.cpp | 34 ++++++++++++------- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/Userland/Libraries/LibWebSocket/Impl/TLSv12WebSocketConnectionImpl.cpp b/Userland/Libraries/LibWebSocket/Impl/TLSv12WebSocketConnectionImpl.cpp index c1bf4ced3c..9ad1bc41d2 100644 --- a/Userland/Libraries/LibWebSocket/Impl/TLSv12WebSocketConnectionImpl.cpp +++ b/Userland/Libraries/LibWebSocket/Impl/TLSv12WebSocketConnectionImpl.cpp @@ -26,15 +26,13 @@ void TLSv12WebSocketConnectionImpl::connect(ConnectionInfo const& connection) VERIFY(on_ready_to_read); m_socket = TLS::TLSv12::construct(this); - m_notifier = Core::Notifier::construct(m_socket->fd(), Core::Notifier::Read); - m_notifier->on_ready_to_read = [this] { - on_ready_to_read(); - }; - m_socket->set_root_certificates(DefaultRootCACertificates::the().certificates()); m_socket->on_tls_error = [this](TLS::AlertDescription) { on_connection_error(); }; + m_socket->on_tls_ready_to_read = [this] { + on_ready_to_read(); + }; m_socket->on_tls_ready_to_write = [this] { on_connected(); }; diff --git a/Userland/Libraries/LibWebSocket/Impl/TLSv12WebSocketConnectionImpl.h b/Userland/Libraries/LibWebSocket/Impl/TLSv12WebSocketConnectionImpl.h index 90c74b5b86..fecdd4d38f 100644 --- a/Userland/Libraries/LibWebSocket/Impl/TLSv12WebSocketConnectionImpl.h +++ b/Userland/Libraries/LibWebSocket/Impl/TLSv12WebSocketConnectionImpl.h @@ -38,7 +38,6 @@ public: virtual void discard_connection() override; private: - RefPtr m_notifier; RefPtr m_socket; }; diff --git a/Userland/Libraries/LibWebSocket/WebSocket.cpp b/Userland/Libraries/LibWebSocket/WebSocket.cpp index b1fbf9e129..d69c39b282 100644 --- a/Userland/Libraries/LibWebSocket/WebSocket.cpp +++ b/Userland/Libraries/LibWebSocket/WebSocket.cpp @@ -114,19 +114,27 @@ void WebSocket::drain_read() return; } - while (m_impl->can_read()) { - if (m_state == WebSocket::InternalState::WaitingForServerHandshake) { - read_server_handshake(); - return; - } - if (m_state == WebSocket::InternalState::Open) { - read_frame(); - return; - } - if (m_state == WebSocket::InternalState::Closing) { - read_frame(); - return; - } + switch (m_state) { + case InternalState::NotStarted: + case InternalState::EstablishingProtocolConnection: + case InternalState::SendingClientHandshake: { + auto initializing_bytes = m_impl->read(1024); + dbgln("drain_read() was called on a websocket that isn't opened yet. Read {} bytes from the socket.", initializing_bytes.size()); + } break; + case InternalState::WaitingForServerHandshake: { + read_server_handshake(); + } break; + case InternalState::Open: + case InternalState::Closing: { + read_frame(); + } break; + case InternalState::Closed: + case InternalState::Errored: { + auto closed_bytes = m_impl->read(1024); + dbgln("drain_read() was called on a closed websocket. Read {} bytes from the socket.", closed_bytes.size()); + } break; + default: + VERIFY_NOT_REACHED(); } }