From dd1635250908c8370814d811c72e7e1c626e7a82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Martin?= Date: Sat, 3 Oct 2026 18:42:24 +0200 Subject: [PATCH] IRC: edit server settings as a draft, applied under the service lock The IRC App edited the live config while the IRC task could be reading it to connect. The App now edits a copy; applyConfig() validates it and swaps it in under the lock, and the task connects from a snapshot. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01EhqxQ49eCju4CzKYNjZzwT --- lib/irc/src/irc_config.h | 15 +++++++++ src/apps/irc_app.cpp | 13 +++---- src/apps/irc_app.h | 2 ++ src/services/irc_service.cpp | 43 +++++++++++++++--------- src/services/irc_service.h | 7 ++-- test/test_irc_config/test_irc_config.cpp | 14 ++++++++ 6 files changed, 69 insertions(+), 25 deletions(-) diff --git a/lib/irc/src/irc_config.h b/lib/irc/src/irc_config.h index 924218f..144e130 100644 --- a/lib/irc/src/irc_config.h +++ b/lib/irc/src/irc_config.h @@ -22,6 +22,21 @@ class IrcConfig { std::string nickservPassword; // otherwise IDENTIFY with NickServ, if set std::vector autojoin; + // A copy is a draft the UI can edit freely; copySettingsFrom() applies one. + IrcConfig(const IrcConfig&) = default; + void copySettingsFrom(const IrcConfig& other) { + host = other.host; + port = other.port; + tls = other.tls; + allowSelfSigned = other.allowSelfSigned; + pinnedSha256 = other.pinnedSha256; + nick = other.nick; + saslUser = other.saslUser; + saslPassword = other.saslPassword; + nickservPassword = other.nickservPassword; + autojoin = other.autojoin; + } + void load(const std::string& defaultNick); std::string save(); // empty on success, otherwise why it was refused std::string validate() const; diff --git a/src/apps/irc_app.cpp b/src/apps/irc_app.cpp index 7bee493..c36c8cf 100644 --- a/src/apps/irc_app.cpp +++ b/src/apps/irc_app.cpp @@ -98,6 +98,7 @@ bool IrcApp::onChatKey(const KeyEvent& e) { input_.setText(""); scroll_ = 0; if (lowered(text) == "/settings") { + draft_.reset(new IrcConfig(irc_.draftConfig())); page_ = Page::Settings; fields_.setCount(kFields); break; @@ -127,7 +128,7 @@ std::string IrcApp::fieldLabel(int f) const { } std::string IrcApp::fieldValue(int f) const { - const IrcConfig& c = irc_.config(); + const IrcConfig& c = *draft_; auto secret = [](const std::string& s) { return s.empty() ? std::string("-") : std::string("(set)"); }; switch (f) { case kHost: return c.host; @@ -144,7 +145,7 @@ std::string IrcApp::fieldValue(int f) const { } void IrcApp::startEditing(int f) { - const IrcConfig& c = irc_.config(); + const IrcConfig& c = *draft_; fieldEditor_ = LineEditor(f == kAutojoin ? 200 : 63); switch (f) { case kHost: fieldEditor_.setText(c.host); break; @@ -158,7 +159,7 @@ void IrcApp::startEditing(int f) { } void IrcApp::finishEditing() { - IrcConfig& c = irc_.config(); + IrcConfig& c = *draft_; const std::string& v = fieldEditor_.text(); switch (fields_.selected()) { case kHost: c.host = v; break; @@ -185,12 +186,12 @@ bool IrcApp::onSettingsKey(const KeyEvent& e) { } return true; } - IrcConfig& c = irc_.config(); + IrcConfig& c = *draft_; switch (e.key) { case Key::Up: fields_.up(); break; case Key::Down: fields_.down(); break; case Key::Back: - c.load(c.nick); // discard unsaved edits + draft_.reset(); // discard unsaved edits page_ = Page::Chat; break; case Key::Select: @@ -201,7 +202,7 @@ bool IrcApp::onSettingsKey(const KeyEvent& e) { c.pinnedSha256.clear(); break; case kSave: { - std::string error = irc_.saveConfig(); + std::string error = irc_.applyConfig(c); if (!error.empty()) { warn(error); break; diff --git a/src/apps/irc_app.h b/src/apps/irc_app.h index d711529..77e8215 100644 --- a/src/apps/irc_app.h +++ b/src/apps/irc_app.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include "app.h" @@ -50,6 +51,7 @@ class IrcApp : public App { ListModel fields_{theme::kContent.h / theme::kLineHeight}; bool editing_ = false; LineEditor fieldEditor_{63}; + std::unique_ptr draft_; // the settings being edited, applied on Save }; } // namespace roro diff --git a/src/services/irc_service.cpp b/src/services/irc_service.cpp index 44c9da8..d69d23f 100644 --- a/src/services/irc_service.cpp +++ b/src/services/irc_service.cpp @@ -48,15 +48,19 @@ int IrcService::totalUnread() { return session_->totalUnread(); } -std::string IrcService::saveConfig() { - std::string error = config_.save(); +IrcConfig IrcService::draftConfig() { + Lock l(lock_); + return config_; +} + +std::string IrcService::applyConfig(const IrcConfig& draft) { + std::string error = draft.validate(); if (!error.empty()) return error; - if (wanted_) { - restart_ = true; // the task reconnects with a fresh session - } else { - Lock l(lock_); - session_.reset(new IrcSession(config_)); // a new server or nick starts a fresh session - } + Lock l(lock_); + config_.copySettingsFrom(draft); + config_.save(); + if (wanted_) restart_ = true; // the task reconnects with a fresh session + else session_.reset(new IrcSession(config_)); return ""; } @@ -72,14 +76,15 @@ void IrcService::scheduleRetry(const std::string& why) { bool IrcService::open() { status_ = Status::Connecting; - bool pinning = config_.tls && config_.allowSelfSigned; - conn_ = config_.tls ? static_cast(&tlsClient_) : &plainClient_; - if (config_.tls) { + IrcConfig settings = draftConfig(); // a snapshot: the UI may apply new settings meanwhile + bool pinning = settings.tls && settings.allowSelfSigned; + conn_ = settings.tls ? static_cast(&tlsClient_) : &plainClient_; + if (settings.tls) { tlsClient_.setTimeout(15); // seconds, for the handshake if (pinning) tlsClient_.setInsecure(); else tlsClient_.useBuiltinCACertBundle(); } - if (!conn_->connect(config_.host.c_str(), config_.port)) return false; + if (!conn_->connect(settings.host.c_str(), settings.port)) return false; if (pinning) { uint8_t sha[32]; @@ -88,10 +93,11 @@ bool IrcService::open() { return false; } std::string fingerprint = hex(sha, sizeof(sha)); - if (config_.pinnedSha256.empty()) { + if (settings.pinnedSha256.empty()) { + Lock l(lock_); config_.pinnedSha256 = fingerprint; // trust on first use config_.save(); - } else if (config_.pinnedSha256 != fingerprint) { + } else if (settings.pinnedSha256 != fingerprint) { tlsClient_.stop(); Lock l(lock_); session_->disconnected(clock_.utcNow(), "server certificate changed: not connecting"); @@ -145,6 +151,11 @@ void IrcService::flushEffects() { if (open_) conn_->print((line + "\r\n").c_str()); std::string date = clock_.localDate(); + std::string host; + { + Lock l(lock_); + host = config_.host; + } for (auto& entry : fx.logs) { const IrcLine& l = entry.line; std::string when = l.utc >= 0 ? ClockModel::formatLocalTime(l.utc) : "--:--"; @@ -156,7 +167,7 @@ void IrcService::flushEffects() { case IrcLine::Kind::Info: text = l.text; break; default: text = "<" + l.nick + "> " + l.text; break; } - storage_.appendLine(storage::dailyLogPath({"irc", config_.host, entry.buffer}, date), when + " " + text); + storage_.appendLine(storage::dailyLogPath({"irc", host, entry.buffer}, date), when + " " + text); } for (auto& n : fx.notifications) bus_.publish(Event::withText(EventType::Notification, n.c_str(), static_cast(NotificationLevel::Message))); @@ -202,7 +213,7 @@ void IrcService::loop() { retryAtMs_ = now; // reconnect as soon as Wi-Fi is back } else if (!open_) { if (static_cast(now - retryAtMs_) >= 0) { - if (!open()) scheduleRetry("could not connect to " + config_.host); + if (!open()) scheduleRetry("could not connect to " + draftConfig().host); } } else if (!conn_->connected()) { close(""); diff --git a/src/services/irc_service.h b/src/services/irc_service.h index a65a224..5db3e79 100644 --- a/src/services/irc_service.h +++ b/src/services/irc_service.h @@ -44,9 +44,10 @@ class IrcService : public Service { } int totalUnread(); - IrcConfig& config() { return config_; } - // Saves the config and, if running, reconnects with it (a fresh session). - std::string saveConfig(); + // A copy of the server settings to edit; applyConfig() validates, saves and, if running, + // reconnects with it (a fresh session). + IrcConfig draftConfig(); + std::string applyConfig(const IrcConfig& draft); private: struct Lock { diff --git a/test/test_irc_config/test_irc_config.cpp b/test/test_irc_config/test_irc_config.cpp index 142b6b2..5773032 100644 --- a/test/test_irc_config/test_irc_config.cpp +++ b/test/test_irc_config/test_irc_config.cpp @@ -64,11 +64,25 @@ void test_autojoin_text_round_trip() { TEST_ASSERT_EQUAL_STRING("#a #b", IrcConfig::formatChannels({"#a", "#b"}).c_str()); } +void test_copy_settings_from_a_draft() { + MemoryStore store; + IrcConfig live(store); + live.load("x"); + IrcConfig draft = live; + draft.host = "irc.example.org"; + draft.autojoin = {"#a"}; + TEST_ASSERT_EQUAL_STRING("irc.libera.chat", live.host.c_str()); // the draft is independent + live.copySettingsFrom(draft); + TEST_ASSERT_EQUAL_STRING("irc.example.org", live.host.c_str()); + TEST_ASSERT_EQUAL(1, live.autojoin.size()); +} + int main() { UNITY_BEGIN(); RUN_TEST(test_defaults_point_at_libera_over_tls); RUN_TEST(test_save_and_reload); RUN_TEST(test_invalid_values_are_refused_with_a_reason); RUN_TEST(test_autojoin_text_round_trip); + RUN_TEST(test_copy_settings_from_a_draft); return UNITY_END(); }