Skip to content

Commit 6141a98

Browse files
ovrovr
committed
fix: apply timezone normalization across all API entry points
1 parent 47516ce commit 6141a98

8 files changed

Lines changed: 178 additions & 68 deletions

File tree

‎packages/cubejs-api-gateway/src/gateway.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ import {
9292
normalizeQueryCancelPreAggregations,
9393
normalizeQueryPreAggregationPreview,
9494
normalizeQueryPreAggregations,
95+
normalizeTimezone,
9596
parseInputMemberExpression,
9697
preAggsJobsRequestSchema,
9798
remapToQueryAdapterFormat,
@@ -480,7 +481,9 @@ class ApiGateway {
480481
try {
481482
await this.assertApiScope('data', req.context?.securityContext);
482483

483-
await this.sqlServer.execSql(req.body.query, res, req.context?.securityContext, req.body.cache, req.body.timezone, req.body.throwContinueWait, req.context?.requestId);
484+
const timezone = normalizeTimezone(req.body.timezone);
485+
486+
await this.sqlServer.execSql(req.body.query, res, req.context?.securityContext, req.body.cache, timezone, req.body.throwContinueWait, req.context?.requestId);
484487
} catch (e: any) {
485488
// Quickfix for https://github.com/cube-js/cube/issues/10450,
486489
// Right now, it's too complicated to fix the issue correctly, because
@@ -985,7 +988,7 @@ class ApiGateway {
985988
throw new UserError('No job description provided');
986989
}
987990

988-
const { error } = preAggsJobsRequestSchema.validate(query);
991+
const { error, value } = preAggsJobsRequestSchema.validate(query);
989992
if (error) {
990993
throw new UserError(`Invalid Job query format: ${error.message || error.toString()}`);
991994
}
@@ -994,7 +997,8 @@ class ApiGateway {
994997
case 'post':
995998
result = await this.preAggregationsJobsPOST(
996999
context,
997-
<PreAggsSelector>query.selector
1000+
// Use the selector normalized by the schema (canonical IANA timezones).
1001+
<PreAggsSelector>value.selector
9981002
);
9991003
if (result.length === 0) {
10001004
throw new UserError(

‎packages/cubejs-api-gateway/src/query.js‎

Lines changed: 42 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -56,16 +56,43 @@ const evaluatedPatchMeasureExpression = parsedPatchMeasureExpression.keys({
5656
});
5757

5858
const id = Joi.string().regex(/^[a-zA-Z0-9_]+\.[a-zA-Z0-9_]+$/);
59-
const timezoneSchema = Joi.string().custom((value, helpers) => {
59+
const canonicalTimezone = (value) => {
6060
const zone = moment.tz.zone(value);
61-
if (!zone) {
62-
return helpers.error('any.invalid');
61+
if (zone) {
62+
// Normalize to the canonical IANA name.
63+
return zone.name;
64+
}
65+
66+
return null;
67+
};
68+
69+
const timezoneSchema = Joi.string().custom((value, helpers) => {
70+
const name = canonicalTimezone(value);
71+
if (!name) {
72+
return helpers.message(`{{#label}} must be a valid IANA time zone, got "${value}"`);
6373
}
6474

65-
// Normalize to the canonical IANA name (case-insensitively).
66-
return zone.name;
75+
// Normalize to the canonical IANA name.
76+
return name;
6777
}, 'timezone');
6878

79+
/**
80+
* @param {string|undefined} value
81+
* @returns {string|undefined}
82+
*/
83+
export const normalizeTimezone = (value) => {
84+
if (!value) {
85+
return value;
86+
}
87+
88+
const name = canonicalTimezone(value);
89+
if (!name) {
90+
throw new UserError(`timezone must be a valid IANA time zone, got "${value}"`);
91+
}
92+
93+
return name;
94+
};
95+
6996
// It might be member name, td+granularity or member expression
7097
const idOrMemberExpressionName = Joi.string().regex(/^[a-zA-Z0-9_]+\.[a-zA-Z0-9_]+$|^[a-zA-Z0-9_]+$|^[a-zA-Z0-9_]+\.[a-zA-Z0-9_]+\.[a-zA-Z0-9_]+$/);
7198
const dimensionWithTime = Joi.string().regex(/^[a-zA-Z0-9_]+\.[a-zA-Z0-9_]+(\.[a-zA-Z0-9_]+)?$/);
@@ -426,7 +453,7 @@ function normalizeQueryCacheMode(query, cacheMode) {
426453
const normalizeQuery = (query, persistent, cacheMode) => {
427454
query = normalizeQueryCacheMode(query, cacheMode);
428455
query.timezone = query.timezone || getEnv('defaultTimezone');
429-
const { error } = querySchema.validate(query);
456+
const { error, value } = querySchema.validate(query);
430457
if (error) {
431458
throw new UserError(`Invalid query format: ${error.message || error.toString()}`);
432459
}
@@ -444,10 +471,9 @@ const normalizeQuery = (query, persistent, cacheMode) => {
444471
dimension: d.split('.').slice(0, 2).join('.'),
445472
granularity: d.split('.')[2]
446473
}));
447-
// query.timezone is already validated as a known zone above; normalize it to the
448-
// canonical IANA name (moment matches zones case-insensitively).
449-
const rawTimezone = query.timezone || 'UTC';
450-
const timezone = moment.tz.zone(rawTimezone)?.name || rawTimezone;
474+
// Use the timezone normalized by the schema (canonical IANA name); the raw request
475+
// may carry a different casing.
476+
const timezone = value.timezone || 'UTC';
451477

452478
const def = getEnv('dbQueryDefaultLimit') <= getEnv('dbQueryLimit')
453479
? getEnv('dbQueryDefaultLimit')
@@ -522,14 +548,15 @@ const queryPreAggregationsSchema = Joi.object().keys({
522548
});
523549

524550
const normalizeQueryPreAggregations = (query, defaultValues) => {
525-
const { error } = queryPreAggregationsSchema.validate(query);
551+
const { error, value } = queryPreAggregationsSchema.validate(query);
526552
if (error) {
527553
throw new UserError(`Invalid query format: ${error.message || error.toString()}`);
528554
}
529555

556+
// Use timezones normalized by the schema (canonical IANA names).
530557
return {
531558
metadata: query.metadata,
532-
timezones: query.timezones || (query.timezone && [query.timezone]) || defaultValues?.timezones || ['UTC'],
559+
timezones: value.timezones || (value.timezone && [value.timezone]) || defaultValues?.timezones || ['UTC'],
533560
preAggregations: query.preAggregations,
534561
expand: query.expand
535562
};
@@ -549,12 +576,13 @@ const queryPreAggregationPreviewSchema = Joi.object().keys({
549576
});
550577

551578
const normalizeQueryPreAggregationPreview = (query) => {
552-
const { error } = queryPreAggregationPreviewSchema.validate(query);
579+
const { error, value } = queryPreAggregationPreviewSchema.validate(query);
553580
if (error) {
554581
throw new UserError(`Invalid query format: ${error.message || error.toString()}`);
555582
}
556583

557-
return query;
584+
// Use the timezone normalized by the schema (canonical IANA name).
585+
return { ...query, timezone: value.timezone };
558586
};
559587

560588
const queryCancelPreAggregationPreviewSchema = Joi.object().keys({

‎packages/cubejs-api-gateway/test/normalize-query.test.ts‎

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
// eslint-disable-next-line import/no-extraneous-dependencies
2-
import { normalizeQuery } from '../src/query';
2+
import {
3+
normalizeQuery,
4+
normalizeQueryPreAggregations,
5+
normalizeQueryPreAggregationPreview,
6+
normalizeTimezone,
7+
} from '../src/query';
38

49
const baseQuery = {
510
measures: ['Foo.count'],
@@ -42,7 +47,61 @@ describe('timezone validation', () => {
4247
test.each([
4348
'Not/AZone',
4449
'+05:00',
45-
])('rejects invalid/injection timezone %j', (tz) => {
50+
'foo/bar',
51+
])('rejects invalid timezone %j', (tz) => {
4652
expect(() => normalizeQuery({ ...baseQuery, timezone: tz }, false)).toThrow(/Invalid query format/);
4753
});
4854
});
55+
56+
describe('normalizeQueryPreAggregations timezone handling', () => {
57+
test('normalizes timezone to canonical IANA name', () => {
58+
const result = normalizeQueryPreAggregations({ timezone: 'america/new_york' }, undefined);
59+
expect(result.timezones).toEqual(['America/New_York']);
60+
});
61+
62+
test('normalizes timezones array to canonical IANA names', () => {
63+
const result = normalizeQueryPreAggregations({ timezones: ['utc', 'europe/berlin'] }, undefined);
64+
expect(result.timezones).toEqual(['UTC', 'Europe/Berlin']);
65+
});
66+
67+
test('rejects invalid timezone', () => {
68+
expect(() => normalizeQueryPreAggregations({ timezones: ['Not/AZone'] }, undefined)).toThrow(/Invalid query format/);
69+
});
70+
});
71+
72+
describe('normalizeQueryPreAggregationPreview timezone handling', () => {
73+
const previewQuery = {
74+
preAggregationId: 'cube.preAgg',
75+
versionEntry: { content_version: 'a', structure_version: 'b' },
76+
};
77+
78+
test('normalizes timezone to canonical IANA name', () => {
79+
const result = normalizeQueryPreAggregationPreview({ ...previewQuery, timezone: 'america/new_york' });
80+
expect(result.timezone).toBe('America/New_York');
81+
});
82+
83+
test('rejects invalid timezone', () => {
84+
expect(() => normalizeQueryPreAggregationPreview({ ...previewQuery, timezone: 'Not/AZone' })).toThrow(/Invalid query format/);
85+
});
86+
});
87+
88+
describe('normalizeTimezone helper', () => {
89+
test.each([
90+
['america/new_york', 'America/New_York'],
91+
['UTC', 'UTC'],
92+
['uTc', 'UTC'],
93+
])('normalizes %j -> %j', (tz, expected) => {
94+
expect(normalizeTimezone(tz)).toBe(expected);
95+
});
96+
97+
test.each([undefined, null, ''])('passes through empty value %j', (tz) => {
98+
expect(normalizeTimezone(tz as any)).toBe(tz);
99+
});
100+
101+
test.each([
102+
'Not/AZone',
103+
'foo/bar',
104+
])('throws on invalid timezone %j', (tz) => {
105+
expect(() => normalizeTimezone(tz)).toThrow(/valid IANA time zone/);
106+
});
107+
});

‎packages/cubejs-schema-compiler/src/adapter/BaseQuery.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ export class BaseQuery {
299299
this.timezone = this.options.timezone;
300300

301301
if (this.timezone && !moment.tz.zone(this.timezone)) {
302-
throw new UserError(`Incorrect timezone: ${this.timezone}`);
302+
throw new UserError(`Incorrect timezone ${this.timezone}`);
303303
}
304304

305305
this.rowLimit = this.options.rowLimit;

‎packages/cubejs-schema-compiler/test/unit/base-query.test.ts‎

Lines changed: 39 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -124,55 +124,47 @@ describe('SQL Generation', () => {
124124
expect(queryAndParams[0]).toContain('ORDER BY 2');
125125
});
126126

127-
it('validate timezone', async () => {
128-
await compilers.compiler.compile();
127+
const buildTimezoneQuery = (timezone: string) => new PostgresQuery(compilers, {
128+
measures: ['cards.count'],
129+
timeDimensions: [
130+
{
131+
dimension: 'cards.createdAt',
132+
granularity: 'day',
133+
dateRange: ['2021-01-01', '2021-01-02']
134+
}
135+
],
136+
timezone,
137+
filters: [],
138+
});
129139

130-
const maliciousTimezones = [
131-
'Not/AZone',
132-
'+05:00',
133-
'+05',
134-
'05'
135-
];
136-
137-
for (const timezone of maliciousTimezones) {
138-
expect(() => new PostgresQuery(compilers, {
139-
measures: ['cards.count'],
140-
timeDimensions: [
141-
{
142-
dimension: 'cards.createdAt',
143-
granularity: 'day',
144-
dateRange: ['2021-01-01', '2021-01-02']
145-
}
146-
],
147-
timezone,
148-
filters: [],
149-
})).toThrow(UserError);
150-
}
140+
it.each([
141+
'Not/AZone',
142+
'+05:00',
143+
'+05',
144+
'05',
145+
'foo/bar',
146+
])('rejects invalid timezone %j', async (timezone) => {
147+
await compilers.compiler.compile();
148+
expect(() => buildTimezoneQuery(timezone)).toThrow(UserError);
149+
});
150+
151+
// Valid IANA zones are accepted regardless of case (normalization to the
152+
// canonical name happens at the API gateway input layer, not in BaseQuery).
153+
it.each([
154+
'America/New_York',
155+
'america/new_york',
156+
'AMERICA/NEW_YORK',
157+
'utc',
158+
'uTc',
159+
])('accepts valid timezone regardless of case: %s', async (timezone) => {
160+
await compilers.compiler.compile();
161+
expect(() => buildTimezoneQuery(timezone)).not.toThrow();
162+
});
151163

152-
// Valid IANA zones are accepted regardless of case (normalization to the
153-
// canonical name happens at the API gateway input layer).
154-
const validTimezones = [
155-
'America/New_York',
156-
'america/new_york',
157-
'AMERICA/NEW_YORK',
158-
'utc',
159-
'uTc',
160-
];
161-
162-
for (const timezone of validTimezones) {
163-
expect(() => new PostgresQuery(compilers, {
164-
measures: ['cards.count'],
165-
timeDimensions: [
166-
{
167-
dimension: 'cards.createdAt',
168-
granularity: 'day',
169-
dateRange: ['2021-01-01', '2021-01-02']
170-
}
171-
],
172-
timezone,
173-
filters: [],
174-
})).not.toThrow();
175-
}
164+
it('renders a canonical timezone into SQL', async () => {
165+
await compilers.compiler.compile();
166+
const [sql] = buildTimezoneQuery('America/New_York').buildSqlAndParams();
167+
expect(sql).toContain("AT TIME ZONE 'America/New_York'");
176168
});
177169

178170
it('Simple query - complex measure', async () => {

‎packages/cubejs-server-core/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
"lodash.clonedeep": "^4.5.0",
5252
"lru-cache": "^11.1.0",
5353
"moment": "^2.29.1",
54+
"moment-timezone": "^0.5.46",
5455
"node-fetch": "^2.6.0",
5556
"p-limit": "^3.1.0",
5657
"promise-timeout": "^1.3.0",

‎packages/cubejs-server-core/src/core/optionsValidate.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,16 @@
11
import Joi from 'joi';
2+
import moment from 'moment-timezone';
23
import DriverDependencies from './DriverDependencies';
34

5+
// Reject anything that is not a known IANA timezone name (matched case-insensitively),
6+
// so a misconfigured zone fails fast at startup instead of deep in the refresh scheduler.
7+
const timezoneSchema = Joi.string().custom((value, helpers) => {
8+
if (!moment.tz.zone(value)) {
9+
return helpers.message({ custom: `{{#label}} must be a valid IANA time zone name, got "${value}"` });
10+
}
11+
return value;
12+
}, 'timezone');
13+
414
const schemaQueueOptions = Joi.object().strict(true).keys({
515
concurrency: Joi.number().min(1).integer(),
616
continueWaitTimeout: Joi.number().min(0).max(90).integer(),
@@ -96,7 +106,7 @@ const schemaOptions = Joi.object().keys({
96106
Joi.number().min(0).integer()
97107
),
98108
scheduledRefreshTimeZones: Joi.alternatives().try(
99-
Joi.array().items(Joi.string()),
109+
Joi.array().items(timezoneSchema),
100110
Joi.func()
101111
),
102112
scheduledRefreshContexts: Joi.func(),

‎packages/cubejs-server-core/test/unit/optionsValidate.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import optionsValidate from '../../src/core/optionsValidate';
2+
3+
describe('optionsValidate scheduledRefreshTimeZones', () => {
4+
test('accepts valid IANA timezones', () => {
5+
expect(() => optionsValidate({ scheduledRefreshTimeZones: ['UTC', 'America/New_York'] })).not.toThrow();
6+
});
7+
8+
test('accepts a function', () => {
9+
expect(() => optionsValidate({ scheduledRefreshTimeZones: async () => ['UTC'] })).not.toThrow();
10+
});
11+
12+
test('rejects an invalid timezone with a descriptive message', () => {
13+
expect(() => optionsValidate({ scheduledRefreshTimeZones: ['Not/AZone'] }))
14+
.toThrow(/valid IANA time zone name/);
15+
});
16+
});

0 commit comments

Comments
 (0)