From d82240dd4d27bba4e954589e574448b3a0a076ce Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 16:16:43 +0000 Subject: [PATCH] fix(rest): stamp the export download filename in the business timezone (#8484) exportContentDisposition built the -YYYYMMDD-HHMMSS stamp from process-local getters, which read the deployment host's TZ rather than the caller's business timezone. After #8373 moved the export's contents onto the business timezone, the filename was the last export surface still on the host clock, so the name and the rows inside disagreed whenever TZ was not the business zone. The route had already resolved ExecutionContext.timezone one frame up for the cells; it is now threaded to the filename too, so both read one clock. The no-timezone fallback stays PROCESS-LOCAL, deliberately not UTC: each surface keeps its own historical output, and defaulting to UTC would silently re-time the filename of every deployment with a host TZ set. zonedWallClock now returns null when no usable zone resolves so each caller supplies its own fallback; the fallback pins own a skewed process TZ so they cannot pass vacuously on a UTC runner. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NaS1PAHJcPfAA2acnV53Tn --- .../export-filename-business-timezone.md | 39 +++++ packages/rest/src/export-format.test.ts | 136 ++++++++++++++++-- packages/rest/src/export-format.ts | 107 ++++++++++---- packages/rest/src/rest-server.ts | 6 +- 4 files changed, 249 insertions(+), 39 deletions(-) create mode 100644 .changeset/export-filename-business-timezone.md diff --git a/.changeset/export-filename-business-timezone.md b/.changeset/export-filename-business-timezone.md new file mode 100644 index 0000000000..8e11a00ee3 --- /dev/null +++ b/.changeset/export-filename-business-timezone.md @@ -0,0 +1,39 @@ +--- +"@objectstack/rest": patch +--- + +fix(rest): stamp the export download's filename in the business timezone (#8484) + +`exportContentDisposition` built the `-YYYYMMDD-HHMMSS` half of the suggested +filename from process-local getters (`now.getFullYear()` / `getHours()` / …), +which read the deployment host's `TZ` — a hosting fact, not the caller's +business timezone. The route had already resolved that timezone one frame up +(`ExecutionContext.timezone`, the platform-default → global → tenant cascade) +and simply never passed it here. + +After #8373 moved the export's **contents** onto the business timezone, the +filename was the last export surface still on the host clock, so the two +disagreed exactly when `TZ` was not the business zone: a container at `TZ=UTC` +serving an Asia/Shanghai tenant downloaded `orders-20260731-220000.csv` whose +first row read `2026-08-01 06:00:00` — off by a day, and at a month boundary by +a month. The name and the rows inside it now read one clock. + +**The no-timezone fallback stays PROCESS-LOCAL, deliberately not UTC.** This is +the opposite of the cell path's UTC fallback, and the asymmetry is the point: +each fallback preserves the historical output of the surface it serves. The +cells were hardcoded to UTC before #8373; this filename has always used the +process clock. Defaulting it to UTC would look safer while silently re-timing +the filename of every deployment that sets a host `TZ` but resolves no business +timezone — a user-visible rename for zero correctness gain. An explicitly +resolved `'UTC'` is a *resolved* zone, not a missing one, and does produce a UTC +stamp regardless of the host. + +The shared clock helper is split rather than parameterised with a default: +`zonedWallClock` now returns `null` when no usable zone resolves, and each of +the two callers supplies its own fallback at the call site where it can be read +and pinned. Baking either fallback into the shared helper would silently +re-time the other surface. + +Filename **naming** is untouched — label selection, sanitization and the RFC +5987/6266 `filename*` encoding all behave exactly as before, and the export's +contents are not touched at all. diff --git a/packages/rest/src/export-format.test.ts b/packages/rest/src/export-format.test.ts index 3bee3d4ce9..468cc15fc8 100644 --- a/packages/rest/src/export-format.test.ts +++ b/packages/rest/src/export-format.test.ts @@ -1,55 +1,171 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. /** - * Unit tests for the xlsx colour helpers on the export path: {@link toArgb} - * (hex → exceljs ARGB) and {@link cellFontColor} (select/radio option colour - * for one cell). Both are pure and return `undefined` whenever a cell should - * stay unstyled, so the export never emits an invalid workbook. + * Unit tests for the pure helpers on the export path: + * + * - {@link exportContentDisposition} — the download's suggested filename: how + * it is named and sanitized, and (#8484) which clock its timestamp reads. + * - {@link toArgb} (hex → exceljs ARGB) and {@link cellFontColor} (select/radio + * option colour for one cell). Both are pure and return `undefined` whenever + * a cell should stay unstyled, so the export never emits an invalid workbook. + * - {@link buildFieldMetaMap} — the presentation-only metadata copy (#6536). */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { toArgb, cellFontColor, exportContentDisposition, + formatCellValue, buildFieldMetaMap, type ExportFieldMeta, } from './export-format'; +/** + * The NAMING half of the header: label selection, sanitization, RFC 5987 + * encoding, zero-padding. Every case here passes `undefined` for the business + * timezone, so all of them also ride the #8484 process-local fallback path. + * + * Their expected stamps did NOT move when #8484 landed, and that is the point + * rather than an oversight: `NOW` is built from LOCAL calendar components + * (`new Date(2026, 6, 14, …)`), so the local getters read back the same + * components under every host zone. Pinning `20260714-153045` is therefore a + * host-zone-independent statement that "no resolved timezone ⇒ process-local" + * — the very fallback #8484 preserves. What moved is the ARITY: the timezone + * now sits between `ext` and the injected clock, so a caller that forgets it + * gets a type error rather than a silently re-timed filename. + * + * These cases cannot, however, tell process-local apart from UTC on a UTC + * runner (CI's default for this package — `packages/rest` is not in the skewed + * -zone job). The clock describe below owns that distinction and controls `TZ` + * itself to make it real. + */ describe('exportContentDisposition', () => { const NOW = new Date(2026, 6, 14, 15, 30, 45); // 2026-07-14 15:30:45 local it('uses the localized label in filename* and the API name as ASCII fallback', () => { - expect(exportContentDisposition('contracts', '合同', 'xlsx', NOW)).toBe( + expect(exportContentDisposition('contracts', '合同', 'xlsx', undefined, NOW)).toBe( `attachment; filename="contracts-20260714-153045.xlsx"; filename*=UTF-8''${encodeURIComponent('合同-20260714-153045.xlsx')}`, ); }); it('falls back to the API name when no label is available', () => { - expect(exportContentDisposition('contracts', undefined, 'csv', NOW)).toBe( + expect(exportContentDisposition('contracts', undefined, 'csv', undefined, NOW)).toBe( `attachment; filename="contracts-20260714-153045.csv"; filename*=UTF-8''contracts-20260714-153045.csv`, ); }); it('sanitizes hostile characters in both names', () => { - const header = exportContentDisposition('a/b', '合 同: v2?', 'csv', NOW); + const header = exportContentDisposition('a/b', '合 同: v2?', 'csv', undefined, NOW); expect(header).toContain('filename="a_b-20260714-153045.csv"'); expect(header).toContain(`filename*=UTF-8''${encodeURIComponent('合 同_ v2-20260714-153045.csv')}`); }); it('percent-encodes RFC 5987 non-attr-chars that encodeURIComponent leaves alone', () => { - const header = exportContentDisposition('obj', "a'b(c)", 'csv', NOW); + const header = exportContentDisposition('obj', "a'b(c)", 'csv', undefined, NOW); expect(header).toContain("filename*=UTF-8''a%27b%28c%29-20260714-153045.csv"); }); it('zero-pads date and time parts', () => { const early = new Date(2026, 0, 5, 9, 8, 7); - expect(exportContentDisposition('obj', undefined, 'json', early)).toContain( + expect(exportContentDisposition('obj', undefined, 'json', undefined, early)).toContain( 'filename="obj-20260105-090807.json"', ); }); }); +/** + * The CLOCK half (#8484): which zone the `-YYYYMMDD-HHMMSS` stamp is read in. + * + * The defect these pin: after #8373 moved the export's CELLS onto the business + * timezone, the filename was the last export surface still on the process + * clock — so a container at `TZ=UTC` serving an Asia/Shanghai tenant downloaded + * `orders-20260731-220000.csv` whose first row read `2026-08-01 06:00:00`. The + * name and the contents disagreed by a day, and at a month boundary by a month. + * + * ⚠️ The fallback here is the OPPOSITE of the cell path's and must stay that + * way: no resolved timezone ⇒ **process-local**, never UTC. Each surface keeps + * its own historical output (the cells were hardcoded UTC; this filename has + * always been process-local), so "UTC is the safe default" would in fact + * re-time the filename of every deployment that sets a host `TZ` but resolves + * no business timezone. The `TZ`-controlled cases below exist so that inversion + * cannot be quietly reversed later while the suite stays green. + */ +describe("exportContentDisposition — the stamp's clock (#8484)", () => { + /** 2026-08-01 02:00 UTC = 10:00 in +08, and still 2026-07-31 22:00 in -04. */ + const INSTANT = new Date('2026-08-01T02:00:00Z'); + + const stampOf = (header: string) => /filename="orders-([\d-]+)\.csv"/.exec(header)?.[1]; + + it('reads the resolved business timezone, not the process clock', () => { + expect(stampOf(exportContentDisposition('orders', undefined, 'csv', 'Asia/Shanghai', INSTANT))) + .toBe('20260801-100000'); + }); + + it('crosses the day AND month boundary with the zone, like the cells do', () => { + // The issue's own scenario, mirrored: west of UTC the same instant is still + // the previous month. A stamp that stayed on the host clock could not move. + expect(stampOf(exportContentDisposition('orders', undefined, 'csv', 'America/New_York', INSTANT))) + .toBe('20260731-220000'); + }); + + it('agrees with the datetime cells the same export streams', () => { + // The whole defect was the two surfaces disagreeing, so pin them together + // rather than trusting each in isolation. + const tz = 'Asia/Shanghai'; + const cell = formatCellValue(INSTANT.toISOString(), { name: 'created', type: 'datetime' }, tz); + expect(cell).toBe('2026-08-01 10:00:00'); + expect(stampOf(exportContentDisposition('orders', undefined, 'csv', tz, INSTANT))) + .toBe('20260801-100000'); + }); + + /** + * The fallback cases own a non-UTC PROCESS zone for their duration. + * + * Without that, every assertion below would be vacuously green on a UTC + * runner — process-local and UTC are the same stamp there, so flipping the + * fallback to UTC would pass. Mutating `process.env.TZ` is what makes the + * distinction observable on any host; Node re-reads it per call, so even a + * `Date` built earlier reports the new zone. + */ + describe('with a skewed process zone', () => { + const ORIGINAL_TZ = process.env.TZ; + + beforeEach(() => { process.env.TZ = 'Asia/Shanghai'; }); + afterEach(() => { + if (ORIGINAL_TZ === undefined) delete process.env.TZ; + else process.env.TZ = ORIGINAL_TZ; + }); + + it('actually took the skewed zone (guards every case below from passing vacuously)', () => { + // The same assert-the-axis-is-real discipline CI applies to its skewed + // -zone job: if the zone silently failed to take, say so here rather than + // letting the fallback pins below succeed for the wrong reason. + expect(INSTANT.getHours()).toBe(10); + }); + + it('⛔ keeps PROCESS-LOCAL when no timezone resolves — does NOT fall back to UTC', () => { + const stamp = stampOf(exportContentDisposition('orders', undefined, 'csv', undefined, INSTANT)); + expect(stamp).toBe('20260801-100000'); // the host's +08 wall clock + expect(stamp).not.toBe('20260801-020000'); // what a UTC fallback would emit + }); + + it('keeps process-local when the resolved zone is not one the platform knows', () => { + // An unresolvable zone is a MISSING zone, not a reason to switch clocks. + expect(stampOf(exportContentDisposition('orders', undefined, 'csv', 'Not/AZone', INSTANT))) + .toBe('20260801-100000'); + }); + + it('honours an explicitly resolved UTC over the host zone', () => { + // `'UTC'` is a RESOLVED business timezone, not a missing one — the one + // case where the stamp is UTC on purpose, and the reason the fallback + // above cannot simply be spelled as "default to UTC". + expect(stampOf(exportContentDisposition('orders', undefined, 'csv', 'UTC', INSTANT))) + .toBe('20260801-020000'); + }); + }); +}); + describe('toArgb', () => { it('expands 3-digit hex to opaque ARGB', () => { expect(toArgb('#3ab')).toBe('FF33AABB'); diff --git a/packages/rest/src/export-format.ts b/packages/rest/src/export-format.ts index 609f14d99b..06f03aec1d 100644 --- a/packages/rest/src/export-format.ts +++ b/packages/rest/src/export-format.ts @@ -17,6 +17,13 @@ * `ExecutionContext` carries — and falls back to UTC when there is none, which * is byte-identical to the pre-#8373 output. A `date` cell is a timezone-naive * calendar day and never reads it (ADR-0053). See {@link formatDate}. + * + * Third contract, same clock, OPPOSITE fallback (#8484): the download + * filename's timestamp reads that same business timezone, so the name a + * browser saves agrees with the rows inside the file. Its no-timezone fallback + * is the PROCESS-LOCAL clock, not UTC — see {@link exportContentDisposition} + * for why the two contracts deliberately differ, and {@link zonedWallClock} + * for where that choice is left to each caller. */ export interface ExportFieldMeta { @@ -60,17 +67,39 @@ export interface ExportFieldMeta { * Non-ASCII labels ride the RFC 5987/6266 `filename*` parameter; the plain * `filename` keeps an ASCII-safe fallback derived from the object API name * for clients that don't understand `filename*`. + * + * **The stamp's clock (#8484).** `timezone` is the request's business timezone + * (`ExecutionContext.timezone`, the platform-default → global → tenant + * cascade) — the SAME value the cells are rendered in. Before #8373 both the + * name and the contents were wrong in different directions; #8373 moved the + * contents onto the business zone and left this the last export surface on the + * host clock, so a container at `TZ=UTC` serving an Asia/Shanghai tenant + * downloaded `orders-20260731-220000.csv` whose first row read + * `2026-08-01 06:00:00`. Reading one clock for both is the whole point. + * + * **⚠️ The no-timezone fallback is PROCESS-LOCAL, not UTC — deliberately the + * opposite of {@link formatDate}'s.** Each fallback preserves ITS OWN surface's + * historical output, and the two surfaces have different histories: the cells + * were hardcoded to UTC, this filename has always used the process clock. UTC + * here would look like the "safe default" and would in fact re-time the + * filename of every deployment that sets a host `TZ` but resolves no business + * timezone — a silent change to a user-visible name, for zero correctness gain. + * A deployment that explicitly resolves `'UTC'` is a resolved zone, not a + * missing one, and does get UTC. */ export function exportContentDisposition( objectName: string, label: string | undefined, ext: string, + timezone?: string, now: Date = new Date(), ): string { const pad = (n: number) => String(n).padStart(2, '0'); - const stamp = - `${now.getFullYear()}${pad(now.getMonth() + 1)}${pad(now.getDate())}` + - `-${pad(now.getHours())}${pad(now.getMinutes())}${pad(now.getSeconds())}`; + const zoned = zonedWallClock(now, timezone); + const stamp = zoned + ? `${zoned.ymd.replace(/-/g, '')}-${zoned.hms.replace(/:/g, '')}` + : `${now.getFullYear()}${pad(now.getMonth() + 1)}${pad(now.getDate())}` + + `-${pad(now.getHours())}${pad(now.getMinutes())}${pad(now.getSeconds())}`; const asciiBase = objectName.replace(/[^A-Za-z0-9_.-]/g, '_') || 'export'; // Keep unicode letters (CJK labels) but drop filesystem-hostile characters. // eslint-disable-next-line no-control-regex @@ -200,40 +229,62 @@ function zonedFormatter(timezone: string): Intl.DateTimeFormat | null { return fmt; } +/** The UTC wall clock of an instant — `YYYY-MM-DD` + `HH:mm:ss`. */ +function utcWallClock(d: Date): { ymd: string; hms: string } { + return { + ymd: `${d.getUTCFullYear()}-${pad2(d.getUTCMonth() + 1)}-${pad2(d.getUTCDate())}`, + hms: `${pad2(d.getUTCHours())}:${pad2(d.getUTCMinutes())}:${pad2(d.getUTCSeconds())}`, + }; +} + /** - * The wall clock an instant shows in `timezone` — `YYYY-MM-DD` + `HH:mm:ss`, - * split so callers can use either half. + * The wall clock an instant shows in `timezone`, or **`null` when there is no + * usable zone** — `timezone` absent, or not a zone this platform knows. * * Reads the calendar components from `Intl.DateTimeFormat().formatToParts()` * so DST transitions come from the platform's tz database rather than * hand-rolled offset arithmetic (the same primitive `@objectstack/core`'s * `calendarPartsInTz` and `@objectstack/spec`'s autonumber date tokens use). * - * Falls back to the UTC wall clock whenever `timezone` is absent, `'UTC'`, or - * not a zone this platform knows — the pre-#8373 behaviour, kept as the - * backward-compatibility contract for deployments that never set one. + * WHY THIS RETURNS `null` INSTEAD OF FALLING BACK: its two callers need + * OPPOSITE fallbacks, and that difference is a contract rather than a detail. + * The cell path ({@link formatDate}) falls back to UTC — its pre-#8373 output; + * the filename stamp ({@link exportContentDisposition}) falls back to the + * process-local clock — its own pre-#8484 output. Each preserves the history of + * the surface it serves. Baking either one in here would silently re-time the + * other surface for every deployment that resolves no business timezone, so the + * choice is left at each call site where it can be read and pinned. + * + * `'UTC'` is a RESOLVED zone, not a missing one, so it yields UTC parts rather + * than `null`: a deployment that configures UTC gets UTC on both surfaces + * whatever the host `TZ` says. + */ +function zonedWallClock(d: Date, timezone?: string): { ymd: string; hms: string } | null { + if (!timezone) return null; + if (timezone === 'UTC') return utcWallClock(d); + const fmt = zonedFormatter(timezone); + if (!fmt) return null; + const parts = fmt.formatToParts(d); + const get = (t: string) => parts.find((p) => p.type === t)?.value; + const y = get('year'); + const mo = get('month'); + const da = get('day'); + const h = get('hour'); + const mi = get('minute'); + const s = get('second'); + if (!(y && mo && da && h && mi && s)) return null; + return { ymd: `${y}-${mo}-${da}`, hms: `${h}:${mi}:${s}` }; +} + +/** + * The wall clock an instant shows in `timezone`, falling back to UTC whenever + * `timezone` is absent, `'UTC'`, or not a zone this platform knows — the + * pre-#8373 behaviour, kept as the backward-compatibility contract for + * deployments that never set one. See {@link zonedWallClock} for why the + * fallback lives here rather than inside it. */ function wallClock(d: Date, timezone?: string): { ymd: string; hms: string } { - if (timezone && timezone !== 'UTC') { - const fmt = zonedFormatter(timezone); - if (fmt) { - const parts = fmt.formatToParts(d); - const get = (t: string) => parts.find((p) => p.type === t)?.value; - const y = get('year'); - const mo = get('month'); - const da = get('day'); - const h = get('hour'); - const mi = get('minute'); - const s = get('second'); - if (y && mo && da && h && mi && s) { - return { ymd: `${y}-${mo}-${da}`, hms: `${h}:${mi}:${s}` }; - } - } - } - return { - ymd: `${d.getUTCFullYear()}-${pad2(d.getUTCMonth() + 1)}-${pad2(d.getUTCDate())}`, - hms: `${pad2(d.getUTCHours())}:${pad2(d.getUTCMinutes())}:${pad2(d.getUTCSeconds())}`, - }; + return zonedWallClock(d, timezone) ?? utcWallClock(d); } function toDate(value: unknown): Date | null { diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 8a1819ce29..4658f8e7f2 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -8177,7 +8177,11 @@ export class RestServer { } else { res.header('Content-Type', 'application/json; charset=utf-8'); } - res.header('Content-Disposition', exportContentDisposition(objectName, objectLabel, format)); + // [#8484] Same `timezone` the cells below render in — the + // filename's stamp and the file's contents must not read + // two different clocks. `undefined` keeps the historical + // process-local stamp (NOT UTC — see the function's doc). + res.header('Content-Disposition', exportContentDisposition(objectName, objectLabel, format, timezone)); res.header('X-Export-Format', format); res.header('X-Export-Limit', String(limit)); // Signal whether select-option colours were applied. Only