DragonX/ObsidianDragon
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 4 Wiki Activity

fix(console): audit fixes — HiDPI, fold UX, stop-confirm, RPC-trace UAF

Browse Source 
From the console-tab audit:
- HiDPI: the "New output" pill (140x24 + margin/rounding/border) and the JSON
  indent-guide thickness were hand-drawn in raw px; scale them by dpiScale().
- Fold triangle: size it from the DPI/density-scaled gutter width (not the
  zoomed font) and center it in the gutter band so glyph and clickable cell
  stay aligned; only draw/handle it in the unfiltered view (folding_active_) —
  in a filtered flat view a click silently flipped the collapsed flag with no
  visible effect and the glyph disagreed with the rendered block.
- Selection: ignore left-clicks in the gutter (< output_origin_.x) so toggling
  a fold no longer clears the user's active text selection.
- 'stop' confirmation: promote the function-local static to a member and reset
  it in clear(), so a toolbar/context-menu clear between the two 'stop's can't
  leave a stale arm that skips the shutdown warning.
- RPC-trace callback: dereference the console pointer under the mutex (not
  after releasing it) so ~ConsoleTab can't destroy the object mid-call —
  closing a latent use-after-free on shutdown (fires on RPC worker threads).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
DanS
2026-07-12 17:45:51 -05:00
parent 232b81f8ee
commit 4f71c18884
2 changed files with 39 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

61
src/ui/windows/console_tab.cpp
View File

