Skip to content

Validate l10n ID is non-empty to prevent crash on empty string ID#146

Draft
imnasnainaec wants to merge 3 commits into
masterfrom
fix/104-empty-id
Draft

Validate l10n ID is non-empty to prevent crash on empty string ID#146
imnasnainaec wants to merge 3 commits into
masterfrom
fix/104-empty-id

Conversation

@imnasnainaec

@imnasnainaec imnasnainaec commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

Closes #104

Passing an empty string as the id argument to GetString/GetDynamicString produced a malformed or zero-length .xlf file that crashed the application on the next launch.

Added an ArgumentException guard at the GetDynamicString and GetDynamicStringOrEnglish entry points in LocalizationManagerInternal, and a defensive early-return in XliffTransUnitUpdater.Update mirroring the existing LangId check.

(Technically a breaking change.)


This change is Reviewable

Devin review: https://app.devin.ai/review/sillsdev/l10nsharp/pull/146

…pdater (#104)

An empty string ID propagated silently through the system, producing a
malformed .xlf file that crashed the application on next launch. Added
ArgumentException at the GetDynamicString/GetDynamicStringOrEnglish
entry points and a defensive early-return in XliffTransUnitUpdater.Update
mirroring the existing LangId guard.
@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

Test Results

    7 files  ±0  104 suites  ±0   24s ⏱️ -5s
168 tests +2  163 ✔️ +2    5 💤 ±0  0 ±0 
638 runs  +8  623 ✔️ +8  15 💤 ±0  0 ±0 

Results for commit 6be1a22. ± Comparison against base commit d72d90c.

♻️ This comment has been updated with latest results.

@imnasnainaec imnasnainaec marked this pull request as ready for review June 5, 2026 15:14
@imnasnainaec imnasnainaec requested a review from andrew-polk June 5, 2026 15:14
@imnasnainaec imnasnainaec self-assigned this Jun 5, 2026
@imnasnainaec imnasnainaec removed the request for review from andrew-polk June 5, 2026 15:15
@imnasnainaec imnasnainaec marked this pull request as draft June 5, 2026 15:15
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.

Empty string for l10n ID causes crash

1 participant