Skip to content

Commit 67b9049

Browse files
mountinycursoragent
andcommitted
Address Copilot review feedback on currency sanitization
- Normalize whitespace + case before validating in sanitizeCurrencyCode so common backend malformations like 'eur' and ' USD ' display the intended currency instead of incorrectly falling back to USD. - Throttle the invalid-currency Log.warn to fire at most once per unique malformed value using a module-level Set, avoiding log spam on hot paths like getLocalizedCurrencySymbol. - Loosen sanitizeCurrencyCode/isValidCurrencyCode parameter typing to unknown so non-string inputs from JS callers are handled safely without unsafe-type lint errors. - Use 'currency' in options (instead of truthiness) in the NumberFormatUtils try/catch fallback so Intl.NumberFormat throws triggered by an empty-string currency are still recovered to USD. - Add unit tests for isValidCurrencyCode, sanitizeCurrencyCode, and the malformed-currency behavior of convertToDisplayString and getLocalizedCurrencySymbol. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 8ca1527 commit 67b9049

3 files changed

Lines changed: 70 additions & 6 deletions

File tree

src/libs/CurrencyUtils.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,20 +24,31 @@ Onyx.connect({
2424
* Returns true when the provided value is a syntactically valid ISO 4217 currency code
2525
* (exactly 3 uppercase ASCII letters).
2626
*/
27-
function isValidCurrencyCode(currencyCode: string | undefined | null): currencyCode is string {
27+
function isValidCurrencyCode(currencyCode: unknown): currencyCode is string {
2828
return typeof currencyCode === 'string' && /^[A-Z]{3}$/.test(currencyCode);
2929
}
3030

31+
// Tracks invalid currency codes already warned about so the same bad value doesn't spam the log on every render.
32+
const warnedInvalidCurrencyCodes = new Set<string>();
33+
3134
/**
3235
* Validates a currency code and returns it unchanged if it is a valid ISO 4217 code.
33-
* Returns CONST.CURRENCY.USD and logs a warning when the code is malformed or missing, to prevent Intl.NumberFormat
36+
* Whitespace and case-only variations (e.g. " usd ") are normalized rather than discarded.
37+
* Returns CONST.CURRENCY.USD and logs a warning at most once per unique malformed value, to prevent Intl.NumberFormat
3438
* from throwing a RangeError. See https://github.com/Expensify/App/issues/91113
3539
*/
36-
function sanitizeCurrencyCode(currencyCode: string): string {
40+
function sanitizeCurrencyCode(currencyCode: unknown): string {
3741
if (isValidCurrencyCode(currencyCode)) {
3842
return currencyCode;
3943
}
40-
Log.warn('CurrencyUtils: invalid currency code, defaulting to USD', {currencyCode});
44+
const normalized = typeof currencyCode === 'string' ? currencyCode.trim().toUpperCase() : '';
45+
if (isValidCurrencyCode(normalized)) {
46+
return normalized;
47+
}
48+
if (!warnedInvalidCurrencyCodes.has(normalized)) {
49+
warnedInvalidCurrencyCodes.add(normalized);
50+
Log.warn('CurrencyUtils: invalid currency code, defaulting to USD', {currencyCode});
51+
}
4152
return CONST.CURRENCY.USD;
4253
}
4354

src/libs/NumberFormatUtils/index.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@ function format(locale: Locale | undefined, number: number, options?: Intl.Numbe
1313
try {
1414
return new MemoizedNumberFormat(locale ?? CONST.LOCALES.DEFAULT, options).format(number);
1515
} catch (e) {
16-
if (e instanceof RangeError && options?.currency) {
16+
// Intl throws RangeError for empty/malformed currency values; check presence of the option (not truthiness)
17+
// so we still recover when currency is '' rather than rethrowing and crashing the screen.
18+
if (e instanceof RangeError && options && 'currency' in options) {
1719
Log.warn('NumberFormatUtils: malformed currency code, falling back to USD', {currency: options.currency});
1820
return new MemoizedNumberFormat(locale ?? CONST.LOCALES.DEFAULT, {...options, currency: CONST.CURRENCY.USD}).format(number);
1921
}
@@ -25,7 +27,7 @@ function formatToParts(locale: Locale | undefined, number: number, options?: Int
2527
try {
2628
return new MemoizedNumberFormat(locale ?? CONST.LOCALES.DEFAULT, options).formatToParts(number);
2729
} catch (e) {
28-
if (e instanceof RangeError && options?.currency) {
30+
if (e instanceof RangeError && options && 'currency' in options) {
2931
Log.warn('NumberFormatUtils: malformed currency code, falling back to USD', {currency: options.currency});
3032
return new MemoizedNumberFormat(locale ?? CONST.LOCALES.DEFAULT, {...options, currency: CONST.CURRENCY.USD}).formatToParts(number);
3133
}

tests/unit/CurrencyUtilsTest.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,4 +189,55 @@ describe('CurrencyUtils', () => {
189189
IntlStore.load(CONST.LOCALES.ES).then(() => expect(CurrencyUtils.convertToShortDisplayString(amount, currency)).toBe(expectedResult)),
190190
);
191191
});
192+
193+
describe('isValidCurrencyCode', () => {
194+
test.each([
195+
['USD', true],
196+
['EUR', true],
197+
['JPY', true],
198+
['', false],
199+
['eur', false],
200+
[' USD', false],
201+
['US', false],
202+
['USDX', false],
203+
['US1', false],
204+
[undefined, false],
205+
[null, false],
206+
])('isValidCurrencyCode(%p) → %p', (input, expected) => {
207+
expect(CurrencyUtils.isValidCurrencyCode(input)).toBe(expected);
208+
});
209+
});
210+
211+
describe('sanitizeCurrencyCode', () => {
212+
test('returns the input unchanged for a valid ISO 4217 code', () => {
213+
expect(CurrencyUtils.sanitizeCurrencyCode('EUR')).toBe('EUR');
214+
});
215+
216+
test('normalizes whitespace and case before validating', () => {
217+
expect(CurrencyUtils.sanitizeCurrencyCode(' usd ')).toBe('USD');
218+
expect(CurrencyUtils.sanitizeCurrencyCode('eur')).toBe('EUR');
219+
});
220+
221+
test.each(['', 'XX', 'USDX', 'US1', '???'])('falls back to USD for malformed input %p', (input) => {
222+
expect(CurrencyUtils.sanitizeCurrencyCode(input)).toBe(CONST.CURRENCY.USD);
223+
});
224+
});
225+
226+
describe('convertToDisplayString with malformed currency', () => {
227+
test.each(['', 'XX', 'USDX', '???'])('does not throw and falls back to USD formatting for %p', (input) => {
228+
expect(() => CurrencyUtils.convertToDisplayString(2500, input)).not.toThrow();
229+
expect(CurrencyUtils.convertToDisplayString(2500, input)).toBe('$25.00');
230+
});
231+
232+
test('normalizes case-only variations to the intended currency instead of USD', () => {
233+
expect(CurrencyUtils.convertToDisplayString(2500, 'eur')).toBe(CurrencyUtils.convertToDisplayString(2500, 'EUR'));
234+
});
235+
});
236+
237+
describe('getLocalizedCurrencySymbol with malformed currency', () => {
238+
test.each(['', 'XX', 'USDX'])('returns the USD symbol without throwing for %p', (input) => {
239+
expect(() => CurrencyUtils.getLocalizedCurrencySymbol(CONST.LOCALES.EN, input)).not.toThrow();
240+
expect(CurrencyUtils.getLocalizedCurrencySymbol(CONST.LOCALES.EN, input)).toBe(CurrencyUtils.getLocalizedCurrencySymbol(CONST.LOCALES.EN, CONST.CURRENCY.USD));
241+
});
242+
});
192243
});

0 commit comments

Comments
 (0)