Skip to content

Commit e6b7301

Browse files
committed
fix(tables): read exponent-form amounts and reject bad currency PATCHes up front
Two P1s from review. Scientific notation lost magnitude. `String()` emits exponent form past 1e21, so a stored amount round-trips through the editor as `1e+21` — and the sanitizer treated the `e` as decoration to strip, reading it back as 121. An untouched cell silently lost 19 orders of magnitude on its next edit. Exponent form is now taken at face value, but only when the string is wholly a numeric literal once symbols are removed, so `12 EUR` (whose `E` survives the strip) still parses through the separator path. A failed currency PATCH left a partial rename. `renameColumn` commits in its own transaction before the currency write, so a `currencyCode` the service would reject — an unsupported code, or any code on a non-currency column — errored only after the rename had stuck. Both are now caught before the first write, matching the guard the route already applies to unique-on-select for exactly this reason.
1 parent 01e1e3c commit e6b7301

4 files changed

Lines changed: 85 additions & 0 deletions

File tree

apps/sim/app/api/table/[tableId]/columns/route.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
updateColumnType,
2121
} from '@/lib/table'
2222
import { columnMatchesRef } from '@/lib/table/column-keys'
23+
import { isSupportedCurrencyCode } from '@/lib/table/currency'
2324
import {
2425
accessError,
2526
checkAccess,
@@ -143,6 +144,28 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu
143144
// changing: an options-only update on an existing select column carries the
144145
// same hazard as a conversion does.
145146
const resultingType = updates.type ?? currentColumn?.type
147+
// Same reason as the constraint guard below: `renameColumn` runs first and
148+
// commits on its own, so anything `updateColumnCurrency` would reject has
149+
// to be caught before that write rather than inside the last one —
150+
// otherwise the rename sticks and the request still errors.
151+
if (updates.currencyCode !== undefined) {
152+
if (resultingType !== 'currency') {
153+
return NextResponse.json(
154+
{
155+
error: `Cannot set currency on column "${validated.columnName}" of type "${resultingType}"`,
156+
},
157+
{ status: 400 }
158+
)
159+
}
160+
if (!isSupportedCurrencyCode(updates.currencyCode)) {
161+
return NextResponse.json(
162+
{
163+
error: `Invalid currency code "${updates.currencyCode}". Use an ISO 4217 code, e.g. USD`,
164+
},
165+
{ status: 400 }
166+
)
167+
}
168+
}
146169
if (updates.unique === true && resultingType === 'select') {
147170
return NextResponse.json({ error: 'Cannot set a select column as unique' }, { status: 400 })
148171
}

apps/sim/app/api/v1/tables/[tableId]/columns/route.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
updateColumnType,
2020
} from '@/lib/table'
2121
import { columnMatchesRef } from '@/lib/table/column-keys'
22+
import { isSupportedCurrencyCode } from '@/lib/table/currency'
2223
import {
2324
accessError,
2425
checkAccess,
@@ -177,6 +178,28 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu
177178
// changing: an options-only update on an existing select column carries the
178179
// same hazard as a conversion does.
179180
const resultingType = updates.type ?? currentColumn?.type
181+
// Same reason as the constraint guard below: `renameColumn` runs first and
182+
// commits on its own, so anything `updateColumnCurrency` would reject has
183+
// to be caught before that write rather than inside the last one —
184+
// otherwise the rename sticks and the request still errors.
185+
if (updates.currencyCode !== undefined) {
186+
if (resultingType !== 'currency') {
187+
return NextResponse.json(
188+
{
189+
error: `Cannot set currency on column "${validated.columnName}" of type "${resultingType}"`,
190+
},
191+
{ status: 400 }
192+
)
193+
}
194+
if (!isSupportedCurrencyCode(updates.currencyCode)) {
195+
return NextResponse.json(
196+
{
197+
error: `Invalid currency code "${updates.currencyCode}". Use an ISO 4217 code, e.g. USD`,
198+
},
199+
{ status: 400 }
200+
)
201+
}
202+
}
180203
if (updates.unique === true && resultingType === 'select') {
181204
return NextResponse.json({ error: 'Cannot set a select column as unique' }, { status: 400 })
182205
}

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,31 @@ describe('parseCurrencyInput', () => {
6464
expect(parseCurrencyInput('1234.5')).toBe(1234.5)
6565
})
6666

67+
it('reads exponent form at face value', () => {
68+
// `String()` emits exponent form past 1e21, so a stored amount round-trips
69+
// through the editor as `1e+21`. Treating the `e` as decoration to strip
70+
// read that back as 121 — a silent 19-orders-of-magnitude loss on the next
71+
// edit of an untouched cell.
72+
expect(parseCurrencyInput('1e5')).toBe(100000)
73+
expect(parseCurrencyInput('1e+21')).toBe(1e21)
74+
expect(parseCurrencyInput('1.5e-3')).toBe(0.0015)
75+
expect(parseCurrencyInput('-1e5')).toBe(-100000)
76+
expect(parseCurrencyInput('(1e5)')).toBe(-100000)
77+
expect(parseCurrencyInput('$1e5')).toBe(100000)
78+
})
79+
80+
it('does not mistake an ISO code or prose for an exponent', () => {
81+
// `EUR` survives the symbol strip with its `E` intact; it must still parse
82+
// through the ordinary separator path.
83+
expect(parseCurrencyInput('12 EUR')).toBe(12)
84+
expect(parseCurrencyInput('EUR 12,50')).toBe(12.5)
85+
expect(parseCurrencyInput('Revenue 5')).toBe(5)
86+
})
87+
88+
it('round-trips a magnitude that stringifies to exponent form', () => {
89+
expect(parseCurrencyInput(formatCurrencyForInput(1e21))).toBe(1e21)
90+
})
91+
6792
it('rejects values carrying no amount', () => {
6893
expect(parseCurrencyInput('')).toBeNull()
6994
expect(parseCurrencyInput(' ')).toBeNull()

apps/sim/lib/table/currency.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,20 @@ export function parseCurrencyInput(raw: unknown): number | null {
120120
const parenthesized = /^\((.*)\)$/.exec(trimmed)
121121
const body = parenthesized ? parenthesized[1] : trimmed
122122

123+
// Exponent form first, and taken at face value. `String()` emits it for any
124+
// magnitude past 1e21, so a stored amount round-trips through the editor as
125+
// `1e+21` — and stripping the `e` as decoration would read that back as 121,
126+
// silently losing 19 orders of magnitude. Only a string that is *wholly* a
127+
// numeric literal once symbols are removed qualifies, so `12 EUR` (whose `E`
128+
// survives the strip) still falls through to the separator logic below.
129+
const exponentCandidate = body.replace(/[^\d.,\-+eE]/g, '')
130+
if (/[eE]/.test(exponentCandidate)) {
131+
const parsed = Number(exponentCandidate)
132+
if (Number.isFinite(parsed)) {
133+
return parenthesized ? -Math.abs(parsed) : parsed
134+
}
135+
}
136+
123137
// Drop symbols, letters, and every flavor of space, leaving only digits, the
124138
// two separator characters, and a leading sign.
125139
const stripped = body.replace(/[^\d.,\-+]/g, '')

0 commit comments

Comments
 (0)