From afa6ab1fd81220b8f4fa5c138c6b9e3c1987abc5 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 5 Jul 2026 10:07:12 +0200 Subject: [PATCH 01/18] fix(AModule): don't crash on click/scroll commands with literal braces handleUserEvent ran the configured command through fmt::format(fmt::runtime(...)) to substitute {x}/{y}. Commands containing literal braces that aren't {x}/{y} (e.g. `echo ${HOME}`, `awk '{print $1}'`, brace expansions) made libfmt throw fmt::format_error. Uncaught inside a GTK signal handler this aborts the whole bar. Only format when {x}/{y} is present and fall back to the raw command on failure. Fixes bar abort/std::terminate on on-click/on-scroll commands containing braces. --- src/AModule.cpp | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/AModule.cpp b/src/AModule.cpp index 83bafbcf..65ffb433 100644 --- a/src/AModule.cpp +++ b/src/AModule.cpp @@ -238,9 +238,23 @@ bool AModule::handleUserEvent(GdkEventButton* const& e) { if (!format.empty()) { const int width = gdk_window_get_width(e->window); const int height = gdk_window_get_height(e->window); - const std::string cmd = - fmt::format(fmt::runtime(format), fmt::arg("x", (int)round(100. * e->x / width)), - fmt::arg("y", (int)round(100. * e->y / height))); + // Substitute {x}/{y} with the click position. The configured command is + // arbitrary user input that may contain literal braces which are not {x}/{y} + // (e.g. `echo ${HOME}`, `awk '{print $1}'`, brace expansions). Those make + // libfmt throw fmt::format_error; since we run inside a GTK signal handler an + // uncaught exception aborts the whole bar. Only format when a placeholder is + // actually present, and fall back to the raw command if formatting throws. + std::string cmd = format; + if (format.find("{x}") != std::string::npos || format.find("{y}") != std::string::npos) { + try { + cmd = fmt::format(fmt::runtime(format), fmt::arg("x", (int)round(100. * e->x / width)), + fmt::arg("y", (int)round(100. * e->y / height))); + } catch (const fmt::format_error& err) { + spdlog::warn("Failed to format command '{}': {}. Running it unformatted.", format, + err.what()); + cmd = format; + } + } pid_children_.push_back(util::command::forkExec(cmd)); } dp.emit(); From 48db4aa36e85fe1e0d67ac4b4c9ef8e576d71e70 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 5 Jul 2026 10:07:21 +0200 Subject: [PATCH 02/18] fix(bar): make disable-on-sleep DPMS suspend actually reach modules toggleSuspend dynamic_cast-ed the children of the left/center/right Gtk::Box. But modules are packed via AModule::operator Gtk::Widget&(), which returns the member event_box_, so every box child is a Gtk::EventBox and the cast is always null -- suspend()/resume() never ran, making disable-on-sleep a silent no-op. Iterate modules_all_ (the real module pointers) instead. Fixes disable-on-sleep DPMS suspend/resume never firing. --- src/bar.cpp | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/bar.cpp b/src/bar.cpp index cf284e0d..1eb5ce41 100644 --- a/src/bar.cpp +++ b/src/bar.cpp @@ -331,12 +331,14 @@ waybar::Bar::Bar(struct waybar_output* w_output, const Json::Value& w_config) * returned to the main loop, when any late initial configure has been dispatched and widgets have * had a chance to allocate/draw. */ - Glib::signal_idle().connect(sigc::track_obj([this] { - window.queue_resize(); - window.queue_draw(); - forceLayerCommit(); - return false; - }, *this)); + Glib::signal_idle().connect(sigc::track_obj( + [this] { + window.queue_resize(); + window.queue_draw(); + forceLayerCommit(); + return false; + }, + *this)); if (spdlog::should_log(spdlog::level::debug)) { // Unfortunately, this function isn't in the C++ bindings, so we have to call the C version. @@ -750,20 +752,18 @@ void waybar::Bar::onOutputGeometryChanged() { } void waybar::Bar::toggleSuspend(bool suspend) { - auto process_modules = [suspend](Gtk::Box& module_box) { - for (auto* widget : module_box.get_children()) { - auto* module = dynamic_cast(widget); - if (module && module->shouldSuspend()) { - if (suspend) { - module->suspend(); - } else { - module->resume(); - } + // Iterate the actual module objects. Modules are packed into the Gtk::Box via + // AModule::operator Gtk::Widget&(), which returns the member event_box_, so the + // box children are Gtk::EventBox, never AModule -- a dynamic_cast over them is + // always null and suspend()/resume() would never fire. modules_all_ holds the + // real module pointers (including group children), so use it instead. + for (auto const& module : modules_all_) { + if (module && module->shouldSuspend()) { + if (suspend) { + module->suspend(); + } else { + module->resume(); } } - }; - - process_modules(left_); - process_modules(center_); - process_modules(right_); + } } From 77734e9b02856b9195ee55e38eca627e6bcf0bc0 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 5 Jul 2026 10:07:29 +0200 Subject: [PATCH 03/18] custom-graph: fix worker crash, JSON UTF-8 validation, SIGRTMIN guard Bring src/modules/custom_graph.cpp in line with the hardened custom.cpp: - continuousWorker: on the restart path, an open() failure threw std::runtime_error out of the SleeperThread lambda, which escaped the thread and called std::terminate, killing all of Waybar. Log the error and stop the worker gracefully instead of throwing. - parseOutputJson: validate/make_valid the text/alt/tooltip JSON string fields before they reach fmt markup / set_tooltip_markup. Invalid UTF-8 from a script otherwise aborts the bar in g_utf8_* (parseOutputRaw already validated the same way). - refresh: wrap the SIGRTMIN-based signal check in #ifdef SIGRTMIN so the module builds on platforms without SIGRTMIN (e.g. some BSDs). Fixes a std::terminate crash on continuous-exec restart failure, an invalid-UTF-8 bar abort via JSON output, and a build break on platforms lacking SIGRTMIN. --- src/modules/custom_graph.cpp | 38 ++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/src/modules/custom_graph.cpp b/src/modules/custom_graph.cpp index 8b0c70af..06e45d06 100644 --- a/src/modules/custom_graph.cpp +++ b/src/modules/custom_graph.cpp @@ -99,7 +99,13 @@ void waybar::modules::CustomGraph::continuousWorker() { thread_.sleep_for(std::chrono::seconds(config_["restart-interval"].asUInt())); fp_ = util::command::open(cmd, pid_, output_name_); if (!fp_) { - throw std::runtime_error("Unable to open " + cmd); + // Letting this exception escape the SleeperThread would call + // std::terminate and kill all of Waybar. Degrade gracefully instead. + output_ = {1, ""}; + dp.emit(); + spdlog::error("Unable to restart {}: unable to open {}", name_, cmd); + thread_.stop(); + return; } } else { thread_.stop(); @@ -139,9 +145,11 @@ void waybar::modules::CustomGraph::waitingWorker() { } void waybar::modules::CustomGraph::refresh(int sig) { +#ifdef SIGRTMIN if (config_["signal"].isInt() && sig == SIGRTMIN + config_["signal"].asInt()) { thread_.wake_up(); } +#endif } void waybar::modules::CustomGraph::handleEvent() { @@ -268,22 +276,32 @@ void waybar::modules::CustomGraph::parseOutputJson() { std::istringstream output(output_.out); std::string line; class_.clear(); + // A script can emit invalid UTF-8; passing it unchecked to Pango/GTK aborts + // the whole bar in g_utf8_* (see parseOutputRaw, which validates the same way). + auto sanitize = [](const std::string& s) -> Glib::ustring { + Glib::ustring value = s; + if (!value.validate()) { + value = value.make_valid(); + } + return value; + }; while (getline(output, line)) { auto parsed = parser_.parse(line); - if (config_["escape"].isBool() && config_["escape"].asBool()) { - text_ = Glib::Markup::escape_text(parsed["text"].asString()); + const bool escape = config_["escape"].isBool() && config_["escape"].asBool(); + if (escape) { + text_ = Glib::Markup::escape_text(sanitize(parsed["text"].asString())); } else { - text_ = parsed["text"].asString(); + text_ = sanitize(parsed["text"].asString()); } - if (config_["escape"].isBool() && config_["escape"].asBool()) { - alt_ = Glib::Markup::escape_text(parsed["alt"].asString()); + if (escape) { + alt_ = Glib::Markup::escape_text(sanitize(parsed["alt"].asString())); } else { - alt_ = parsed["alt"].asString(); + alt_ = sanitize(parsed["alt"].asString()); } - if (config_["escape"].isBool() && config_["escape"].asBool()) { - tooltip_ = Glib::Markup::escape_text(parsed["tooltip"].asString()); + if (escape) { + tooltip_ = Glib::Markup::escape_text(sanitize(parsed["tooltip"].asString())); } else { - tooltip_ = parsed["tooltip"].asString(); + tooltip_ = sanitize(parsed["tooltip"].asString()); } if (parsed["class"].isString()) { class_.push_back(parsed["class"].asString()); From c16e7efa135e6516cd7321783a81370ee5cbe9cf Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 5 Jul 2026 10:06:55 +0200 Subject: [PATCH 04/18] fix(niri): close the IPC socket fd once (ScopedFd owns it), not twice IPC::send() wrapped the socket fd in a util::ScopedFd, which closes the fd in its destructor. The input stream was created with close_fd=true, so the stream also closed the same fd, resulting in a double-close. In multithreaded Waybar another thread can open a new fd with the same number between the two close() calls, which the second close() then wrongly closes. Pass close_fd=false so ScopedFd is the sole owner and the fd is closed exactly once. The streams are declared after socketfd, so they flush and destruct while the fd is still open, then ScopedFd closes it. --- src/modules/niri/backend.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/niri/backend.cpp b/src/modules/niri/backend.cpp index 460754cc..19eeada0 100644 --- a/src/modules/niri/backend.cpp +++ b/src/modules/niri/backend.cpp @@ -285,7 +285,7 @@ void IPC::unregisterForIPC(EventHandler* ev_handler) { Json::Value IPC::send(const Json::Value& request) { util::ScopedFd socketfd(connectToSocket()); - auto unix_istream = Gio::UnixInputStream::create(socketfd, true); + auto unix_istream = Gio::UnixInputStream::create(socketfd, false); auto unix_ostream = Gio::UnixOutputStream::create(socketfd, false); auto istream = Gio::DataInputStream::create(unix_istream); auto ostream = Gio::DataOutputStream::create(unix_ostream); From 4764a62afc34d6662f77fcf122cc0b256e17d223 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 5 Jul 2026 10:07:10 +0200 Subject: [PATCH 05/18] fix(wwan): null members after unref and guard destructor to avoid double-free The destructor unconditionally unref'd current_modem, manager and connection, but the constructor can leave them NULL or already-unref'd: - On the mm_manager_new_sync failure path the ctor unref'd connection without nulling it, so the dtor unref'd it a second time -> double-free. - On the g_bus_get_sync failure path all three stay NULL, and in the common no-WWAN-hardware case current_modem is NULL, so the dtor ran g_object_unref(NULL) -> G_IS_OBJECT assertion criticals. Use g_clear_object() in the failing ctor path (unref + null) and in the destructor (NULL-safe unref + null). Teardown is now safe for every ctor outcome (bus fail, MM fail, no modem, normal), and a normal run still unrefs each owned ref exactly once. --- src/modules/wwan.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/modules/wwan.cpp b/src/modules/wwan.cpp index 86e68624..c741405d 100644 --- a/src/modules/wwan.cpp +++ b/src/modules/wwan.cpp @@ -43,7 +43,7 @@ waybar::modules::Wwan::Wwan(const std::string& id, const Json::Value& config) if (error) { spdlog::error("Failed to create ModemManager proxy: " + std::string(error->message)); g_error_free(error); - g_object_unref(connection); + g_clear_object(&connection); return; } @@ -302,7 +302,7 @@ auto waybar::modules::Wwan::update() -> void { } waybar::modules::Wwan::~Wwan() { - g_object_unref(current_modem); - g_object_unref(manager); - g_object_unref(connection); + g_clear_object(¤t_modem); + g_clear_object(&manager); + g_clear_object(&connection); } From 30dcd7a7ca70deb405a40a9d025c38d7f4b6d99e Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 5 Jul 2026 10:09:35 +0200 Subject: [PATCH 06/18] fix(hyprland/workspaces): own debounce timer on main thread, fix UAF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The debounce timer added for flicker prevention was armed from the IPC listener thread via Glib::signal_timeout().connect(), while its timeout lambda and the m_updatePending flag ran on the GTK main thread — an unsynchronized cross-thread data race on GLib timer/main-loop state. Additionally ~Workspaces() never disconnected the timer, so a pending timeout could fire on a freed 'this' (use-after-free). Restore the pre-refactor threading model: onEvent now only mutates state under m_mutex on the IPC thread and calls dp.emit() (Glib::Dispatcher is thread-safe). The debounce timer is owned entirely by the main-thread update() path, which arms/re-arms it on each dispatch and coalesces bursts into a single refresh. ~Workspaces() disconnects the timer (guarded) so none outlives the object. Debounce behavior is preserved. --- include/modules/hyprland/workspaces.hpp | 3 +- src/modules/hyprland/workspaces.cpp | 43 ++++++++++++++----------- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/include/modules/hyprland/workspaces.hpp b/include/modules/hyprland/workspaces.hpp index 4be7fd44..1237ef24 100644 --- a/include/modules/hyprland/workspaces.hpp +++ b/include/modules/hyprland/workspaces.hpp @@ -227,8 +227,9 @@ class Workspaces : public AModule, public EventHandler { sigc::connection m_scrollEventConnection_; IPC& m_ipc; + // Coalesces bursts of Hyprland events into a single UI refresh. Armed and + // disconnected only on the GTK main thread (see Workspaces::update). sigc::connection m_debounceTimer; - bool m_updatePending = false; }; } // namespace waybar::modules::hyprland diff --git a/src/modules/hyprland/workspaces.cpp b/src/modules/hyprland/workspaces.cpp index 4cd0d2ae..202b892b 100644 --- a/src/modules/hyprland/workspaces.cpp +++ b/src/modules/hyprland/workspaces.cpp @@ -37,6 +37,11 @@ Workspaces::~Workspaces() { if (m_scrollEventConnection_.connected()) { m_scrollEventConnection_.disconnect(); } + // Cancel any pending debounce timeout so it cannot fire on a freed `this`. + // Runs on the main thread, same as where the timer is armed. + if (m_debounceTimer.connected()) { + m_debounceTimer.disconnect(); + } m_ipc.unregisterForIPC(this); // wait for possible event handler to finish std::lock_guard lg(m_mutex); @@ -332,23 +337,11 @@ void Workspaces::onEvent(const std::string& ev) { } } - if (m_debounceTimer.connected()) { - m_debounceTimer.disconnect(); - m_updatePending = false; - } - - m_updatePending = true; - m_debounceTimer = Glib::signal_timeout().connect( - [this]() { - if (!m_updatePending) return false; - std::lock_guard lock(m_mutex); - if (m_updatePending) { - dp.emit(); - m_updatePending = false; - } - return false; - }, - 7); + // Notify the main thread. dp (Glib::Dispatcher) is the only thread-safe way to + // hand off to the GTK main loop; GLib timer state must never be touched from the + // IPC listener thread. The debounce timer is owned entirely by the main-thread + // update() path (see Workspaces::update). + dp.emit(); } void Workspaces::onWorkspaceActivated(std::string const& payload) { @@ -1041,8 +1034,20 @@ void Workspaces::setUrgentWorkspace(std::string const& windowaddress) { } auto Workspaces::update() -> void { - doUpdate(); - AModule::update(); + // Debounce rapid events (e.g. out-of-order create/destroy workspace events from + // Hyprland) to prevent workspace button flicker. This runs on the GTK main thread + // (invoked via the dp dispatcher), so arming/disconnecting the GLib timer here is + // thread-safe. Each event re-arms the timer, coalescing bursts into one refresh. + if (m_debounceTimer.connected()) { + m_debounceTimer.disconnect(); + } + m_debounceTimer = Glib::signal_timeout().connect( + [this]() { + doUpdate(); + AModule::update(); + return false; + }, + 7); } void Workspaces::updateWindowCount() { From 74cf45d53017da6ab9ccf54b6740d488d2bfa977 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 5 Jul 2026 10:11:59 +0200 Subject: [PATCH 07/18] fix(hyprland): detect Lua protocol without side-effecting dispatch isLuaProtocol() probed the protocol by sending a real "dispatch workspace __waybar_probe__". On Hyprland < 0.54 "workspace" is a valid dispatcher, so the probe actually switched the user to a junk workspace named __waybar_probe__ on the first workspace click/scroll. Detect the protocol with the read-only "version" IPC query instead: parse the numeric "version" field (falling back to the always-present "tag" field) and treat Hyprland >= 0.54 as Lua. This has no side effects. On any parse/query failure we log and fall back to the legacy protocol, preserving prior behavior for older versions. --- src/modules/hyprland/backend.cpp | 39 ++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/src/modules/hyprland/backend.cpp b/src/modules/hyprland/backend.cpp index f38f8216..891bed1a 100644 --- a/src/modules/hyprland/backend.cpp +++ b/src/modules/hyprland/backend.cpp @@ -297,11 +297,40 @@ bool IPC::isLuaProtocol() { return *s_luaProtocolDetected_; } - // Probe: send a harmless old-style dispatch and check the error. - // In Lua-based Hyprland (>= 0.54) the error contains "hl.dispatch". - // In older versions it returns "ok" or a different error. - auto reply = getSocket1Reply("dispatch workspace __waybar_probe__"); - bool luaProto = reply.find("hl.dispatch") != std::string::npos; + // Detect the Lua-based dispatch protocol (Hyprland >= 0.54) via the read-only + // "version" query. This MUST have no side effects: an earlier probe issued a real + // "dispatch workspace __waybar_probe__", which on Hyprland < 0.54 actually switched + // the user to a junk workspace named __waybar_probe__ on the first click/scroll. + bool luaProto = false; + try { + util::JsonParser parser; + const Json::Value ver = parser.parse(getSocket1Reply("j/version")); + + // Prefer the numeric "version" field ("0.54.0"); fall back to the "tag" field + // ("v0.54.0" or "v0.54.0-16-gdeadbee"), which is present on all releases. + std::string versionStr = ver["version"].asString(); + if (versionStr.empty()) { + versionStr = ver["tag"].asString(); + } + + const size_t firstDigit = versionStr.find_first_of("0123456789"); + if (firstDigit != std::string::npos) { + // std::stoi parses the leading integer and stops at the first non-digit, so it + // tolerates the trailing ".patch-commits-ghash" suffix on the tag. + const int major = std::stoi(versionStr.substr(firstDigit)); + int minor = 0; + const size_t dot = versionStr.find('.', firstDigit); + if (dot != std::string::npos && dot + 1 < versionStr.size()) { + minor = std::stoi(versionStr.substr(dot + 1)); + } + luaProto = major > 0 || (major == 0 && minor >= 54); + } else { + spdlog::warn("Hyprland IPC: could not parse version '{}', assuming legacy protocol", + versionStr); + } + } catch (const std::exception& e) { + spdlog::warn("Hyprland IPC: version detection failed ({}), assuming legacy protocol", e.what()); + } if (luaProto) { spdlog::info("Hyprland IPC: detected Lua-based dispatch protocol (Hyprland >= 0.54)"); From 66139e4440b626e0d92c4b902185ced10ce78df7 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 5 Jul 2026 10:11:59 +0200 Subject: [PATCH 08/18] fix(tray): stop reorderItems from re-adding items (iterator UAF + double add) The item-ordering feature made Host::reorderItems() re-run the full remove/add path over items_ via std::ranges::for_each(on_remove_/on_add_). This caused two confirmed bugs: BUG 1 (iterator invalidation / UAF): on_add_ (Tray::onAdd) calls Host::checkIgnoreList, which erases from items_ while for_each is still iterating items_, invalidating iterators/pointers. Triggered by a non-empty ignore-list matching an item with >=2 items present. BUG 2 (double add): reorderItems runs while an item's Id is resolved in proxyReady, i.e. before setReady(). It added the not-yet-ready item (re-parenting its event_box, pushing into Tray::items_, connecting signal_show/hide), then setReady() -> itemReady -> onAdd added it again: GTK 'widget already has a parent' critical, duplicate Item* and signal handlers that accumulated unbounded. Fixes: - reorderItems() now only reorders already-added GTK box children via a dedicated on_reorder_ callback (Tray::reorderBox), never re-adding or removing. reorderBox stable-sorts items_ by order_ and repositions children with gtk_box_reorder_child (honouring reverse-direction). - Tray::onAdd is idempotent (guards against an already-added item) and positions the new widget via reorderBox before the ignore-list check. - signal_show/signal_hide connections are stored per item and disconnected in Tray::onRemove; onRemove is a no-op for items that were never added. --- include/modules/sni/host.hpp | 7 +++++- include/modules/sni/tray.hpp | 10 ++++++++ src/modules/sni/host.cpp | 17 ++++++++----- src/modules/sni/tray.cpp | 49 +++++++++++++++++++++++++++++++++--- 4 files changed, 72 insertions(+), 11 deletions(-) diff --git a/include/modules/sni/host.hpp b/include/modules/sni/host.hpp index 49771fac..4dad1f94 100644 --- a/include/modules/sni/host.hpp +++ b/include/modules/sni/host.hpp @@ -16,7 +16,8 @@ class Host { public: Host(std::size_t id, const Json::Value&, const Bar&, const std::vector&, const std::function&)>&, - const std::function&)>&, const std::function&); + const std::function&)>&, const std::function&, + const std::function&); ~Host(); void checkIgnoreList(const std::vector& ignore_list, @@ -55,6 +56,10 @@ class Host { const std::vector ignore_list_; const std::function&)> on_add_; const std::function&)> on_remove_; + // Re-applies the configured ordering to the already-added tray widgets. This + // must NOT re-run the add path (which would re-parent widgets and reconnect + // signals); it only reorders existing children. + const std::function on_reorder_; ItemOrderMap orders_; const std::function on_update_; diff --git a/include/modules/sni/tray.hpp b/include/modules/sni/tray.hpp index a2fd08eb..c3ac4e34 100644 --- a/include/modules/sni/tray.hpp +++ b/include/modules/sni/tray.hpp @@ -1,6 +1,10 @@ #pragma once #include +#include + +#include +#include #include "AModule.hpp" #include "bar.hpp" @@ -19,6 +23,9 @@ class Tray : public AModule { private: void onAdd(std::unique_ptr& item); void onRemove(std::unique_ptr& item); + // Reorders the already-added tray widgets by their configured order. Does not + // add or remove any widget. + void reorderBox(); void checkIgnoreList(std::unique_ptr* item); std::vector parseIgnoreList(const Json::Value& config); void queueUpdate(); @@ -29,6 +36,9 @@ class Tray : public AModule { std::vector ignore_list_; SNI::Host host_; std::vector items_; + // signal_show/signal_hide connections owned per added item, so they can be + // disconnected on removal instead of leaking and accumulating. + std::unordered_map> item_connections_; }; } // namespace waybar::modules::SNI diff --git a/src/modules/sni/host.cpp b/src/modules/sni/host.cpp index 272e05aa..c55c8f2e 100644 --- a/src/modules/sni/host.cpp +++ b/src/modules/sni/host.cpp @@ -16,7 +16,7 @@ Host::Host(std::size_t id, const Json::Value& config, const Bar& bar, const std::vector& ignore_list, const std::function&)>& on_add, const std::function&)>& on_remove, - const std::function& on_update) + const std::function& on_reorder, const std::function& on_update) : bus_name_("org.kde.StatusNotifierHost-" + std::to_string(getpid()) + "-" + std::to_string(id)), object_path_("/StatusNotifierHost/" + std::to_string(id)), @@ -27,6 +27,7 @@ Host::Host(std::size_t id, const Json::Value& config, const Bar& bar, ignore_list_(ignore_list), on_add_(on_add), on_remove_(on_remove), + on_reorder_(on_reorder), on_update_(on_update) { auto orders = config["orders"]; if (!orders.isNull()) { @@ -292,11 +293,15 @@ void Host::addRegisteredItem(const std::string& service) { } void Host::reorderItems() { - std::ranges::for_each(items_, on_remove_); - std::ranges::sort(items_, [](std::unique_ptr& item1, std::unique_ptr& item2) { - return item1->order_ < item2->order_; - }); - std::ranges::for_each(items_, on_add_); + // Re-apply the configured ordering to the tray. This is invoked while an + // item's Id/order is first resolved (from Item::setCustomIcon), which happens + // *before* the item is marked ready and added. It must therefore only reorder + // the widgets that have already been added; re-running the full add path here + // would (a) re-parent widgets and reconnect signals for every item and (b) + // mutate items_ from within checkIgnoreList while it is being iterated, + // invalidating iterators/pointers. Delegating to on_reorder_ keeps this to a + // pure reordering of existing children. + on_reorder_(); } } // namespace waybar::modules::SNI diff --git a/src/modules/sni/tray.cpp b/src/modules/sni/tray.cpp index f3e413a8..285c103f 100644 --- a/src/modules/sni/tray.cpp +++ b/src/modules/sni/tray.cpp @@ -38,7 +38,7 @@ Tray::Tray(const std::string& id, const Bar& bar, const Json::Value& config) host_((initIconsConfig(config), nb_hosts_), config, bar, ignore_list_, std::bind(&Tray::onAdd, this, std::placeholders::_1), std::bind(&Tray::onRemove, this, std::placeholders::_1), - std::bind(&Tray::queueUpdate, this)) { + std::bind(&Tray::reorderBox, this), std::bind(&Tray::queueUpdate, this)) { box_.set_name("tray"); event_box_.add(box_); if (!id.empty()) { @@ -63,6 +63,16 @@ void Tray::onAdd(std::unique_ptr& item) { spdlog::info("Tray::onAdd - item bus_name='{}', category='{}', icon_name='{}', title='{}'", item->bus_name, item->category, item->icon_name, item->title); + // Idempotency guard: onAdd can be reached more than once for the same item + // (e.g. an item is processed while its Id/order is resolved and then again + // when it becomes ready). Re-adding would re-parent the event_box (GTK + // "widget already has a parent" critical), push a duplicate pointer into + // items_ and leak extra signal connections. + if (std::find(items_.begin(), items_.end(), item.get()) != items_.end()) { + spdlog::debug("Tray::onAdd - item already added, skipping"); + return; + } + if (config_["reverse-direction"].isBool() && config_["reverse-direction"].asBool()) { box_.pack_end(item->event_box); } else { @@ -70,8 +80,13 @@ void Tray::onAdd(std::unique_ptr& item) { } items_.push_back(item.get()); - item->event_box.signal_show().connect([this] { dp.emit(); }); - item->event_box.signal_hide().connect([this] { dp.emit(); }); + auto show_conn = item->event_box.signal_show().connect([this] { dp.emit(); }); + auto hide_conn = item->event_box.signal_hide().connect([this] { dp.emit(); }); + item_connections_[item.get()] = {show_conn, hide_conn}; + + // Position the freshly added widget according to the configured order. This + // must happen before the ignore-list check below, which may erase `item`. + reorderBox(); // After this point `item` may be erased/invalidated by the ignore-list check; // do not touch it again below. @@ -82,11 +97,37 @@ void Tray::onAdd(std::unique_ptr& item) { } void Tray::onRemove(std::unique_ptr& item) { + // May be called for items that were never added (e.g. the ignore-list check + // runs over items that are not yet ready). Only touch state we actually own. + auto it = std::find(items_.begin(), items_.end(), item.get()); + if (it == items_.end()) { + return; + } + + auto conn_it = item_connections_.find(item.get()); + if (conn_it != item_connections_.end()) { + conn_it->second.first.disconnect(); + conn_it->second.second.disconnect(); + item_connections_.erase(conn_it); + } + box_.remove(item->event_box); - items_.erase(std::remove(items_.begin(), items_.end(), item.get()), items_.end()); + items_.erase(it); dp.emit(); } +void Tray::reorderBox() { + const bool reverse = + config_["reverse-direction"].isBool() && config_["reverse-direction"].asBool(); + // Stable sort keeps insertion order among items sharing the same order value. + std::stable_sort(items_.begin(), items_.end(), + [](const Item* a, const Item* b) { return a->order_ < b->order_; }); + for (std::size_t i = 0; i < items_.size(); ++i) { + const int pos = reverse ? static_cast(items_.size() - 1 - i) : static_cast(i); + box_.reorder_child(items_[i]->event_box, pos); + } +} + auto Tray::update() -> void { // Check if any items should be ignored now that properties have loaded if (!ignore_list_.empty()) { From 8db89cc289a84405c420bc7b066e46911c5cb5c2 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 5 Jul 2026 10:07:45 +0200 Subject: [PATCH 09/18] fix(mango): use xkb brief for shortDescription in language module Remove leftover unconditional assignment that clobbered the value computed from rxkb_layout_get_brief() with short_name, which made short_description always equal short_name and defeated format- / {shortDescription}. --- src/modules/mango/language.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/modules/mango/language.cpp b/src/modules/mango/language.cpp index 9415002a..549d22ed 100644 --- a/src/modules/mango/language.cpp +++ b/src/modules/mango/language.cpp @@ -110,8 +110,6 @@ Language::Layout Language::getLayout(const std::string& fullName) { short_description = short_name; } - short_description = short_name; - Layout info{desc, short_name, variant, short_description}; return info; } From 2f2479ca35eae795e5f2e5f721efa455164d7767 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 5 Jul 2026 10:09:26 +0200 Subject: [PATCH 10/18] fix(mango): reconnect IPC event thread on disconnect The IPC event thread had no reconnect: on POLLHUP/POLLERR/POLLNVAL or read()==0/error it broke out of the loop and the thread exited permanently, freezing every mango module with stale content until Waybar was restarted. Wrap the connect + poll/read loop in a reconnect loop with a bounded 2s backoff, re-establishing the socket and resuming on disconnect, modeled on the niri backend. Add an atomic running_ flag so the worker exits cleanly on teardown; the destructor now sets it false before closing the socket so the worker breaks out and joins, and leaves the final close to the destructor to avoid a double close. --- include/modules/mango/backend.hpp | 2 + src/modules/mango/backend.cpp | 131 +++++++++++++++++++++--------- 2 files changed, 93 insertions(+), 40 deletions(-) diff --git a/include/modules/mango/backend.hpp b/include/modules/mango/backend.hpp index a1b0392e..7e4313f3 100644 --- a/include/modules/mango/backend.hpp +++ b/include/modules/mango/backend.hpp @@ -1,6 +1,7 @@ // include/modules/mango/backend.hpp #pragma once +#include #include #include #include @@ -52,6 +53,7 @@ class IPC { static Json::Value sendCommand(const std::string& cmd); + std::atomic running_ = true; int sockfd_ = -1; std::thread ipc_thread_; mutable std::mutex data_mutex_; diff --git a/src/modules/mango/backend.cpp b/src/modules/mango/backend.cpp index 5083a7fc..63aecb7b 100644 --- a/src/modules/mango/backend.cpp +++ b/src/modules/mango/backend.cpp @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -100,71 +101,121 @@ void IPC::sendAsync(const Json::Value& request) { IPC::IPC() : sockfd_(-1), active_client_(Json::nullValue) { startIPC(); } IPC::~IPC() { + running_ = false; if (sockfd_ != -1) close(sockfd_); if (ipc_thread_.joinable()) ipc_thread_.join(); } void IPC::startIPC() { + // Connect synchronously so a missing socket (this WM isn't the active + // compositor) throws here and lets the module constructor fail, instead of + // the module always attaching with a permanently empty widget. sockfd_ = IPC::connectToSocket(); ipc_thread_ = std::thread([this]() { spdlog::info("Mango IPC thread started"); - struct pollfd pfd; - pfd.fd = sockfd_; - pfd.events = POLLIN; - - const std::vector subs = {"watch all-monitors"}; - for (const auto& cmd : subs) { - if (write(sockfd_, cmd.c_str(), cmd.size()) != (ssize_t)cmd.size() || - write(sockfd_, "\n", 1) != 1) { - spdlog::error("Failed to subscribe to {}", cmd); - return; - } - } - char buf[4096]; std::string buffer; - while (true) { - int ret = poll(&pfd, 1, 1000); - if (ret == 0) continue; - if (ret < 0) { - if (errno == EINTR) continue; - spdlog::error("IPC poll error: {}", strerror(errno)); - break; - } + bool have_initial_fd = true; - if (pfd.revents & (POLLERR | POLLHUP | POLLNVAL)) { - spdlog::info("Mango IPC socket closed or invalid"); - break; + // Reconnect loop: if the event stream drops (POLLHUP/POLLERR, read()==0 or + // an error) we back off briefly and re-establish the socket instead of + // leaving every mango module frozen forever with stale content. + while (running_) { + if (!have_initial_fd) { + try { + sockfd_ = IPC::connectToSocket(); + } catch (const std::exception& e) { + spdlog::error("Mango IPC: failed to reconnect: {}", e.what()); + std::this_thread::sleep_for(std::chrono::seconds(2)); + continue; + } } + have_initial_fd = false; - if (pfd.revents & POLLIN) { - ssize_t n = read(sockfd_, buf, sizeof(buf)); - if (n == 0) { - spdlog::info("Mango IPC connection closed"); + bool subscribed = true; + const std::vector subs = {"watch all-monitors"}; + for (const auto& cmd : subs) { + if (write(sockfd_, cmd.c_str(), cmd.size()) != (ssize_t)cmd.size() || + write(sockfd_, "\n", 1) != 1) { + spdlog::error("Failed to subscribe to {}", cmd); + subscribed = false; break; } - if (n < 0) { + } + if (!subscribed) { + if (sockfd_ != -1) { + close(sockfd_); + sockfd_ = -1; + } + std::this_thread::sleep_for(std::chrono::seconds(2)); + continue; + } + + struct pollfd pfd; + pfd.fd = sockfd_; + pfd.events = POLLIN; + buffer.clear(); + + bool connected = true; + while (running_ && connected) { + int ret = poll(&pfd, 1, 1000); + if (ret == 0) continue; + if (ret < 0) { if (errno == EINTR) continue; - spdlog::error("IPC read error: {}", strerror(errno)); + spdlog::error("IPC poll error: {}", strerror(errno)); + connected = false; break; } - buffer.append(buf, n); - size_t pos; - while ((pos = buffer.find('\n')) != std::string::npos) { - std::string line = buffer.substr(0, pos); - buffer.erase(0, pos + 1); - if (line.empty()) continue; - try { - parseIPC(line); - } catch (const std::exception& e) { - spdlog::warn("Failed to parse IPC line: {} - {}", line, e.what()); + if (pfd.revents & (POLLERR | POLLHUP | POLLNVAL)) { + spdlog::info("Mango IPC socket closed or invalid"); + connected = false; + break; + } + + if (pfd.revents & POLLIN) { + ssize_t n = read(sockfd_, buf, sizeof(buf)); + if (n == 0) { + spdlog::info("Mango IPC connection closed"); + connected = false; + break; + } + if (n < 0) { + if (errno == EINTR) continue; + spdlog::error("IPC read error: {}", strerror(errno)); + connected = false; + break; + } + buffer.append(buf, n); + + size_t pos; + while ((pos = buffer.find('\n')) != std::string::npos) { + std::string line = buffer.substr(0, pos); + buffer.erase(0, pos + 1); + if (line.empty()) continue; + try { + parseIPC(line); + } catch (const std::exception& e) { + spdlog::warn("Failed to parse IPC line: {} - {}", line, e.what()); + } } } } + + // On shutdown leave the socket for the destructor to close (avoids a + // double close); on a genuine disconnect close it before reconnecting. + if (!running_) break; + if (sockfd_ != -1) { + close(sockfd_); + sockfd_ = -1; + } + spdlog::warn("Mango IPC: event stream closed, reconnecting"); + std::this_thread::sleep_for(std::chrono::seconds(2)); } + + spdlog::info("Mango IPC thread stopping"); }); } From 892ab479ba3c4897ed3f0532413c76f30a7b91a1 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 5 Jul 2026 10:10:10 +0200 Subject: [PATCH 11/18] group: disconnect pending reveal timeout in destructor to fix UAF With reveal-delay set, handleMouseEnter arms a Glib::signal_timeout that captures 'this'. sigc::connection's destructor does not remove the GLib source, so a Group destroyed with a pending reveal timer would fire the timeout on freed memory. Add a destructor that disconnects reveal_timeout_. --- include/group.hpp | 2 +- src/group.cpp | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/include/group.hpp b/include/group.hpp index 0e981d0d..1e2a5d55 100644 --- a/include/group.hpp +++ b/include/group.hpp @@ -14,7 +14,7 @@ class Group : public AModule { public: Group(const std::string&, const std::string&, const Json::Value&, bool); - ~Group() override = default; + ~Group() override; auto update() -> void override; operator Gtk::Widget&() override; diff --git a/src/group.cpp b/src/group.cpp index 9f11661f..5fa04884 100644 --- a/src/group.cpp +++ b/src/group.cpp @@ -105,6 +105,14 @@ Group::Group(const std::string& name, const std::string& id, const Json::Value& event_box_.add(box); } +Group::~Group() { + // Disconnect any pending reveal timeout so it cannot fire on a destroyed + // instance (the GLib source is not removed by sigc::connection's destructor). + if (reveal_timeout_.connected()) { + reveal_timeout_.disconnect(); + } +} + void Group::show_group() { box.set_state_flags(Gtk::StateFlags::STATE_FLAG_PRELIGHT); revealer.set_reveal_child(true); From 8441d5e124658ff226cfed6ac640832d82f816a5 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 5 Jul 2026 10:10:10 +0200 Subject: [PATCH 12/18] config: harden output-dimensions parsing against malformed values The parser assumed each entry was ' '. An entry with no space caused str.substr((size_t)-1) to throw out_of_range, and a non-integer value made std::stoi throw invalid_argument, failing the whole bar on that output. Validate spaces with find()!=npos and wrap stoi in try/catch; log a warning and skip malformed entries instead of throwing. --- src/config.cpp | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/src/config.cpp b/src/config.cpp index ad136339..800b6b02 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -240,12 +240,33 @@ bool isValidOutput(const Json::Value& config, const std::string& name, continue; } std::string str = config_output_dimension.asString(); - int i = str.find(" "); - std::string dimension = str.substr(0, i); - str = str.substr(i + 1); - i = str.find(" "); - std::string comparator = str.substr(0, i); - int value = std::stoi(str.substr(i)); + auto first_space = str.find(' '); + if (first_space == std::string::npos) { + spdlog::warn( + "Ignoring malformed 'output-dimensions' entry (expected ' " + "'): '{}'", + str); + continue; + } + std::string dimension = str.substr(0, first_space); + str = str.substr(first_space + 1); + auto second_space = str.find(' '); + if (second_space == std::string::npos) { + spdlog::warn( + "Ignoring malformed 'output-dimensions' entry (expected ' " + "'): '{}'", + config_output_dimension.asString()); + continue; + } + std::string comparator = str.substr(0, second_space); + int value; + try { + value = std::stoi(str.substr(second_space + 1)); + } catch (const std::exception& e) { + spdlog::warn("Ignoring 'output-dimensions' entry with non-integer value: '{}'", + config_output_dimension.asString()); + continue; + } int comparison_value; if (dimension == "height") { From 91ca603b04cdb95a330ae054f049dc32578ab325 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 5 Jul 2026 10:10:10 +0200 Subject: [PATCH 13/18] keyboard_state: close fd when openDevice throws to fix fd leak openDevice() throws without closing the fd if libevdev_new_from_fd fails. In both update() and tryAddDevice() the outer catch only logged, so closeFile(fd) was never reached and a descriptor leaked on every failing tick. Guard openDevice with a try/catch that closes the fd before rethrowing. --- src/modules/keyboard_state.cpp | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/modules/keyboard_state.cpp b/src/modules/keyboard_state.cpp index 87a843c4..fa80ef64 100644 --- a/src/modules/keyboard_state.cpp +++ b/src/modules/keyboard_state.cpp @@ -320,7 +320,14 @@ auto waybar::modules::KeyboardState::update() -> void { for (const auto& dev_path : dev_paths) { try { int fd = openFile(dev_path, O_NONBLOCK | O_CLOEXEC | O_RDONLY); - auto dev = openDevice(fd); + libevdev* dev; + try { + dev = openDevice(fd); + } catch (...) { + // openDevice does not close the fd if libevdev_new_from_fd fails. + closeFile(fd); + throw; + } numl |= libevdev_get_event_value(dev, EV_LED, LED_NUML); capsl |= libevdev_get_event_value(dev, EV_LED, LED_CAPSL); scrolll |= libevdev_get_event_value(dev, EV_LED, LED_SCROLLL); @@ -376,7 +383,14 @@ auto waybar::modules::KeyboardState::update() -> void { auto waybar::modules ::KeyboardState::tryAddDevice(const std::string& dev_path) -> void { try { int fd = openFile(dev_path, O_NONBLOCK | O_CLOEXEC | O_RDONLY); - auto dev = openDevice(fd); + libevdev* dev; + try { + dev = openDevice(fd); + } catch (...) { + // openDevice does not close the fd if libevdev_new_from_fd fails. + closeFile(fd); + throw; + } if (supportsLockStates(dev)) { spdlog::info("Found device {} at '{}'", libevdev_get_name(dev), dev_path); std::lock_guard lock(devices_mutex_); From c9012c4107204301f473620606a0931e7e1d95a1 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 5 Jul 2026 10:10:10 +0200 Subject: [PATCH 14/18] sway/window: escape window title in tooltip markup setTooltipMarkup uses set_tooltip_markup without escaping. Raw window titles routinely contain &, < and >, which break Pango markup parsing and the tooltip. Escape the title with Glib::Markup::escape_text before passing it. --- src/modules/sway/window.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/sway/window.cpp b/src/modules/sway/window.cpp index f005bd14..4faa7d77 100644 --- a/src/modules/sway/window.cpp +++ b/src/modules/sway/window.cpp @@ -101,7 +101,7 @@ auto Window::update() -> void { fmt::arg("shell", shell_), fmt::arg("marks", marks_)), config_["rewrite"])); if (tooltipEnabled()) { - setTooltipMarkup(window_); + setTooltipMarkup(Glib::Markup::escape_text(window_)); } // Resolve the app icon on the main thread to avoid racing with GTK draw on the From b17743ff6a5b54b7b6c4cc38218289deb8bdb202 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 5 Jul 2026 10:10:10 +0200 Subject: [PATCH 15/18] network: treat negative/failed link speed as 0 /sys/class/net//speed reports -1 with no carrier. Reading it into a uint32_t wrapped to 4294967295 (without setting failbit), so {linkSpeed} showed an absurd value. Read into int64_t, check fail(), and treat negative or failed reads as 0. --- src/modules/network.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/modules/network.cpp b/src/modules/network.cpp index 7b4dc15f..72a367fc 100644 --- a/src/modules/network.cpp +++ b/src/modules/network.cpp @@ -90,13 +90,16 @@ uint32_t waybar::modules::Network::readLinkSpeed() const { if (!sysfs_speed) return 0; - uint32_t speed; + // Read into a signed type: /sys/class/net//speed reports -1 when there is + // no carrier. Extracting -1 into an unsigned type would wrap to a huge value + // (and would not set failbit), so use a signed type and validate the result. + int64_t speed = 0; sysfs_speed >> speed; - if (sysfs_speed.bad()) // read fails on incompatible devices + if (sysfs_speed.fail() || speed < 0) // read fails on incompatible devices return 0; - return speed; + return static_cast(speed); } waybar::modules::Network::Network(const std::string& id, const Json::Value& config) From 88064137ca8f5259dda800d784ccb9e2bb2f65b7 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 5 Jul 2026 10:10:26 +0200 Subject: [PATCH 16/18] fix(mpris): clear stale GError in playerctld selection loop When playerctl_player_new_from_name() fails for a candidate player, the loop continued without clearing the GError. The stale non-NULL error then leaked into the next GLib call (GLib-CRITICAL assertion) and made the post-loop 'if (error) goto errorexit' fire even when a valid playing player had been selected, blanking the whole module. Clear the error at the discard point with g_clear_error(). --- src/modules/mpris/mpris.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/modules/mpris/mpris.cpp b/src/modules/mpris/mpris.cpp index 387e0597..d70f5902 100644 --- a/src/modules/mpris/mpris.cpp +++ b/src/modules/mpris/mpris.cpp @@ -491,7 +491,13 @@ auto Mpris::getPlayerInfo() -> std::optional { continue; } auto* tmp = playerctl_player_new_from_name(pn, &error); - if (error || !tmp) continue; + if (error || !tmp) { + // Discard any error from this candidate so it doesn't leak into the next + // playerctl_player_new_from_name() call or the post-loop metadata calls, which + // assert that the passed GError is NULL (otherwise: GLib-CRITICAL / spurious errorexit). + g_clear_error(&error); + continue; + } if (!first_valid_player) { first_valid_player = tmp; first_valid_name = name; From 34522b4ccdc60bd91bff543505ac44b5c4bff816 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 5 Jul 2026 10:10:33 +0200 Subject: [PATCH 17/18] fix(clock): guard tooltip formatting against unsupported specifiers Only the label vformat was wrapped in try/catch. An unsupported specifier (e.g. %-I / %OI) in tooltip-format or the calendar format still threw out of update() every tick via the calendar/tooltip vformat calls. Wrap the tooltip-building section in try/catch that warns once and skips the tooltip for that tick instead of letting the exception escape update(). --- src/modules/clock.cpp | 41 ++++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/src/modules/clock.cpp b/src/modules/clock.cpp index 38dec4b2..bd0c828d 100644 --- a/src/modules/clock.cpp +++ b/src/modules/clock.cpp @@ -220,20 +220,35 @@ auto waybar::modules::Clock::update() -> void { if (tzInTooltip_) tzText_ = getTZtext(now.get_sys_time()); if (cldInTooltip_) cldText_ = get_calendar(today, shiftedDay, tz); if (ordInTooltip_) ordText_ = get_ordinal_date(shiftedDay); - if (tzInTooltip_ || cldInTooltip_ || ordInTooltip_) { - // std::vformat doesn't support named arguments. - m_tlpText_ = - std::regex_replace(m_tlpFmt_, std::regex("\\{" + kTZPlaceholder + "\\}"), tzText_); - m_tlpText_ = std::regex_replace( - m_tlpText_, std::regex("\\{" + kCldPlaceholder + "\\}"), - fmt_lib::vformat(m_locale_, cldText_, fmt_lib::make_format_args(shiftedNow))); - m_tlpText_ = - std::regex_replace(m_tlpText_, std::regex("\\{" + kOrdPlaceholder + "\\}"), ordText_); - } else { - m_tlpText_ = m_tlpFmt_; - } + try { + if (tzInTooltip_ || cldInTooltip_ || ordInTooltip_) { + // std::vformat doesn't support named arguments. + m_tlpText_ = + std::regex_replace(m_tlpFmt_, std::regex("\\{" + kTZPlaceholder + "\\}"), tzText_); + m_tlpText_ = std::regex_replace( + m_tlpText_, std::regex("\\{" + kCldPlaceholder + "\\}"), + fmt_lib::vformat(m_locale_, cldText_, fmt_lib::make_format_args(shiftedNow))); + m_tlpText_ = + std::regex_replace(m_tlpText_, std::regex("\\{" + kOrdPlaceholder + "\\}"), ordText_); + } else { + m_tlpText_ = m_tlpFmt_; + } - m_tlpText_ = fmt_lib::vformat(m_locale_, m_tlpText_, fmt_lib::make_format_args(now)); + m_tlpText_ = fmt_lib::vformat(m_locale_, m_tlpText_, fmt_lib::make_format_args(now)); + } catch (const std::exception& e) { + // An unsupported/invalid specifier (e.g. %-I / %OI) in the tooltip-format or the + // calendar format must not take the whole module down every tick. Warn once and skip + // the tooltip for this update so the bar keeps working. + static bool tlpWarned = false; + if (!tlpWarned) { + spdlog::warn( + "Clock: could not format tooltip \"{}\": {}. Skipping tooltip; check your " + "tooltip-format/calendar format specifiers.", + m_tlpFmt_, e.what()); + tlpWarned = true; + } + m_tlpText_.clear(); + } // Pango doesn't support CSS classes but to continue using it while staying // backwards compatible this approach uses post-posting to replace fake From fc6a567974b61481334d034c8dbcc4b0a23702e7 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 5 Jul 2026 10:10:38 +0200 Subject: [PATCH 18/18] fix(systemd-failed-units): guard label/tooltip format against bad config A malformed user format or tooltip-format (unknown {placeholder}) made fmt::format throw fmt::format_error out of update(). Wrap the label and tooltip format calls in try/catch that warn once and fall back to a safe label / skip the tooltip instead of taking the module down. --- src/modules/systemd_failed_units.cpp | 43 +++++++++++++++++++++------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/src/modules/systemd_failed_units.cpp b/src/modules/systemd_failed_units.cpp index faa1ca7e..3804cbe2 100644 --- a/src/modules/systemd_failed_units.cpp +++ b/src/modules/systemd_failed_units.cpp @@ -281,21 +281,42 @@ auto SystemdFailedUnits::update() -> void { last_status_ = overall_state_; - setLabelMarkup(fmt::format( - fmt::runtime(nr_failed_ == 0 ? format_ok_ : format_), fmt::arg("nr_failed", nr_failed_), - fmt::arg("nr_failed_system", nr_failed_system_), fmt::arg("nr_failed_user", nr_failed_user_), - fmt::arg("system_state", system_state_), fmt::arg("user_state", user_state_), - fmt::arg("overall_state", overall_state_))); + // A malformed user format/tooltip-format (e.g. an unknown {placeholder}) makes fmt throw a + // fmt::format_error; catch it so a bad config warns once instead of taking update() down. + try { + setLabelMarkup(fmt::format( + fmt::runtime(nr_failed_ == 0 ? format_ok_ : format_), fmt::arg("nr_failed", nr_failed_), + fmt::arg("nr_failed_system", nr_failed_system_), + fmt::arg("nr_failed_user", nr_failed_user_), fmt::arg("system_state", system_state_), + fmt::arg("user_state", user_state_), fmt::arg("overall_state", overall_state_))); + } catch (const std::exception& e) { + static bool labelWarned = false; + if (!labelWarned) { + spdlog::warn("systemd-failed-units: invalid format, using fallback: {}", e.what()); + labelWarned = true; + } + setLabelMarkup(fmt::format("{} failed", nr_failed_)); + } if (tooltipEnabled()) { std::string failed_list = BuildTooltipFailedList(); auto tooltip_template = overall_state_ == "ok" ? tooltip_format_ok_ : tooltip_format_; if (!tooltip_template.empty()) { - setTooltipMarkup(fmt::format( - fmt::runtime(tooltip_template), fmt::arg("nr_failed", nr_failed_), - fmt::arg("nr_failed_system", nr_failed_system_), - fmt::arg("nr_failed_user", nr_failed_user_), fmt::arg("system_state", system_state_), - fmt::arg("user_state", user_state_), fmt::arg("overall_state", overall_state_), - fmt::arg("failed_units_list", failed_list))); + try { + setTooltipMarkup(fmt::format( + fmt::runtime(tooltip_template), fmt::arg("nr_failed", nr_failed_), + fmt::arg("nr_failed_system", nr_failed_system_), + fmt::arg("nr_failed_user", nr_failed_user_), fmt::arg("system_state", system_state_), + fmt::arg("user_state", user_state_), fmt::arg("overall_state", overall_state_), + fmt::arg("failed_units_list", failed_list))); + } catch (const std::exception& e) { + static bool tooltipWarned = false; + if (!tooltipWarned) { + spdlog::warn("systemd-failed-units: invalid tooltip-format, skipping tooltip: {}", + e.what()); + tooltipWarned = true; + } + setTooltipMarkup(""); + } } else { setTooltipMarkup(""); }