From d456af1cd311ab4895778bc138e6a7d9bac4a897 Mon Sep 17 00:00:00 2001 From: brapru Date: Thu, 12 Aug 2021 11:28:19 -0400 Subject: [PATCH] LibCore+LibHTTP: Check the status of the socket after EINPROGRESS Previously the system would assume the socket was connected after the file descriptor became writeable. Just because the fd is signaled as ready for output does not necessarily indicate the socket is connected. Instead, we should check the status of the socket with SO_ERROR and handle successes/errors accordingly. --- Userland/Libraries/LibCore/Socket.cpp | 29 ++++++++++++++++++++------ Userland/Libraries/LibCore/Socket.h | 1 + Userland/Libraries/LibHTTP/HttpJob.cpp | 6 ++++++ 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/Userland/Libraries/LibCore/Socket.cpp b/Userland/Libraries/LibCore/Socket.cpp index dc28ed58a5..418f3a8c7a 100644 --- a/Userland/Libraries/LibCore/Socket.cpp +++ b/Userland/Libraries/LibCore/Socket.cpp @@ -118,16 +118,33 @@ bool Socket::connect(const SocketAddress& address) bool Socket::common_connect(const struct sockaddr* addr, socklen_t addrlen) { auto connected = [this] { - dbgln_if(CSOCKET_DEBUG, "{} connected!", *this); - if (!m_connected) { + int so_error; + socklen_t so_error_len = sizeof(so_error); + + int rc = getsockopt(fd(), SOL_SOCKET, SO_ERROR, &so_error, &so_error_len); + if (rc < 0) { + dbgln_if(CSOCKET_DEBUG, "Failed to check the status of SO_ERROR"); + m_connected = false; + if (on_error) + on_error(); + } + + if (so_error == 0) { + dbgln_if(CSOCKET_DEBUG, "{} connected!", *this); m_connected = true; ensure_read_notifier(); - if (m_notifier) { - m_notifier->remove_from_parent(); - m_notifier = nullptr; - } if (on_connected) on_connected(); + } else { + dbgln_if(CSOCKET_DEBUG, "Failed to connect to {}", *this); + m_connected = false; + if (on_error) + on_error(); + } + + if (m_notifier) { + m_notifier->remove_from_parent(); + m_notifier = nullptr; } }; int rc = ::connect(fd(), addr, addrlen); diff --git a/Userland/Libraries/LibCore/Socket.h b/Userland/Libraries/LibCore/Socket.h index 58823b7fbd..359e8a56c8 100644 --- a/Userland/Libraries/LibCore/Socket.h +++ b/Userland/Libraries/LibCore/Socket.h @@ -43,6 +43,7 @@ public: int destination_port() const { return m_destination_port; } Function on_connected; + Function on_error; Function on_ready_to_read; protected: diff --git a/Userland/Libraries/LibHTTP/HttpJob.cpp b/Userland/Libraries/LibHTTP/HttpJob.cpp index d8c480c09a..6e8d36de78 100644 --- a/Userland/Libraries/LibHTTP/HttpJob.cpp +++ b/Userland/Libraries/LibHTTP/HttpJob.cpp @@ -20,6 +20,12 @@ void HttpJob::start() dbgln_if(CHTTPJOB_DEBUG, "HttpJob: on_connected callback"); on_socket_connected(); }; + m_socket->on_error = [this] { + dbgln_if(CHTTPJOB_DEBUG, "HttpJob: on_error callback"); + deferred_invoke([this](auto&) { + did_fail(Core::NetworkJob::Error::ConnectionFailed); + }); + }; bool success = m_socket->connect(m_request.url().host(), m_request.url().port()); if (!success) { deferred_invoke([this](auto&) {