Skip to content

Commit d4148d1

Browse files
committed
fix(tables): vet a currency marker against an allowlist, not a scale denylist
Bugbot found that `SCALE_SUFFIX` misses `mio`, `mrd`, `bio`, `tsd`, `mln` and `md`. Each is one to three letters, so each matched the currency-marker pattern and was stripped: `1,2 mio` read as 1.2, a millionfold too small, which is the same failure `1.2 M` had. Adding the six words would leave the next six. The denylist is the defect — it has to enumerate every magnitude abbreviation in every language, and each one it misses silently rescales a value rather than refusing it. So the check is inverted. A bare letter token is stripped only when it names a currency: an ISO 4217 code the runtime can enumerate, or one of the common non-ISO markers people type (`kr`, `zł`, `Kč`). Anything else is left in place, fails the amount-shape check, and is refused. `12 units` and `12 pcs` now fall out of the same rule the magnitude words do, without naming either. A marker carrying a currency symbol skips the check — no magnitude abbreviation contains one — so `R$`, `CHF`, `1 234,56 kr` are untouched. This rests on ISO codes and magnitude words being disjoint, which is true for all 162 codes the runtime knows, and is now pinned by a test so a future collision fails loudly instead of silently. `SCALE_SUFFIX` survives only for a runtime without `Intl.supportedValuesOf`, which cannot enumerate codes and so keeps the old permissive behaviour; rejecting every letter marker there would break `USD 12.50`. It no longer has to be complete, since every modern runtime rejects by non-recognition. Verified the new tests fail when the vetting is reverted.
1 parent c64d42c commit d4148d1

2 files changed

Lines changed: 101 additions & 17 deletions

File tree

apps/sim/lib/table/__tests__/currency.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,35 @@ describe('parseCurrencyInput', () => {
203203
expect(parseCurrencyInput('1 234,56 kr')).toBe(1234.56)
204204
})
205205

