Skip to content

Commit afc0a7f

Browse files
waleedlatif1waleedlatif1
committed
feat(tables): drop URL and Duration, harden Email and Phone validation
Removes the `url` and `duration` types entirely — type files, icons, registry entries, tests and docs. `percent` stays, and with it the shared `precision` key that also gives Number columns decimal places. The remaining two now validate properly rather than permissively. Email was `/^[^\s@]+@[^\s@.]+(\.[^\s@.]+)+$/`, which accepted `a..b@x.com`, `.a@x.com`, `a.@x.com`, `a,b@x.com` (a LIST of addresses, not one), `a@-x.com`, `a@x-.com`, `a@x.123`, and an address of unbounded length. It now checks an RFC 5322 dot-atom local part, real DNS labels, an alphabetic TLD, and the RFC 5321 length caps (254 total, 64 local, 253 domain, 63 per label). Quoted local parts stay unsupported, deliberately: admitting them would mean carrying quoting rules through case-folding and every downstream comparison. Phone accepted `+0123456789` — an E.164 number whose country code starts with zero, which no network can route. A leading `+` now requires a country code starting 1-9, while a number WITHOUT one keeps its leading zero, since that is a real national trunk prefix (UK 020, DE 030). Whether the country code exists is deliberately not checked: that needs a table that goes stale and then starts rejecting valid numbers. Every new rule is covered, and each was verified to fail with the old loose rule restored — 12 email cases and the phone one.
1 parent a14c409 commit afc0a7f

13 files changed

Lines changed: 220 additions & 394 deletions

File tree

‎apps/docs/content/docs/en/tables/index.mdx‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,13 @@ Every column has a type, which decides how its values are stored and validated.
2929
| **Select** | One of a fixed set of options, or several | `Pro` |
3030
| **Email** | An email address | `person@example.com` |
3131
| **Phone** | A phone number | `+1 555 123 4567` |
32-
| **URL** | A link | `https://sim.ai` |
33-
| **Duration** | A length of time | `1:30:00` |
3432
| **JSON** | An object or array | `{ "tier": "pro" }` |
3533

3634
Types are enforced as you enter values, so a Number column only takes numbers.
3735

38-
Currency, Percent, and Duration columns all store a plain number, so filters, sorts, and exports see the amount itself rather than its formatting — `> 50%` and `>= 1h` are ordinary numeric comparisons. Changing a column's currency relabels it; it does not convert the amounts. Number and Percent columns take a decimal-place setting that changes how values are shown without rounding what is stored.
36+
Currency and Percent columns both store a plain number, so filters, sorts, and exports see the amount itself rather than its formatting — `> 50%` is an ordinary numeric comparison. Changing a column's currency relabels it; it does not convert the amounts. Number and Percent columns take a decimal-place setting that changes how values are shown without rounding what is stored.
3937

40-
Email, Phone, and URL columns tidy values as you enter them — addresses are lower-cased, phone numbers stripped to digits, and links given a scheme — so the same value entered two ways matches. URL cells render as clickable links.
38+
Email and Phone columns tidy and check values as you enter them — addresses are lower-cased, phone numbers stripped to their digits — so the same value entered two ways matches. A value that isn't a valid address or number is rejected rather than stored.
4139

4240
A Date column can carry a time of day or just a calendar date. Turning **Include time** off on a column that already has times will drop them.
4341

‎apps/sim/lib/table/__tests__/column-types-contact.test.ts‎

