From ea1906004bb16bbc10dae93b35ebbb80ea50a328 Mon Sep 17 00:00:00 2001 From: omar Date: Wed, 22 Aug 2018 00:06:55 +0200 Subject: [PATCH] InputText: Fixed a few pathological crash cases on single-line InputText widget with multiple millions characters worth of contents. Because the current text drawing function reserve for a worst-case amount of vertices and how we handle horizontal clipping, we currently just avoid displaying those single-line widgets when they are over a threshold of 2 millions characters, until a better solution is found. --- CHANGELOG.txt | 6 +++++- imgui.cpp | 14 ++++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 35393088..4bd66ad6 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -57,6 +57,10 @@ Other Changes: - InputText: Added support for buffer size/capacity changes via the ImGuiInputTextFlags_CallbackResize flag. (#2006, #1443, #1008). - InputText: Fixed not tracking the cursor horizontally When modifying the text buffer through a callback. - InputText: Fixed minor off-by-one issue when submitting a buffer size smaller than the initial zero-terminated buffer contents. + - InputText: Fixed a few pathological crash cases on single-line InputText widget with multiple millions characters worth of contents. + Because the current text drawing function reserve for a worst-case amount of vertices and how we handle horizontal clipping, + we currently just avoid displaying those single-line widgets when they are over a threshold of 2 millions characters, + until a better solution is found. - Drag and Drop: Fixed an incorrect assert when dropping a source that is submitted after the target (bug introduced with 1.62 changes related to the addition of IsItemDeactivated()). (#1875, #143) - Drag and Drop: Fixed ImGuiDragDropFlags_SourceNoDisableHover to affect hovering state prior to calling IsItemHovered() + fixed description. (#143) @@ -74,7 +78,7 @@ Other Changes: - Fixed PushID() from keeping alive the new ID Stack top value (if a previously active widget shared the ID it would be erroneously kept alive). - Fixed horizontal mouse wheel not forwarding the request to the parent window if ImGuiWindowFlags_NoScrollWithMouse is set. (#1463, #1380, #1502) - Fixed a include build issue for Cygwin in non-POSIX (Win32) mode. (#1917, #1319, #276) - - ImDrawList: Improved handling for worst-case vertices reservation policy when large amount of text (e.g. ~1 million character strings) + - ImDrawList: Improved handling for worst-case vertices reservation policy when large amount of text (e.g. 1+ million character strings) are being submitted in a single call. It would typically have crashed InputTextMultiline(). (#200) - OS/Windows: Fixed missing ImmReleaseContext() call in the default Win32 IME handler. (#1932) [@vby] - Metrics: Changed io.MetricsActiveWindows to reflect the number of active windows (!= from visible windows), which is useful diff --git a/imgui.cpp b/imgui.cpp index 654a9349..481fa951 100644 --- a/imgui.cpp +++ b/imgui.cpp @@ -11170,6 +11170,11 @@ bool ImGui::InputTextEx(const char* label, char* buf, int buf_size, const ImVec2 // Select which buffer we are going to display. When ImGuiInputTextFlags_NoLiveEdit is set 'buf' might still be the old value. We set buf to NULL to prevent accidental usage from now on. const char* buf_display = (g.ActiveId == id && is_editable) ? edit_state.TempBuffer.Data : buf; buf = NULL; + // Set upper limit of single-line InputTextEx() at 2 million characters strings. The current pathological worst case is a long line + // without any carriage return, which would makes ImFont::RenderText() reserve too many vertices and probably crash. Avoid it altogether. + // Note that we only use this limit on single-line InputText(), so a pathologically large line on a InputTextMultiline() would still crash. + const int buf_display_max_length = 2 * 1024 * 1024; + if (!is_multiline) { RenderNavHighlight(frame_bb, id); @@ -11303,7 +11308,9 @@ bool ImGui::InputTextEx(const char* label, char* buf, int buf_size, const ImVec2 } } - draw_window->DrawList->AddText(g.Font, g.FontSize, render_pos - render_scroll, GetColorU32(ImGuiCol_Text), buf_display, buf_display + edit_state.CurLenA, 0.0f, is_multiline ? NULL : &clip_rect); + const int buf_display_len = edit_state.CurLenA; + if (is_multiline || buf_display_len < buf_display_max_length) + draw_window->DrawList->AddText(g.Font, g.FontSize, render_pos - render_scroll, GetColorU32(ImGuiCol_Text), buf_display, buf_display + buf_display_len, 0.0f, is_multiline ? NULL : &clip_rect); // Draw blinking cursor bool cursor_is_visible = (!g.IO.ConfigCursorBlink) || (g.InputTextState.CursorAnim <= 0.0f) || ImFmod(g.InputTextState.CursorAnim, 1.20f) <= 0.80f; @@ -11322,7 +11329,10 @@ bool ImGui::InputTextEx(const char* label, char* buf, int buf_size, const ImVec2 const char* buf_end = NULL; if (is_multiline) text_size = ImVec2(size.x, InputTextCalcTextLenAndLineCount(buf_display, &buf_end) * g.FontSize); // We don't need width - draw_window->DrawList->AddText(g.Font, g.FontSize, render_pos, GetColorU32(ImGuiCol_Text), buf_display, buf_end, 0.0f, is_multiline ? NULL : &clip_rect); + else + buf_end = buf_display + strlen(buf_display); + if (is_multiline || (buf_end - buf_display) < buf_display_max_length) + draw_window->DrawList->AddText(g.Font, g.FontSize, render_pos, GetColorU32(ImGuiCol_Text), buf_display, buf_end, 0.0f, is_multiline ? NULL : &clip_rect); } if (is_multiline)