Skip to content

fix(notes): handle nullish and non-iterable placeholders in isRegenerableNoteTitle - #1816

Closed
hsusul wants to merge 1 commit into
OpenWhispr:mainfrom
hsusul:fix/1815-regenerable-note-title-null-safety
Closed

fix(notes): handle nullish and non-iterable placeholders in isRegenerableNoteTitle#1816
hsusul wants to merge 1 commit into
OpenWhispr:mainfrom
hsusul:fix/1815-regenerable-note-title-null-safety

Conversation

@hsusul

@hsusul hsusul commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Fixes #1815

Problem

In src/helpers/regenerableNoteTitle.js:
isRegenerableNoteTitle defaulted placeholders = [], which only triggers when placeholders === undefined. If passed null or a non-iterable value, iterating with for (const p of placeholders) threw TypeError: placeholders is not iterable.

Solution

  • Fall back to [] if placeholders is null or not iterable before iterating.
  • Added unit tests in test/helpers/regenerableNoteTitle.test.js.

Verification

  • node --test test/helpers/regenerableNoteTitle.test.js (passes, 6/6 tests)
  • npm run typecheck (passes, 0 errors)
  • npm run lint (passes, 0 errors)
  • npm run i18n:check (passes)
  • npm run build:renderer (passes)
  • git diff --check (clean)

@Chadpiha Chadpiha self-assigned this Aug 25, 2026
@Chadpiha

Copy link
Copy Markdown
Collaborator

Thanks @hsusul! Closing together with #1815.

This helper has a single caller (PersonalNotesView.tsx), which always passes a literal inline array of three t(...) results — and react-i18next's t() always returns a string — so placeholders is a string array by construction on every call. The one parameter that genuinely crosses a trust boundary here is title (it comes from the DB and can be null), and the helper already guards that with typeof title === "string". Our convention is to draw the line exactly there: validate what crosses a boundary, trust what internal code guarantees.

The tests themselves were well done and I ran them green — this is purely a scope-of-validation call, not a quality one. Thanks for the contribution, and keep 'em coming!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

isRegenerableNoteTitle throws TypeError when placeholders is null or non-iterable

2 participants