fix(contacts): adversarial-review fixes — delete UB, JPEG decode, footer clip

From an adversarial review of the avatar edit-dialog + image picker:

- HIGH: the per-row Delete icon called doDelete() (which erases from
  book.entries()) INSIDE the loop iterating that same vector — out-of-bounds
  reads / wrong rows on the confirming click. Defer it until after the loop.
- HIGH: the decode stack was compiled PNG-only (STBI_ONLY_PNG) while the image
  picker accepts .jpg/.jpeg/.bmp/.gif, so picking a JPEG (the common photo
  case) silently produced a non-loading avatar + an orphaned copy on disk.
  Enable JPEG/BMP/GIF decoders, and guard the pick: verify the source decodes
  before copying/committing (new contact_avatar_bad_image string, 8 langs).
- MED: the fixed, non-scrolling edit card floored bodyH at 260*dp, which could
  push Save/Cancel below the card on short windows — floor lowered so the
  footer always stays inside.
- LOW: the live-preview panel rounding is now dp-scaled (10*dp) to match the
  real list card it mirrors.
- LOW: re-picking the same source path after its contents changed showed a
  stale cached texture — evict the avatar texture cache entry on re-pick.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-07-15 09:49:34 -05:00
parent e1df5ea798
commit 9649654c3c
11 changed files with 43 additions and 5 deletions

View File

@@ -288,6 +288,7 @@
"console_zoom_in": "Vergrößern",
"console_zoom_out": "Verkleinern",
"contact_avatar": "AVATAR",
"contact_avatar_bad_image": "Dieses Bild konnte nicht geladen werden.",
"contact_avatar_badge": "Abzeichen",
"contact_avatar_badge_hint": "Das Abzeichen wird automatisch anhand des Adresstyps gewählt.",
"contact_avatar_choose": "Bild auswählen…",

View File

@@ -288,6 +288,7 @@
"console_zoom_in": "Acercar",
"console_zoom_out": "Alejar",
"contact_avatar": "AVATAR",
"contact_avatar_bad_image": "No se pudo cargar esa imagen.",
"contact_avatar_badge": "Insignia",
"contact_avatar_badge_hint": "La insignia se elige automáticamente según el tipo de dirección.",
"contact_avatar_choose": "Elegir imagen…",

View File

@@ -288,6 +288,7 @@
"console_zoom_in": "Agrandir",
"console_zoom_out": "Réduire",
"contact_avatar": "AVATAR",
"contact_avatar_bad_image": "Cette image n'a pas pu être chargée.",
"contact_avatar_badge": "Badge",
"contact_avatar_badge_hint": "Le badge est choisi automatiquement selon le type d'adresse.",
"contact_avatar_choose": "Choisir une image…",

View File

@@ -288,6 +288,7 @@
"console_zoom_in": "拡大",
"console_zoom_out": "縮小",
"contact_avatar": "アバター",
"contact_avatar_bad_image": "その画像を読み込めませんでした。",
"contact_avatar_badge": "バッジ",
"contact_avatar_badge_hint": "バッジはアドレスの種類に応じて自動的に選ばれます。",
"contact_avatar_choose": "画像を選択…",

View File

@@ -288,6 +288,7 @@
"console_zoom_in": "확대",
"console_zoom_out": "축소",
"contact_avatar": "아바타",
"contact_avatar_bad_image": "그 이미지를 불러올 수 없습니다.",
"contact_avatar_badge": "배지",
"contact_avatar_badge_hint": "배지는 주소 유형에 따라 자동으로 선택됩니다.",
"contact_avatar_choose": "이미지 선택…",

View File

@@ -288,6 +288,7 @@
"console_zoom_in": "Aumentar zoom",
"console_zoom_out": "Diminuir zoom",
"contact_avatar": "AVATAR",
"contact_avatar_bad_image": "Não foi possível carregar essa imagem.",
"contact_avatar_badge": "Selo",
"contact_avatar_badge_hint": "O selo é escolhido automaticamente conforme o tipo de endereço.",
"contact_avatar_choose": "Escolher imagem…",

View File

@@ -288,6 +288,7 @@
"console_zoom_in": "Увеличить",
"console_zoom_out": "Уменьшить",
"contact_avatar": "АВАТАР",
"contact_avatar_bad_image": "Не удалось загрузить это изображение.",
"contact_avatar_badge": "Значок",
"contact_avatar_badge_hint": "Значок выбирается автоматически по типу адреса.",
"contact_avatar_choose": "Выбрать изображение…",

View File

@@ -288,6 +288,7 @@
"console_zoom_in": "放大",
"console_zoom_out": "缩小",
"contact_avatar": "头像",
"contact_avatar_bad_image": "无法加载该图片。",
"contact_avatar_badge": "标记",
"contact_avatar_badge_hint": "标记会根据地址类型自动选择。",
"contact_avatar_choose": "选择图片…",

View File