@@ -241,12 +241,11 @@ ConsoleTab::ConsoleTab()
s_rpc_trace_console = this;
}
rpc::RPCClient::setTraceCallback([](const std::string& source, const std::string& method) {
ConsoleTab* console = nullptr;
{
std::lock_guard<std::mutex> lock(s_rpc_trace_console_mutex);
console = s_rpc_trace_console;
}
if (console) console->addRpcTraceLine(source, method);
// Dereference under the lock, not after releasing it: ~ConsoleTab clears
// s_rpc_trace_console under the same lock, so holding it here blocks destruction until the
// call returns — closing the shutdown use-after-free window (this fires on RPC worker threads).
std::lock_guard<std::mutex> lock(s_rpc_trace_console_mutex);
if (s_rpc_trace_console) s_rpc_trace_console->addRpcTraceLine(source, method);
});
rpc::RPCClient::setTraceEnabled(s_rpc_trace_enabled);
@@ -848,6 +847,10 @@ void ConsoleTab::computeVisibleLines(bool& hasTextFilter, std::string& filterLow
bool has_filter = hasTextFilter || !outputFilter.daemonMessagesEnabled ||
!outputFilter.rpcTraceEnabled || !outputFilter.appMessagesEnabled ||
outputFilter.errorsOnly;
// Folding is only honored in the unfiltered view; the filtered view is flat. Record it so the
// fold triangles aren't drawn/clickable while filtering (else clicks would silently mutate the
// collapsed flag with no visible effect and the glyph would disagree with the rendered block).
folding_active_ = !has_filter;
visible_indices_.clear();
const int n = static_cast<int>(model_.size());
if (has_filter) {
@@ -883,8 +886,10 @@ void ConsoleTab::handleOutputInteraction(ImVec2 mousePos, bool mouseInOutput)
if (mouseInOutput) {
ImGui::SetMouseCursor(ImGuiMouseCursor_TextInput);
}
// Selection drag lifecycle (continues even if the mouse leaves the window).
if (mouseInOutput && io.MouseClicked[0]) {
// Selection drag lifecycle (continues even if the mouse leaves the window). Ignore clicks in the
// left gutter (< output_origin_.x) — that strip holds the accent bar + fold triangles, so a fold
// toggle there shouldn't begin (and thus clear) a text selection.
if (mouseInOutput && io.MouseClicked[0] && mousePos.x >= output_origin_.x) {
selection_.beginDrag(screenToTextPos(mousePos));
}
if (selection_.dragging() && io.MouseDown[0]) {
@@ -995,16 +1000,19 @@ void ConsoleTab::drawVisibleLines(float padX, float lineHeight, bool hasTextFilt
for (size_t c = 2; c < leading; c += 2) {
float gx = lineOrigin.x + static_cast<float>(c) * spaceW;
dl->AddLine(ImVec2(gx, lineOrigin.y), ImVec2(gx, lineOrigin.y + totalH),
IM_COL32(255, 255, 255, 20), 1.0f);
IM_COL32(255, 255, 255, 20), 1.0f * Layout::dpiScale());
}
}
}
// JSON fold toggle — a small triangle in the left gutter for block-opener lines.
// Openers carry no accent bar (result/JSON channels), so the gutter is free.
if (line.foldSpan > 0) {
float sz = fontSize * 0.30f;
float cx = output_origin_.x - padX + sz + 1.0f * Layout::hScale();
// Openers carry no accent bar (result/JSON channels), so the gutter is free. Only shown in the
// unfiltered view, where folding actually applies (see folding_active_ in computeVisibleLines).
if (folding_active_ && line.foldSpan > 0) {
// Size from the (DPI/density-scaled) gutter width, not the zoomed font, and center it in
// the gutter band [origin-padX, origin) so the glyph and its clickable cell stay aligned.
float sz = std::min(fontSize * 0.30f, padX * 0.34f);
float cx = output_origin_.x - padX * 0.5f;
float cy = lineOrigin.y + lineHeight * 0.5f;
ImU32 triCol = WithAlpha(OnSurfaceMedium(), 210);
if (line.collapsed) {
@@ -1174,18 +1182,21 @@ void ConsoleTab::drawNewOutputIndicator()
// "New output" indicator when the user is scrolled up and new lines arrived.
if (scroll_.autoScroll() || scroll_.newLines() <= 0) return;
float indicW = 140.0f;
float indicH = 24.0f;
// The box geometry is hand-drawn absolute px, so scale it by dpiScale() (the font metrics below
// are already scaled — left alone). Without this the pill renders native-size on a HiDPI display.
float dp = Layout::dpiScale();
float indicW = 140.0f * dp;
float indicH = 24.0f * dp;
ImDrawList* dlInd = ImGui::GetWindowDrawList();
ImVec2 wMin = ImGui::GetWindowPos();
ImVec2 wSize = ImGui::GetWindowSize();
float ix = wMin.x + (wSize.x - indicW) * 0.5f;
float iy = wMin.y + wSize.y - indicH - 8.0f;
float iy = wMin.y + wSize.y - indicH - 8.0f * dp;
ImVec2 iMin(ix, iy);
ImVec2 iMax(ix + indicW, iy + indicH);
dlInd->AddRectFilled(iMin, iMax, IM_COL32(40, 40, 40, 220), 12.0f);
dlInd->AddRect(iMin, iMax, IM_COL32(255, 218, 0, 120), 12.0f);
dlInd->AddRectFilled(iMin, iMax, IM_COL32(40, 40, 40, 220), 12.0f * dp);
dlInd->AddRect(iMin, iMax, IM_COL32(255, 218, 0, 120), 12.0f * dp, 0, 1.0f * dp);
char buf[48];
snprintf(buf, sizeof(buf), TR("console_new_lines"), scroll_.newLines());
@@ -1391,9 +1402,10 @@ bool ConsoleTab::submitConsoleCommand(ConsoleCommandExecutor& exec, const std::s
std::transform(first.begin(), first.end(), first.begin(),
[](unsigned char c) { return static_cast<char>(std::tolower(c)); });
}
// 'stop' shuts down the node — require a confirming second 'stop'; any other command clears the pending state.
static bool stopConfirmPending = false;
if (first != "stop") stopConfirmPending = false;
// 'stop' shuts down the node — require a confirming second 'stop'; any other command clears the
// pending state. Member (not a function-local static) so clear() can also cancel it — otherwise a
// toolbar/context-menu clear between the two 'stop's would leave a stale arm and skip the warning.
if (first != "stop") stop_confirm_pending_ = false;
auto add = [this](const std::string& l, ConsoleChannel c) { addLine(l, c); };
if (first == "clear" || first == "cls") {
// View-only clear — NEVER forwarded (the lite backend's `clear` wipes tx history).
@@ -1404,12 +1416,12 @@ bool ConsoleTab::submitConsoleCommand(ConsoleCommandExecutor& exec, const std::s
} else if (first == "quit" || first == "exit") {
addLine(TR("console_quit_note"), ConsoleChannel::Info);
} else if (first == "stop") {
if (!stopConfirmPending) {
stopConfirmPending = true;
if (!stop_confirm_pending_) {
stop_confirm_pending_ = true;
addLine("'stop' will shut down the node and disconnect the wallet. Type 'stop' again to confirm.",
ConsoleChannel::Warning);
} else {
stopConfirmPending = false;
stop_confirm_pending_ = false;
if (!exec.isReady()) addLine(TR("console_not_connected"), ConsoleChannel::Error);
else exec.submit(cmd);
}
@@ -1688,6 +1700,7 @@ void ConsoleTab::clear()
// View-only clear (main thread). The executor keeps its own log cursors, so new output
// still appends. The "cleared" line is ingested and appears on the next frame's drain.
model_.clear();
stop_confirm_pending_ = false; // a pending 'stop' confirmation is cancelled by clearing
addLine(TR("console_cleared"), ConsoleChannel::Info);
}

2
src/ui/windows/console_tab.h
View File

@@ -150,6 +150,7 @@ private:
std::vector<std::string> command_history_;
int history_index_ = -1;
char input_buffer_[4096] = {0};
bool stop_confirm_pending_ = false; // 'stop' typed once, awaiting a confirming second 'stop'
// (log-ingestion cursors + result queue moved to the ConsoleCommandExecutor)
// Auto-scroll state machine (pin-to-bottom, wheel-up cooldown, new-line backlog count).
@@ -173,6 +174,7 @@ private:
mutable int filter_match_count_ = 0; // lines matching the text filter (for the toolbar)
std::string context_token_; // hash/address under the cursor at right-click
mutable std::vector<int> visible_indices_; // Cached for selection mapping
bool folding_active_ = true; // fold UI shown only in the unfiltered (foldable) view
// Wrap layout for the visible lines (segments + per-line heights + cumulative Y),
// recomputed each frame by the pure BuildConsoleLayout (console_text_layout.h) and