206+
it('refuses non-English magnitude words too', () => {
207+
// These are why the marker check is an allowlist. As a denylist each one
208+
// had to be named, and any that was missed read as a bare number — `1,2
209+
// mio` as 1.2 rather than 1.2 million.
210+
expect(parseCurrencyInput('1,2 mio')).toBeNull()
211+
expect(parseCurrencyInput('3,4 mrd')).toBeNull()
212+
expect(parseCurrencyInput('5 tsd')).toBeNull()
213+
expect(parseCurrencyInput('2,5 mln')).toBeNull()
214+
expect(parseCurrencyInput('1,5 bio')).toBeNull()
215+
expect(parseCurrencyInput('7 md')).toBeNull()
216+
// Anything else beside a number is refused by the same rule, so the parser
217+
// no longer has to enumerate what a magnitude word looks like.
218+
expect(parseCurrencyInput('12 units')).toBeNull()
219+
expect(parseCurrencyInput('12 pcs')).toBeNull()
220+
})
221+
222+
it('keeps every ISO code strippable — no code is a magnitude word', () => {
223+
// The allowlist is only safe because these two sets do not overlap. If a
224+
// future ISO code ever collided with a magnitude abbreviation, this fails
225+
// and the marker rule needs a tiebreak.
226+
const codes = new Set(
227+
(Intl as unknown as { supportedValuesOf: (k: string) => string[] }).supportedValuesOf(
228+
'currency'
229+
)
230+
)
231+
const magnitudeWords = ['K', 'M', 'B', 'T', 'BN', 'MN', 'MIO', 'MRD', 'TSD', 'MLN', 'BIO', 'MD']
232+
expect(magnitudeWords.filter((w) => codes.has(w))).toEqual([])
233+
})
234+
206235
it('rejects an identifier whose letters touch its digits', () => {
207236
// The distinguishing rule: a currency marker is always separated from the
208237
// number by a space or a symbol, so letters touching digits mean this is a

apps/sim/lib/table/currency.ts

Lines changed: 72 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -25,24 +25,80 @@ const BIDI_MARKS = /[\u200e\u200f\u061c\u202a-\u202e\u2066-\u2069]/g
2525
const UNICODE_MINUS = /[\u2212\u2012\u2013\uFE63\uFF0D]/g
2626

2727
/**
28-
* A magnitude suffix (`1.2 M`, `5 K`, `3.4 bn`). Not a currency marker, and
29-
* stripping it would silently shrink the value by orders of magnitude.
28+
* A magnitude suffix (`1.2 M`, `5 K`, `3.4 bn`), used only on runtimes that
29+
* cannot enumerate currency codes \u2014 see {@link isCurrencyMarkerLetters}. Every
30+
* other runtime rejects these by not recognising them, so this list does not
31+
* have to be complete.
3032
*/
31-
const SCALE_SUFFIX = /\d\s*(?:k|m|b|t|bn|mn|tn|mm|mil|bil)\.?\s*$/i
33+
const SCALE_SUFFIX = /^(?:k|m|b|t|bn|mn|tn|mm|mil|bil|mio|mrd|bio|tsd|mln|md)$/i
3234

3335
/** A letter directly adjacent to a digit: an identifier, not an amount. */
3436
const LETTER_TOUCHING_DIGIT = /\p{L}\d|\d\p{L}/u
3537

38+
/**
39+
* Currency markers people type that are not ISO 4217 codes. Upper-cased for
40+
* comparison; `\u0141` and `\u010c` upper-case as expected.
41+
*/
42+
const NON_ISO_CURRENCY_MARKERS = new Set([
43+
'KR',
44+
'Z\u0141',
45+
'K\u010c',
46+
'LEI',
47+
'LV',
48+
'FT',
49+
'KN',
50+
'RM',
51+
'RP',
52+
'TL',
53+
'DIN',
54+
'SR',
55+
])
56+
57+
/**
58+
* Whether a bare letter token sitting beside a number is a currency marker.
59+
*
60+
* An allowlist, deliberately. The alternative is a denylist of everything that
61+
* is *not* a currency, which has to guess every magnitude abbreviation in every
62+
* language \u2014 `mio`, `mrd`, `tsd`, `mln`, `bio` \u2014 and silently divides the value
63+
* by a million for each one it has not thought of. An unknown token now simply
64+
* fails to strip, so the leftover letters fail {@link AMOUNT_SHAPE} and the
65+
* input is rejected rather than quietly rescaled.
66+
*
67+
* No ISO code collides with a magnitude abbreviation, so nothing legitimate is
68+
* lost \u2014 `lib/table/__tests__/currency.test.ts` pins that.
69+
*
70+
* A runtime that cannot enumerate codes keeps the old permissive behaviour,
71+
* minus the magnitude words it knows by name; rejecting every letter marker
72+
* there would break `USD 12.50` outright.
73+
*/
74+
function isCurrencyMarkerLetters(letters: string): boolean {
75+
const upper = letters.toUpperCase()
76+
if (NON_ISO_CURRENCY_MARKERS.has(upper)) return true
77+
if (supportedCurrencyCodes === null) return !SCALE_SUFFIX.test(upper)
78+
return supportedCurrencyCodes.has(upper)
79+
}
80+
3681
/**
3782
* A leading currency marker: up to three letters and/or a symbol, optionally
38-
* behind a sign. The sign is captured so `-$12.50` keeps it.
83+
* behind a sign. The sign is captured so `-$12.50` keeps it, and the letters
84+
* are captured so {@link isCurrencyMarkerLetters} can vet them.
3985
*/
4086
const CURRENCY_MARKER_PREFIX =
41-
/^[\s\u00a0\u202f]*([+-]?)[\s\u00a0\u202f]*(?:\p{Sc}\p{L}{0,3}|\p{L}{1,3}\p{Sc}?)[\s\u00a0\u202f]*/u
87+
/^[\s\u00a0\u202f]*([+-]?)[\s\u00a0\u202f]*(?:\p{Sc}\p{L}{0,3}|(\p{L}{1,3})(\p{Sc})?)[\s\u00a0\u202f]*/u
4288

4389
/** The same, trailing. */
4490
const CURRENCY_MARKER_SUFFIX =
45-
/[\s\u00a0\u202f]*(?:\p{Sc}\p{L}{0,3}|\p{L}{1,3}\p{Sc}?)\.?[\s\u00a0\u202f]*$/u
91+
/[\s\u00a0\u202f]*(?:\p{Sc}\p{L}{0,3}|(\p{L}{1,3})(\p{Sc})?)\.?[\s\u00a0\u202f]*$/u
92+
93+
/**
94+
* Whether a matched marker may be stripped. A marker carrying a currency
95+
* SYMBOL needs no vetting — no magnitude abbreviation contains one — and
96+
* `letters === undefined` means the symbol-led alternative matched.
97+
*/
98+
function isStrippableMarker(letters: string | undefined, symbol: string | undefined): boolean {
99+
if (letters === undefined || symbol !== undefined) return true
100+
return isCurrencyMarkerLetters(letters)
101+
}
46102

47103
/** Only digits and separators, with at least one digit. */
48104
const AMOUNT_SHAPE = /^[+-]?[\d.,]*\d[\d.,]*$/
@@ -233,18 +289,17 @@ export function parseCurrencyInput(raw: unknown, currencyCode?: string): number
233289
// by a space or a symbol, so this distinguishes them without a symbol list.
234290
if (LETTER_TOUCHING_DIGIT.test(body)) return null
235291

236-
// A scale suffix is not a currency marker, and dropping it loses orders of
237-
// magnitude: `1.2 M` would read as 1.2, so a column of `1.2 M` / `3.4 M`
238-
// would convert cleanly and rewrite every cell a millionfold too small.
239-
// Refuse rather than guess what the writer meant.
240-
if (SCALE_SUFFIX.test(body)) return null
241-
242292
// Strip the currency marker: up to three letters (an ISO code, `kr`, `zł`)
243-
// optionally joined to a symbol (`R$`, `CHF`), at either end. Bounded at
244-
// three so prose does not qualify — `Revenue 5` keeps its letters and is
245-
// rejected below.
246-
const withoutPrefix = body.replace(CURRENCY_MARKER_PREFIX, '$1')
247-
const withoutMarkers = withoutPrefix.replace(CURRENCY_MARKER_SUFFIX, '')
293+
// optionally joined to a symbol (`R$`, `CHF`), at either end. Only a marker
294+
// that actually NAMES a currency is stripped — a magnitude word like `1.2 M`
295+
// or `3,4 mrd` is left in place, so it fails the amount-shape check below
296+
// instead of silently reading as 1.2, orders of magnitude too small.
297+
const withoutPrefix = body.replace(CURRENCY_MARKER_PREFIX, (match, sign, letters, symbol) =>
298+
isStrippableMarker(letters, symbol) ? sign : match
299+
)
300+
const withoutMarkers = withoutPrefix.replace(CURRENCY_MARKER_SUFFIX, (match, letters, symbol) =>
301+
isStrippableMarker(letters, symbol) ? '' : match
302+
)
248303
const cleaned = withoutMarkers.replace(/[\s\u00a0\u202f\u2019']/gu, '')
249304
// What remains must be ONLY digits and separators. Anything else — a
250305
// US-format date, leftover prose — is not an amount.

0 commit comments

Comments
 (0)