@@ -78,6 +78,14 @@ static const AvatarTex* getAvatarTexture(const std::string& path) {
auto& slot = (s_avatarTexCache[path] = at);
return slot.tex ? &slot : nullptr;
}
// Drop a cached avatar texture so the next getAvatarTexture reloads from disk — needed when the file
// at that path is overwritten (re-picking the same source path whose contents changed).
static void invalidateAvatarTexture(const std::string& path) {
auto it = s_avatarTexCache.find(path);
if (it == s_avatarTexCache.end()) return;
if (it->second.tex) util::DestroyTexture(it->second.tex);
s_avatarTexCache.erase(it);
}
// Draw a contact's avatar into the circle at `c` (radius `r`): a custom image (circular-cropped), a
// Material icon in a tinted circle, or — the default — the Z/T type badge.
@@ -281,7 +289,7 @@ void RenderContactsTab(App* app)
const float pw = ImGui::GetContentRegionAvail().x;
ImVec2 pMin = ImGui::GetCursorScreenPos();
ImVec2 pMax(pMin.x + pw, pMin.y + ph);
material::GlassPanelSpec g; g.rounding = 10.0f; g.fillAlpha = 22; g.borderAlpha = 40;
material::GlassPanelSpec g; g.rounding = 10.0f * dp; g.fillAlpha = 22; g.borderAlpha = 40;
material::DrawGlassPanel(pdl, pMin, pMax, g);
data::AddressBookEntry pv;
@@ -312,7 +320,9 @@ void RenderContactsTab(App* app)
// Body flexes to fill the tall card, leaving just the footer button row — so the icon grid
// and notes field grow into the vertical space instead of leaving it empty.
const float footerReserve = 44.0f * dp + Layout::spacingMd();
const float bodyH = std::max(260.0f * dp, ImGui::GetContentRegionAvail().y - footerReserve);
// Never floor above what's available — the footer (Save/Cancel) must stay inside the fixed,
// non-scrolling card even on short windows. A tiny floor only guards a degenerate size.
const float bodyH = std::max(48.0f * dp, ImGui::GetContentRegionAvail().y - footerReserve);
const float colGap = Layout::spacingLg();
const float bodyAvail = ImGui::GetContentRegionAvail().x;
const float colW = (bodyAvail - colGap) * 0.5f;
@@ -568,9 +578,17 @@ void RenderContactsTab(App* app)
ImGui::SetCursorPosX(ImGui::GetCursorPosX() + std::max(0.0f, (a.x - rowW) * 0.5f));
if (material::TactileButton(TR("contact_avatar_choose"), ImVec2(chooseW, 0), btnFont)) {
ImagePicker::open("", [](const std::string& src) {
// Verify the source actually decodes before committing (guards a corrupt or
// unsupported file from becoming a broken avatar + an orphaned copy on disk).
int iw = 0, ih = 0;
unsigned char* px = util::LoadRawPixelsFromFile(src.c_str(), &iw, &ih);
if (!px) { Notifications::instance().error(TR("contact_avatar_bad_image")); return; }
util::FreeRawPixels(px);
std::string dst = copyAvatarImage(src);
if (!dst.empty()) { s_edit_avatar = "img:" + dst; s_edit_avatar_mode = 2; }
else Notifications::instance().error(TR("contact_avatar_copy_failed"));
if (!dst.empty()) {
invalidateAvatarTexture(dst); // in case this path was cached before an overwrite
s_edit_avatar = "img:" + dst; s_edit_avatar_mode = 2;
} else Notifications::instance().error(TR("contact_avatar_copy_failed"));
});
}
if (hasImg) {
@@ -866,6 +884,7 @@ void RenderContactsTab(App* app)
const float avR = 15.0f * dp;
const float rowH = avR * 2.0f + (asCard ? 16.0f * dp : 12.0f * dp);
const float round = 10.0f * dp;
bool rowDeleteRequested = false; // per-row delete is deferred until after the loop
for (size_t vi = 0; vi < visibleRows.size(); ++vi) {
size_t i = visibleRows[vi];
const auto& entry = book.entries()[i];
@@ -950,7 +969,9 @@ void RenderContactsTab(App* app)
s_selected_index = static_cast<int>(i);
if (a == 0) doCopy();
else if (a == 1) openEdit();
else doDelete();
// Delete mutates book.entries() (removeEntry -> erase) — deferring it past the
// row loop avoids iterating a vector we shrink mid-loop (OOB on later rows).
else rowDeleteRequested = true;
}
const bool armedDel = (a == 2 && s_confirm_delete_idx == static_cast<int>(i));
const ImU32 acol = armedDel ? material::ReadableError()
@@ -967,6 +988,9 @@ void RenderContactsTab(App* app)
ImGui::SetCursorScreenPos(afterRow); // undo the action buttons' cursor moves
ImGui::PopID();
}
// Run the deferred per-row delete now that the loop over book.entries() has finished
// (doDelete's two-stage confirm arms on the first click and removes on the second).
if (rowDeleteRequested) doDelete();
// The per-row SetCursorScreenPos leaves the cursor at the last row's bottom with no item
// submitted there; commit it so ImGui sizes the scroll child (avoids the extend-boundary warning).
ImGui::Dummy(ImVec2(0.0f, 0.0f));

View File

@@ -1243,6 +1243,7 @@ void I18n::loadBuiltinEnglish()
strings_["contact_avatar_remove"] = "Remove";
strings_["contact_avatar_image_hint"] = "The image is copied into the app so it stays available if the original moves.";
strings_["contact_avatar_copy_failed"] = "Could not copy that image.";
strings_["contact_avatar_bad_image"] = "That image couldn't be loaded.";
strings_["contact_global_badge_tt"] = "Global contact — visible in every wallet";
strings_["address_book_added"] = "Address added to book";
strings_["address_book_count"] = "%zu addresses saved";

View File

@@ -7,7 +7,12 @@
// stb_image — single-file image loader (public domain)
// Only compiled here; all other files just include the header.
#define STB_IMAGE_IMPLEMENTATION
// Formats we actually decode: PNG for app assets, plus JPEG/BMP/GIF so user-chosen contact avatars
// (which are commonly JPEGs) load — must stay in sync with ImagePicker's accepted extensions.
#define STBI_ONLY_PNG
#define STBI_ONLY_JPEG
#define STBI_ONLY_BMP
#define STBI_ONLY_GIF
#define STBI_NO_STDIO // we do our own fread for portability
#include "stb_image.h"