Lines changed: 99 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import { describe, expect, it } from 'vitest'
1313
import {
1414
COLUMN_TYPE_REGISTRY,
1515
columnTypeById,
16-
isValueCompatible,
1716
metadataRewritesCells,
1817
metadataWithoutClears,
1918
ownedKeysOf,
@@ -27,116 +26,138 @@ import type { ColumnDefinition } from '@/lib/table/types'
2726
const column = (type: ColumnDefinition['type'], extra: Partial<ColumnDefinition> = {}) =>
2827
({ name: 'c', type, ...extra }) as ColumnDefinition
2928

30-
describe('email', () => {
29+
describe('email validation', () => {
3130
const col = column('email')
31+
const coerce = (v: unknown) => COLUMN_TYPE_REGISTRY.email.coerce(v as never, col)
3232

3333
it.each([
3434
[' Ada@Example.COM ', 'ada@example.com'],
3535
['person@example.co.uk', 'person@example.co.uk'],
3636
['a.b+tag@sub.example.com', 'a.b+tag@sub.example.com'],
37-
])('normalizes %s to %s', (input, expected) => {
38-
const result = COLUMN_TYPE_REGISTRY.email.coerce(input, col)
37+
["o'brien@example.com", "o'brien@example.com"],
38+
['user_name-1@ex-ample.com', 'user_name-1@ex-ample.com'],
39+
])('accepts and normalizes %s', (input, expected) => {
40+
const result = coerce(input)
3941
expect(result.ok && result.value).toBe(expected)
4042
})
4143

42-
it.each(['no-at-sign', 'two @spaces.com', '@example.com', 'a@b', 'a@.com'])(
43-
'rejects %s',
44-
(input) => {
45-
expect(COLUMN_TYPE_REGISTRY.email.coerce(input, col).ok).toBe(false)
44+
it.each([
45+
['no-at-sign', 'no @'],
46+
['@example.com', 'no local part'],
47+
['a@', 'no domain'],
48+
['a@b', 'dotless domain'],
49+
['a b@example.com', 'whitespace in the local part'],
50+
['a..b@example.com', 'consecutive dots in the local part'],
51+
['.a@example.com', 'leading dot in the local part'],
52+
['a.@example.com', 'trailing dot in the local part'],
53+
['a,b@example.com', 'comma — the cell holds a LIST, not one address'],
54+
['a;b@example.com', 'semicolon — likewise'],
55+
['"quoted name"@example.com', 'quoted local part, deliberately unsupported'],
56+
['a@-example.com', 'domain label starting with a hyphen'],
57+
['a@example-.com', 'domain label ending with a hyphen'],
58+
['a@example..com', 'empty domain label'],
59+
['a@example.123', 'numeric TLD'],
60+
['a@example.c', 'single-character TLD'],
61+
['a@b@c.com', 'two @ signs'],
62+
])('rejects %s (%s)', (input) => {
63+
expect(coerce(input).ok).toBe(false)
64+
})
65+
66+
it('enforces the RFC 5321 local-part cap of 64', () => {
67+
expect(coerce(`${'a'.repeat(64)}@example.com`).ok).toBe(true)
68+
expect(coerce(`${'a'.repeat(65)}@example.com`).ok).toBe(false)
69+
})
70+
71+
it('enforces the RFC 5321 total cap of 254', () => {
72+
// Built at the boundary with every OTHER limit satisfied — local part 64,
73+
// each label under 63 — so the total is the only thing under test.
74+
const local = 'a'.repeat(64)
75+
const at254 = `${'b'.repeat(63)}.${'c'.repeat(63)}.${'d'.repeat(57)}.com`
76+
expect(`${local}@${at254}`).toHaveLength(254)
77+
expect(coerce(`${local}@${at254}`).ok).toBe(true)
78+
79+
const at255 = `${'b'.repeat(63)}.${'c'.repeat(63)}.${'d'.repeat(58)}.com`
80+
expect(`${local}@${at255}`).toHaveLength(255)
81+
expect(coerce(`${local}@${at255}`).ok).toBe(false)
82+
})
83+
84+
it('enforces the 63-character DNS label cap', () => {
85+
expect(coerce(`a@${'b'.repeat(63)}.com`).ok).toBe(true)
86+
expect(coerce(`a@${'b'.repeat(64)}.com`).ok).toBe(false)
87+
})
88+
89+
it('accepts nothing its own validateCell then rejects', () => {
90+
for (const input of ['Ada@Example.com', 'a.b+t@x.co.uk', '']) {
91+
const result = coerce(input)
92+
expect(result.ok, input).toBe(true)
93+
if (result.ok) expect(COLUMN_TYPE_REGISTRY.email.validateCell(result.value, col)).toBeNull()
4694
}
47-
)
95+
})
4896

4997
it('case-folds so enrichment matching cannot miss on capitalization', () => {
50-
const upper = COLUMN_TYPE_REGISTRY.email.coerce('ADA@EXAMPLE.COM', col)
51-
const lower = COLUMN_TYPE_REGISTRY.email.coerce('ada@example.com', col)
98+
const upper = coerce('ADA@EXAMPLE.COM')
99+
const lower = coerce('ada@example.com')
52100
expect(upper.ok && upper.value).toBe(lower.ok && lower.value)
53101
})
54102
})
55103

56-
describe('phone', () => {
104+
describe('phone validation', () => {
57105
const col = column('phone')
106+
const coerce = (v: unknown) => COLUMN_TYPE_REGISTRY.phone.coerce(v as never, col)
58107

59108
it.each([
60109
['+1 (555) 123-4567', '+15551234567'],
61110
['555-123-4567', '5551234567'],
62111
['+44 20 7123 4567', '+442071234567'],
63-
])('normalizes %s to %s', (input, expected) => {
64-
const result = COLUMN_TYPE_REGISTRY.phone.coerce(input, col)
112+
['+81-3-1234-5678', '+81312345678'],
113+
[' +1.555.123.4567 ', '+15551234567'],
114+
])('accepts and normalizes %s', (input, expected) => {
115+
const result = coerce(input)
65116
expect(result.ok && result.value).toBe(expected)
66117
})
67118

68-
it('refuses an extension rather than silently truncating to the wrong number', () => {
69-
expect(COLUMN_TYPE_REGISTRY.phone.coerce('555-123-4567 x89', col).ok).toBe(false)
119+
it('keeps a national leading zero, which is a real trunk prefix', () => {
120+
const result = coerce('020 7123 4567')
121+
expect(result.ok && result.value).toBe('02071234567')
70122
})
71123

72-
it.each([['12345'], ['1234567890123456'], ['not a phone']])('rejects %s', (input) => {
73-
expect(COLUMN_TYPE_REGISTRY.phone.coerce(input, col).ok).toBe(false)
124+
it('refuses an E.164 number whose country code starts with 0', () => {
125+
// No network can route it, so storing it as international would be a lie.
126+
expect(coerce('+0123456789').ok).toBe(false)
74127
})
75128

76-
it('keeps a leading + that a numeric cast would have dropped', () => {
77-
const result = COLUMN_TYPE_REGISTRY.phone.coerce('+15551234567', col)
78-
expect(result.ok && String(result.value).startsWith('+')).toBe(true)
79-
expect(columnTypeById('phone').jsonbCast).toBeNull()
80-
})
81-
})
82-
83-
describe('url', () => {
84-
const col = column('url')
85-
86129
it.each([
87-
['sim.ai', 'https://sim.ai/'],
88-
['https://sim.ai/docs', 'https://sim.ai/docs'],
89-
['http://example.com', 'http://example.com/'],
90-
])('normalizes %s to %s', (input, expected) => {
91-
const result = COLUMN_TYPE_REGISTRY.url.coerce(input, col)
92-
expect(result.ok && result.value).toBe(expected)
130+
['555-123-4567 x89', 'an extension has no E.164 form'],
131+
['12345', 'too few digits'],
132+
['1234567890123456', 'too many digits'],
133+
['not a phone', 'letters'],
134+
['+', 'a lone plus'],
135+
['555-1234, 555-5678', 'two numbers in one cell'],
136+
['555/1234567', 'a slash'],
137+
])('rejects %s (%s)', (input) => {
138+
expect(coerce(input).ok).toBe(false)
139+
})
140+
141+
it('accepts a numeric CSV cell but refuses one that cannot be a number', () => {
142+
expect(coerce(15551234567).ok).toBe(true)
143+
// Negative would have its sign eaten as a separator and stored positive.
144+
expect(coerce(-15551234567).ok).toBe(false)
145+
expect(coerce(1.5).ok).toBe(false)
146+
// Past MAX_SAFE_INTEGER the value has already lost digits to float64
147+
// before it reaches us, so it is no longer what the file contained.
148+
expect(coerce(Number.MAX_SAFE_INTEGER + 2).ok).toBe(false)
149+
})
150+
151+
it('never casts to numeric, so the + and leading zeros survive', () => {
152+
expect(columnTypeById('phone').jsonbCast).toBeNull()
93153
})
94154

95-
it.each(['javascript:alert(1)', 'data:text/html,<script>', 'file:///etc/passwd'])(
96-
'refuses the non-http scheme %s, which the grid would render as a live link',
97-
(input) => {
98-
expect(COLUMN_TYPE_REGISTRY.url.coerce(input, col).ok).toBe(false)
155+
it('accepts nothing its own validateCell then rejects', () => {
156+
for (const input of ['+1 (555) 123-4567', '020 7123 4567', '']) {
157+
const result = coerce(input)
158+
expect(result.ok, input).toBe(true)
159+
if (result.ok) expect(COLUMN_TYPE_REGISTRY.phone.validateCell(result.value, col)).toBeNull()
99160
}
100-
)
101-
102-
it('renders as linkable so the grid promotes it to a chip', () => {
103-
expect(COLUMN_TYPE_REGISTRY.url.display?.('https://sim.ai', col)).toEqual({
104-
kind: 'linkable',
105-
text: 'https://sim.ai',
106-
})
107-
})
108-
})
109-
110-
describe('duration', () => {
111-
const col = column('duration')
112-
113-
it.each([
114-
['1:30', 90],
115-
['1:30:00', 5400],
116-
['90:00', 5400],
117-
['1h 30m', 5400],
118-
['45s', 45],
119-
['5400', 5400],
120-
[5400, 5400],
121-
])('parses %s to %s seconds', (input, expected) => {
122-
const result = COLUMN_TYPE_REGISTRY.duration.coerce(input as never, col)
123-
expect(result.ok && result.value).toBe(expected)
124-
})
125-
126-
it.each(['1:75', 'abc', '-5', '1:2:3:4'])('rejects %s', (input) => {
127-
expect(COLUMN_TYPE_REGISTRY.duration.coerce(input, col).ok).toBe(false)
128-
})
129-
130-
it('round-trips display through the editor unchanged', () => {
131-
const shown = COLUMN_TYPE_REGISTRY.duration.formatForInput(5400, col)
132-
const reparsed = COLUMN_TYPE_REGISTRY.duration.coerce(shown, col)
133-
expect(reparsed.ok && reparsed.value).toBe(5400)
134-
})
135-
136-
it('refuses to bulk-convert a number column, whose values are not known to be seconds', () => {
137-
expect(isValueCompatible(90, col)).toBe(false)
138-
// A single deliberate write still means seconds.
139-
expect(COLUMN_TYPE_REGISTRY.duration.coerce(90, col).ok).toBe(true)
140161
})
141162
})
142163

@@ -192,22 +213,6 @@ describe('audit regressions', () => {
192213
expect(COLUMN_TYPE_REGISTRY.date.validateCell(result.value, dateOnly)).toBeNull()
193214
})
194215

195-
it('accepts a bare host:port URL instead of reading the host as a scheme', () => {
196-
const col = column('url')
197-
const result = COLUMN_TYPE_REGISTRY.url.coerce('example.com:8080/path', col)
198-
expect(result.ok && result.value).toBe('https://example.com:8080/path')
199-
})
200-
201-
it('does not mutate a duration cell when the editor opens and closes untouched', () => {
202-
const col = column('duration')
203-
const stored = COLUMN_TYPE_REGISTRY.duration.coerce('90.7', col)
204-
expect(stored.ok).toBe(true)
205-
if (!stored.ok) return
206-
const shown = COLUMN_TYPE_REGISTRY.duration.formatForInput(stored.value, col)
207-
const reopened = COLUMN_TYPE_REGISTRY.duration.coerce(shown, col)
208-
expect(reopened.ok && reopened.value).toBe(stored.value)
209-
})
210-
211216
it('refuses a negative number for a phone rather than storing it positive', () => {
212217
expect(COLUMN_TYPE_REGISTRY.phone.coerce(-15551234567, column('phone')).ok).toBe(false)
213218
})
@@ -235,8 +240,6 @@ describe('review-round regressions', () => {
235240
// stayed a string, hit the numeric cast, and the range filter was rejected.
236241
const cases: Array<[ColumnDefinition['type'], string, number]> = [
237242
['percent', '50%', 50],
238-
['duration', '1h', 3600],
239-
['duration', '1:30', 90],
240243
['currency', '$1,234.56', 1234.56],
241244
]
242245
for (const [type, typed, expected] of cases) {

‎apps/sim/lib/table/__tests__/sql.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ describe('SQL Builder', () => {
127127
expect(out).not.toContain('::timestamp')
128128
})
129129

130-
it.each(['email', 'phone', 'url'] as const)(
130+
it.each(['email', 'phone'] as const)(
131131
'compares a %s column as text rather than casting it to numeric',
132132
(type) => {
133133
const cols: ColumnDefinition[] = [{ name: 'c', type }]

0 commit comments

Comments
 (0)