From aedb013ee365f9abe92c78fae9e6455139cb7d28 Mon Sep 17 00:00:00 2001 From: sin-ack Date: Sat, 18 Dec 2021 12:38:44 +0000 Subject: [PATCH] LibIMAP+Userland: Convert LibIMAP::Client to the Serenity Stream APIs You now cannot get an unconnected LibIMAP::Client, but you can still close it. This makes for a nicer API where we don't have a Client object in a limbo state between being constructed and being connected. This code still isn't as nice as it should be, as TLS::TLSv12 is still not a Core::Stream::Socket subclass, which would allow for consolidating most of the TLS/non-TLS code into a single implementation. --- Userland/Applications/Mail/MailWidget.cpp | 11 +- Userland/Libraries/LibIMAP/Client.cpp | 158 ++++++++++++++-------- Userland/Libraries/LibIMAP/Client.h | 48 ++++--- Userland/Utilities/test-imap.cpp | 39 +++--- 4 files changed, 162 insertions(+), 94 deletions(-) diff --git a/Userland/Applications/Mail/MailWidget.cpp b/Userland/Applications/Mail/MailWidget.cpp index 996246d652b..15c01513f7a 100644 --- a/Userland/Applications/Mail/MailWidget.cpp +++ b/Userland/Applications/Mail/MailWidget.cpp @@ -126,12 +126,15 @@ bool MailWidget::connect_and_login() return false; } - m_imap_client = make(server, port, tls); - auto connection_promise = m_imap_client->connect(); - if (!connection_promise) { - GUI::MessageBox::show_error(window(), String::formatted("Failed to connect to '{}:{}' over {}.", server, port, tls ? "TLS" : "Plaintext")); + auto maybe_imap_client = tls ? IMAP::Client::connect_tls(server, port) : IMAP::Client::connect_plaintext(server, port); + if (maybe_imap_client.is_error()) { + GUI::MessageBox::show_error(window(), String::formatted("Failed to connect to '{}:{}' over {}: {}", server, port, tls ? "TLS" : "Plaintext", maybe_imap_client.error())); return false; } + m_imap_client = maybe_imap_client.release_value(); + + auto connection_promise = m_imap_client->connection_promise(); + VERIFY(!connection_promise.is_null()); connection_promise->await(); auto response = m_imap_client->login(username, password)->await().release_value(); diff --git a/Userland/Libraries/LibIMAP/Client.cpp b/Userland/Libraries/LibIMAP/Client.cpp index 7b1bef5a8ea..d33429dc521 100644 --- a/Userland/Libraries/LibIMAP/Client.cpp +++ b/Userland/Libraries/LibIMAP/Client.cpp @@ -4,107 +4,143 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include "AK/OwnPtr.h" +#include #include namespace IMAP { -Client::Client(StringView host, unsigned int port, bool start_with_tls) + +Client::Client(StringView host, u16 port, NonnullRefPtr socket) : m_host(host) , m_port(port) - , m_tls(start_with_tls) - , m_parser(Parser()) + , m_tls(true) + , m_tls_socket(move(socket)) + , m_connect_pending(Promise::construct()) { - if (start_with_tls) { - m_tls_socket = TLS::TLSv12::construct(nullptr); - m_tls_socket->set_root_certificates(DefaultRootCACertificates::the().certificates()); - } else { - m_socket = Core::TCPSocket::construct(); - } + setup_callbacks(); } -RefPtr> Client::connect() +Client::Client(StringView host, u16 port, NonnullOwnPtr socket) + : m_host(host) + , m_port(port) + , m_tls(false) + , m_socket(move(socket)) + , m_connect_pending(Promise::construct()) +{ + setup_callbacks(); +} + +Client::Client(Client&& other) + : m_host(other.m_host) + , m_port(other.m_port) + , m_tls(other.m_tls) + , m_socket(move(other.m_socket)) + , m_tls_socket(move(other.m_tls_socket)) + , m_connect_pending(move(other.m_connect_pending)) +{ + setup_callbacks(); +} + +void Client::setup_callbacks() { - bool success; if (m_tls) { - success = connect_tls(); + m_tls_socket->on_tls_ready_to_read = [&](TLS::TLSv12&) { + auto maybe_error = on_tls_ready_to_receive(); + if (maybe_error.is_error()) { + dbgln("Error receiving from the socket: {}", maybe_error.error()); + close(); + } + }; + } else { - success = connect_plaintext(); + m_socket->on_ready_to_read = [&] { + auto maybe_error = on_ready_to_receive(); + if (maybe_error.is_error()) { + dbgln("Error receiving from the socket: {}", maybe_error.error()); + close(); + } + }; } - if (!success) - return {}; - m_connect_pending = Promise::construct(); - return m_connect_pending; } -bool Client::connect_tls() +ErrorOr> Client::connect_tls(StringView host, u16 port) { - m_tls_socket->on_tls_ready_to_read = [&](TLS::TLSv12&) { - on_tls_ready_to_receive(); - }; - m_tls_socket->on_tls_error = [&](TLS::AlertDescription alert) { + auto tls_socket = TLS::TLSv12::construct(nullptr); + tls_socket->set_root_certificates(DefaultRootCACertificates::the().certificates()); + + tls_socket->on_tls_error = [&](TLS::AlertDescription alert) { dbgln("failed: {}", alert_name(alert)); }; - m_tls_socket->on_tls_connected = [&] { + tls_socket->on_tls_connected = [&] { dbgln("connected"); }; - auto success = m_tls_socket->connect(m_host, m_port); - dbgln("connecting to {}:{} {}", m_host, m_port, success); - return success; + + auto success = tls_socket->connect(host, port); + dbgln("connecting to {}:{} {}", host, port, success); + + return adopt_nonnull_own_or_enomem(new (nothrow) Client(host, port, tls_socket)); } -bool Client::connect_plaintext() +ErrorOr> Client::connect_plaintext(StringView host, u16 port) { - m_socket->on_ready_to_read = [&] { - on_ready_to_receive(); - }; - auto success = m_socket->connect(m_host, m_port); - dbgln("connecting to {}:{} {}", m_host, m_port, success); - return success; + auto socket = TRY(Core::Stream::TCPSocket::connect(host, port)); + dbgln("Connected to {}:{}", host, port); + return adopt_nonnull_own_or_enomem(new (nothrow) Client(host, port, move(socket))); } -void Client::on_tls_ready_to_receive() +ErrorOr Client::on_tls_ready_to_receive() { if (!m_tls_socket->can_read()) - return; + return {}; auto data = m_tls_socket->read(); + // FIXME: Make TLSv12 return the actual error instead of returning a bogus + // one here. if (!data.has_value()) - return; + return Error::from_errno(EIO); // Once we get server hello we can start sending if (m_connect_pending) { m_connect_pending->resolve({}); m_connect_pending.clear(); - return; + return {}; } m_buffer += data.value(); if (m_buffer[m_buffer.size() - 1] == '\n') { // Don't try parsing until we have a complete line. auto response = m_parser.parse(move(m_buffer), m_expecting_response); - handle_parsed_response(move(response)); + MUST(handle_parsed_response(move(response))); m_buffer.clear(); } + + return {}; } -void Client::on_ready_to_receive() +ErrorOr Client::on_ready_to_receive() { - if (!m_socket->can_read()) - return; - m_buffer += m_socket->read_all(); + if (!TRY(m_socket->can_read_without_blocking())) + return {}; + + auto pending_bytes = TRY(m_socket->pending_bytes()); + auto receive_buffer = TRY(m_buffer.get_bytes_for_writing(pending_bytes)); + TRY(m_socket->read(receive_buffer)); // Once we get server hello we can start sending. if (m_connect_pending) { m_connect_pending->resolve({}); m_connect_pending.clear(); m_buffer.clear(); - return; + return {}; } if (m_buffer[m_buffer.size() - 1] == '\n') { // Don't try parsing until we have a complete line. auto response = m_parser.parse(move(m_buffer), m_expecting_response); - handle_parsed_response(move(response)); + TRY(handle_parsed_response(move(response))); m_buffer.clear(); } + + return {}; } static ReadonlyBytes command_byte_buffer(CommandType command) @@ -170,15 +206,17 @@ static ReadonlyBytes command_byte_buffer(CommandType command) VERIFY_NOT_REACHED(); } -void Client::send_raw(StringView data) +ErrorOr Client::send_raw(StringView data) { if (m_tls) { m_tls_socket->write(data.bytes()); m_tls_socket->write("\r\n"sv.bytes()); } else { - m_socket->write(data.bytes()); - m_socket->write("\r\n"sv.bytes()); + TRY(m_socket->write(data.bytes())); + TRY(m_socket->write("\r\n"sv.bytes())); } + + return {}; } RefPtr>> Client::send_command(Command&& command) @@ -190,7 +228,7 @@ RefPtr>> Client::send_command(Command&& command) m_pending_promises.append(promise); if (m_pending_promises.size() == 1) - send_next_command(); + MUST(send_next_command()); return promise; } @@ -245,7 +283,7 @@ RefPtr>> Client::select(StringView string) return cast_promise(send_command(move(command))); } -void Client::handle_parsed_response(ParseStatus&& parse_status) +ErrorOr Client::handle_parsed_response(ParseStatus&& parse_status) { if (!m_expecting_response) { if (!parse_status.successful) { @@ -268,12 +306,14 @@ void Client::handle_parsed_response(ParseStatus&& parse_status) } if (should_send_next && !m_command_queue.is_empty()) { - send_next_command(); + TRY(send_next_command()); } } + + return {}; } -void Client::send_next_command() +ErrorOr Client::send_next_command() { auto command = m_command_queue.take_first(); ByteBuffer buffer; @@ -287,8 +327,9 @@ void Client::send_next_command() buffer.append(arg.bytes().data(), arg.length()); } - send_raw(buffer); + TRY(send_raw(buffer)); m_expecting_response = true; + return {}; } RefPtr>> Client::examine(StringView string) @@ -358,7 +399,7 @@ RefPtr>> Client::finish_idle() { auto promise = Promise>::construct(); m_pending_promises.append(promise); - send_raw("DONE"); + MUST(send_raw("DONE")); m_expecting_response = true; return cast_promise(promise); } @@ -415,9 +456,9 @@ RefPtr>> Client::append(StringView mailbox, Mess continue_req->on_resolved = [this, message2 { move(message) }](auto& data) { if (!data.has_value()) { - handle_parsed_response({ .successful = false, .response = {} }); + MUST(handle_parsed_response({ .successful = false, .response = {} })); } else { - send_raw(message2.data); + MUST(send_raw(message2.data)); m_expecting_response = true; } }; @@ -452,6 +493,7 @@ RefPtr>> Client::copy(Sequence sequence_set, Str return cast_promise(send_command(move(command))); } + void Client::close() { if (m_tls) { @@ -460,4 +502,10 @@ void Client::close() m_socket->close(); } } + +bool Client::is_open() +{ + return m_tls ? m_tls_socket->is_open() : m_socket->is_open(); +} + } diff --git a/Userland/Libraries/LibIMAP/Client.h b/Userland/Libraries/LibIMAP/Client.h index cea34fe37cb..9796dab86e6 100644 --- a/Userland/Libraries/LibIMAP/Client.h +++ b/Userland/Libraries/LibIMAP/Client.h @@ -8,6 +8,7 @@ #include #include +#include #include #include @@ -16,15 +17,23 @@ template using Promise = Core::Promise; class Client { + AK_MAKE_NONCOPYABLE(Client); friend class Parser; public: - Client(StringView host, unsigned port, bool start_with_tls); + static ErrorOr> connect_tls(StringView host, u16 port); + static ErrorOr> connect_plaintext(StringView host, u16 port); + + Client(Client&&); + + RefPtr> connection_promise() + { + return m_connect_pending; + } - RefPtr> connect(); RefPtr>> send_command(Command&&); RefPtr>> send_simple_command(CommandType); - void send_raw(StringView data); + ErrorOr send_raw(StringView data); RefPtr>> login(StringView username, StringView password); RefPtr>> list(StringView reference_name, StringView mailbox_name); RefPtr>> lsub(StringView reference_name, StringView mailbox_name); @@ -45,37 +54,42 @@ public: RefPtr>> status(StringView mailbox, Vector const& types); RefPtr>> append(StringView mailbox, Message&& message, Optional> flags = {}, Optional date_time = {}); + bool is_open(); void close(); Function unrequested_response_callback; private: - StringView m_host; - unsigned m_port; - RefPtr m_socket; - RefPtr m_tls_socket; + Client(StringView host, u16 port, NonnullRefPtr); + Client(StringView host, u16 port, NonnullOwnPtr); + void setup_callbacks(); - void on_ready_to_receive(); - void on_tls_ready_to_receive(); + ErrorOr on_ready_to_receive(); + ErrorOr on_tls_ready_to_receive(); + + ErrorOr handle_parsed_response(ParseStatus&& parse_status); + ErrorOr send_next_command(); + + StringView m_host; + u16 m_port; bool m_tls; - int m_current_command = 1; + // FIXME: Convert this to a single `NonnullOwnPtr` + // once `TLS::TLSv12` is converted to a `Socket` as well. + OwnPtr m_socket; + RefPtr m_tls_socket; + RefPtr> m_connect_pending {}; - bool connect_tls(); - bool connect_plaintext(); + int m_current_command = 1; // Sent but response not received Vector>>> m_pending_promises; // Not yet sent Vector m_command_queue {}; - RefPtr> m_connect_pending {}; - ByteBuffer m_buffer; - Parser m_parser; + Parser m_parser {}; bool m_expecting_response { false }; - void handle_parsed_response(ParseStatus&& parse_status); - void send_next_command(); }; } diff --git a/Userland/Utilities/test-imap.cpp b/Userland/Utilities/test-imap.cpp index d0565954786..10a18101197 100644 --- a/Userland/Utilities/test-imap.cpp +++ b/Userland/Utilities/test-imap.cpp @@ -43,21 +43,21 @@ ErrorOr serenity_main(Main::Arguments arguments) } Core::EventLoop loop; - auto client = IMAP::Client(host, port, tls); - client.connect()->await(); + auto client = TRY(tls ? IMAP::Client::connect_tls(host, port) : IMAP::Client::connect_plaintext(host, port)); + client->connection_promise()->await(); - auto response = client.login(username, password.view())->await().release_value(); + auto response = client->login(username, password.view())->await().release_value(); outln("[LOGIN] Login response: {}", response.response_text()); - response = move(client.send_simple_command(IMAP::CommandType::Capability)->await().value().get()); + response = move(client->send_simple_command(IMAP::CommandType::Capability)->await().value().get()); outln("[CAPABILITY] First capability: {}", response.data().capabilities().first()); bool idle_supported = !response.data().capabilities().find_if([](auto capability) { return capability.equals_ignoring_case("IDLE"); }).is_end(); - response = client.list("", "*")->await().release_value(); + response = client->list("", "*")->await().release_value(); outln("[LIST] First mailbox: {}", response.data().list_items().first().name); - auto mailbox = "Inbox"; - response = client.select(mailbox)->await().release_value(); + auto mailbox = "Inbox"sv; + response = client->select(mailbox)->await().release_value(); outln("[SELECT] Select response: {}", response.response_text()); auto message = Message { @@ -70,7 +70,7 @@ ErrorOr serenity_main(Main::Arguments arguments) "This is a message just to say hello.\r\n" "So, \"Hello\"." }; - auto promise = client.append("INBOX", move(message)); + auto promise = client->append("INBOX", move(message)); response = promise->await().release_value(); outln("[APPEND] Response: {}", response.response_text()); @@ -79,13 +79,13 @@ ErrorOr serenity_main(Main::Arguments arguments) IMAP::SearchKey::From { "jdoe@machine.example" } }); keys.append(IMAP::SearchKey { IMAP::SearchKey::Subject { "Saying Hello" } }); - response = client.search({}, move(keys), false)->await().release_value(); + response = client->search({}, move(keys), false)->await().release_value(); Vector search_results = move(response.data().search_results()); - int added_message = search_results.first(); + auto added_message = search_results.first(); outln("[SEARCH] Number of results: {}", search_results.size()); - response = client.status("INBOX", { IMAP::StatusItemType::Recent, IMAP::StatusItemType::Messages })->await().release_value(); + response = client->status("INBOX", { IMAP::StatusItemType::Recent, IMAP::StatusItemType::Messages })->await().release_value(); outln("[STATUS] Recent items: {}", response.data().status_item().get(IMAP::StatusItemType::Recent)); for (auto item : search_results) { @@ -118,7 +118,7 @@ ErrorOr serenity_main(Main::Arguments arguments) }; // clang-format on - auto fetch_response = client.fetch(fetch_command, false)->await().release_value(); + auto fetch_response = client->fetch(fetch_command, false)->await().release_value(); outln("[FETCH] Subject of search result: {}", fetch_response.data() .fetch_data() @@ -133,25 +133,28 @@ ErrorOr serenity_main(Main::Arguments arguments) .value()); } - response = client.store(IMAP::StoreMethod::Add, { added_message, added_message }, false, { "\\Deleted" }, false)->await().release_value(); + // FIXME: There is a discrepancy between IMAP::Sequence wanting signed ints + // and IMAP search results returning unsigned ones. Find which one is + // more correct and fix this. + response = client->store(IMAP::StoreMethod::Add, { static_cast(added_message), static_cast(added_message) }, false, { "\\Deleted" }, false)->await().release_value(); outln("[STORE] Store response: {}", response.response_text()); - response = move(client.send_simple_command(IMAP::CommandType::Expunge)->await().release_value().get()); + response = move(client->send_simple_command(IMAP::CommandType::Expunge)->await().release_value().get()); outln("[EXPUNGE] Number of expunged entries: {}", response.data().expunged().size()); if (idle_supported) { - VERIFY(client.idle()->await().has_value()); + VERIFY(client->idle()->await().has_value()); sleep(3); - response = client.finish_idle()->await().release_value(); + response = client->finish_idle()->await().release_value(); outln("[IDLE] Idle response: {}", response.response_text()); } else { outln("[IDLE] Skipped. No IDLE support."); } - response = move(client.send_simple_command(IMAP::CommandType::Logout)->await().release_value().get()); + response = move(client->send_simple_command(IMAP::CommandType::Logout)->await().release_value().get()); outln("[LOGOUT] Bye: {}", response.data().bye_message().value()); - client.close(); + client->close(); return 0; }