From 63935eb7f773914f449a17b42dedd572a6ff9896 Mon Sep 17 00:00:00 2001 From: Sai Asish Y Date: Sun, 18 Oct 2026 12:13:43 -0700 Subject: [PATCH 1/4] fix(web): reject a record array cell that is missing its closing parenthesis Signed-off-by: Sai Asish Y --- apps/web/src/utils/__tests__/upload.test.ts | 7 +++++++ apps/web/src/utils/upload.ts | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/apps/web/src/utils/__tests__/upload.test.ts b/apps/web/src/utils/__tests__/upload.test.ts index 46fdc942a..26c5f72ee 100644 --- a/apps/web/src/utils/__tests__/upload.test.ts +++ b/apps/web/src/utils/__tests__/upload.test.ts @@ -536,6 +536,13 @@ describe('Zod3', () => { it('should leave out a field the record does not mention', async () => { await expect(processItems('RECORD_ARRAY(name:item1)')).resolves.toMatchObject([{ items: [{ name: 'item1' }] }]); }); + + it('should reject a record array missing its closing parenthesis, rather than dropping its last character', async () => { + await expectRejection( + processItems('RECORD_ARRAY(name:item1,quantity:12'), + "Syntax error in RECORD_ARRAY declaration: RECORD_ARRAY(name:item1,quantity:12 at column name: 'items' and row number '1'" + ); + }); }); }); }); diff --git a/apps/web/src/utils/upload.ts b/apps/web/src/utils/upload.ts index 56240b564..f08dd7291 100644 --- a/apps/web/src/utils/upload.ts +++ b/apps/web/src/utils/upload.ts @@ -141,7 +141,7 @@ function extractRecordArrayEntry(entry: string): { [key: string]: string }[] { entry = entry.trim(); const opening = 'RECORD_ARRAY('; const closing = ')'; - if (!entry.startsWith(opening) && !entry.endsWith(closing)) { + if (!entry.startsWith(opening) || !entry.endsWith(closing)) { throw new UploadError({ en: `Syntax error in RECORD_ARRAY declaration: ${entry}`, fr: `Erreur de syntaxe dans la déclaration RECORD_ARRAY : ${entry}` From 3308f49406f67e6b2487e55683269172ea7e7a70 Mon Sep 17 00:00:00 2001 From: Sai Asish Y Date: Sun, 18 Oct 2026 12:13:45 -0700 Subject: [PATCH 2/4] fix(web): split each record array field on its first colon only Signed-off-by: Sai Asish Y --- apps/web/src/utils/__tests__/upload.test.ts | 6 ++++++ apps/web/src/utils/upload.ts | 8 +++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/apps/web/src/utils/__tests__/upload.test.ts b/apps/web/src/utils/__tests__/upload.test.ts index 26c5f72ee..869463600 100644 --- a/apps/web/src/utils/__tests__/upload.test.ts +++ b/apps/web/src/utils/__tests__/upload.test.ts @@ -543,6 +543,12 @@ describe('Zod3', () => { "Syntax error in RECORD_ARRAY declaration: RECORD_ARRAY(name:item1,quantity:12 at column name: 'items' and row number '1'" ); }); + + it('should keep a colon inside a record array value, splitting each field on its first colon only', async () => { + await expect(processItems('RECORD_ARRAY(name:10:30,quantity:1;)')).resolves.toMatchObject([ + { items: [{ name: '10:30', quantity: 1 }] } + ]); + }); }); }); }); diff --git a/apps/web/src/utils/upload.ts b/apps/web/src/utils/upload.ts index f08dd7291..6e3d1f714 100644 --- a/apps/web/src/utils/upload.ts +++ b/apps/web/src/utils/upload.ts @@ -165,15 +165,17 @@ function extractRecordArrayEntry(entry: string): { [key: string]: string }[] { } const records: { [key: string]: string } = {}; recordsList.forEach((rawRecord, i) => { - const [recordKey, recordValue] = rawRecord.split(':').map((s) => s.trim()); - if (!recordKey) { + const separatorIndex = rawRecord.indexOf(':'); + const recordKey = rawRecord.slice(0, separatorIndex).trim(); + const recordValue = rawRecord.slice(separatorIndex + 1).trim(); + if (separatorIndex === -1 || !recordKey) { throw new UploadError({ en: `Malformed record at index ${i}`, fr: `Enregistrement mal formé à l'index ${i}` }); } //recordValue can be empty if the value is optional - records[recordKey] = recordValue!; + records[recordKey] = recordValue; }); return records; }); From f8e2cc7b7836f326936f6912fd2cf923e8ec7fa4 Mon Sep 17 00:00:00 2001 From: Sai Asish Y Date: Thu, 29 Oct 2026 02:14:21 -0700 Subject: [PATCH 3/4] test(web): cover a record array field with no colon Signed-off-by: Sai Asish Y --- apps/web/src/utils/__tests__/upload.test.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/apps/web/src/utils/__tests__/upload.test.ts b/apps/web/src/utils/__tests__/upload.test.ts index 869463600..2f78a36bb 100644 --- a/apps/web/src/utils/__tests__/upload.test.ts +++ b/apps/web/src/utils/__tests__/upload.test.ts @@ -549,6 +549,13 @@ describe('Zod3', () => { { items: [{ name: '10:30', quantity: 1 }] } ]); }); + + it('should reject a record array field with no colon as a malformed record', async () => { + await expectRejection( + processItems('RECORD_ARRAY(name:item1,quantity)'), + "Malformed record at index 1 at column name: 'items' and row number '1'" + ); + }); }); }); }); From 8a21a8536d7183f220ba531f728d01395614e37f Mon Sep 17 00:00:00 2001 From: Sai Asish Y Date: Thu, 29 Oct 2026 02:14:23 -0700 Subject: [PATCH 4/4] test(testing): reject a record array upload missing its closing parenthesis Signed-off-by: Sai Asish Y --- testing/src/specs/upload.spec.ts | 72 +++++++++++++++++++++++++++++++ testing/src/support/api-client.ts | 22 ++++++++++ 2 files changed, 94 insertions(+) diff --git a/testing/src/specs/upload.spec.ts b/testing/src/specs/upload.spec.ts index e4a5730dd..1ec4822f0 100644 --- a/testing/src/specs/upload.spec.ts +++ b/testing/src/specs/upload.spec.ts @@ -1,6 +1,7 @@ import type { UploadInstrumentRecordsData } from '@opendatacapture/schemas/instrument-records'; import { UploadInstrumentPage } from '../pages/_app/upload/$instrumentId.page'; +import { UploadPage } from '../pages/_app/upload/index.page'; import { HAPPINESS_RECORD } from '../support/constants'; import { expect, test } from '../support/fixtures'; @@ -63,6 +64,41 @@ test.describe('upload', () => { await expect(uploadInstrumentPage.submitButton).toBeVisible(); }); + // A cell missing its closing parenthesis was once accepted with its last character cut off, so + // `quantity:12` was stored as 1. No seeded instrument has a record-array field, so the test stores + // its own before creating the group that will use it. + test('should reject a record array cell missing its closing parenthesis, naming its column', async ({ + api, + isolatedGroupManager, + page, + uniqueId + }) => { + const title = `Record Array Upload ${uniqueId}`; + await api.createInstrument(recordArrayFormBundle({ name: `RECORD_ARRAY_UPLOAD_${uniqueId}`, title })); + await isolatedGroupManager(); + const csv = [ + 'subjectID,date,items', + `RecordArray${uniqueId},2024-01-15,"RECORD_ARRAY(name:item1,quantity:12"` + ].join('\n'); + + const uploadPage = new UploadPage(page); + await uploadPage.goto('/upload'); + await uploadPage.selectInstrument(title); + await page.waitForURL('**/upload/**'); + + const uploadInstrumentPage = new UploadInstrumentPage(page); + await expect(uploadInstrumentPage.pageHeader).toContainText(title); + await uploadInstrumentPage.uploadFile({ buffer: Buffer.from(csv), mimeType: 'text/csv', name: 'items.csv' }); + await uploadInstrumentPage.submitButton.click(); + + await expect(uploadInstrumentPage.errorHeading).toBeVisible(); + await expect( + page.getByText( + "Syntax error in RECORD_ARRAY declaration: RECORD_ARRAY(name:item1,quantity:12 at column name: 'items' and row number '1'" + ) + ).toBeVisible(); + }); + // The response was previously every record in the group for the instrument, so a second upload // leaked the first one's records back to the caller. An earlier upload to the same group and // instrument is what distinguishes the scoped response from the group-wide one. @@ -133,3 +169,39 @@ test.describe('upload instrument selection while loading', () => { expect(errors.filter((error) => error.includes('Maximum update depth exceeded'))).toStrictEqual([]); }); }); + +/** + * A form with one record-array field, written as the bundle the api stores: an expression that + * evaluates to the instrument. Its runtime import resolves through the `__resolveImport` the api's + * sandbox provides, and against the web origin in the browser, as a bundled import would. + */ +function recordArrayFormBundle({ name, title }: { name: string; title: string }) { + return `(async () => { + const runtimeImport = (specifier) => import(globalThis.__resolveImport?.(specifier) ?? specifier); + const { z } = await runtimeImport('/runtime/v1/zod@3.x/index.js'); + return { + __runtimeVersion: 1, + content: { + items: { + fieldset: { + name: { kind: 'string', label: 'Name', variant: 'input' }, + quantity: { kind: 'number', label: 'Quantity', variant: 'input' } + }, + kind: 'record-array', + label: 'Items' + } + }, + details: { + description: 'A form with one record-array field', + license: 'Apache-2.0', + title: ${JSON.stringify(title)} + }, + internal: { edition: 1, name: ${JSON.stringify(name)} }, + kind: 'FORM', + language: 'en', + measures: null, + tags: [], + validationSchema: z.object({ items: z.array(z.object({ name: z.string(), quantity: z.number() })) }) + }; + })()`; +} diff --git a/testing/src/support/api-client.ts b/testing/src/support/api-client.ts index ca9e3b838..2de2c5d8b 100644 --- a/testing/src/support/api-client.ts +++ b/testing/src/support/api-client.ts @@ -79,6 +79,28 @@ export class ApiClient { return group; } + /** + * Stores an instrument bundle the way the playground uploads one, with the reduced token minted by + * `GET /auth/create-instrument-token`. The id is derived from the bundle's internal name and + * edition, so each bundle needs a name of its own. Groups created afterwards can use it. + */ + async createInstrument(bundle: string): Promise { + const { accessToken } = await this.expectJson<{ accessToken: string }>( + this.request.get(`${API}/auth/create-instrument-token`, { headers: this.authHeaders }), + 200, + 'mint an instrument token' + ); + const instrument = await this.expectJson<{ id: string }>( + this.request.post(`${API}/instruments`, { + data: { bundle }, + headers: { Authorization: `Bearer ${accessToken}` } + }), + 201, + 'create instrument' + ); + return instrument.id; + } + /** * Creates one record against a session, the way the app does when an instrument is completed in a * sitting. Unlike {@link uploadRecords}, which always writes a RETROSPECTIVE session, this lets a