From 89fc72f955462b01a35facfe543084056036bc7a Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Mon, 31 Jan 2022 14:12:39 +0100 Subject: [PATCH] prevent leakage of moment timezone (#123694) --- .../response_processors/series/time_shift.js | 42 +++++++------ .../series/time_shift.test.js | 5 ++ .../expressions/time_scale/time_scale.test.ts | 30 ++++++++++ .../expressions/time_scale/time_scale_fn.ts | 60 +++++++++++-------- 4 files changed, 92 insertions(+), 45 deletions(-) diff --git a/src/plugins/vis_types/timeseries/server/lib/vis_data/response_processors/series/time_shift.js b/src/plugins/vis_types/timeseries/server/lib/vis_data/response_processors/series/time_shift.js index 429050fab36ccd..6936ea7e1c5d7d 100644 --- a/src/plugins/vis_types/timeseries/server/lib/vis_data/response_processors/series/time_shift.js +++ b/src/plugins/vis_types/timeseries/server/lib/vis_data/response_processors/series/time_shift.js @@ -27,28 +27,32 @@ export function timeShift( const offsetValue = matches[1]; const offsetUnit = matches[2]; - let defaultTimezone; - if (!panel.ignore_daylight_time) { - // the datemath plugin always parses dates by using the current default moment time zone. - // to use the configured time zone, we are switching just for the bounds calculation. - defaultTimezone = moment().zoneName(); - moment.tz.setDefault(timezone); - } + const defaultTimezone = moment().zoneName(); + try { + if (!panel.ignore_daylight_time) { + // the datemath plugin always parses dates by using the current default moment time zone. + // to use the configured time zone, we are switching just for the bounds calculation. - results.forEach((item) => { - if (startsWith(item.id, series.id)) { - item.data = item.data.map((row) => [ - (panel.ignore_daylight_time ? moment.utc : moment)(row[0]) - .add(offsetValue, offsetUnit) - .valueOf(), - row[1], - ]); + // The code between this call and the reset in the finally block is not allowed to get async, + // otherwise the timezone setting can leak out of this function. + moment.tz.setDefault(timezone); } - }); - if (!panel.ignore_daylight_time) { - // reset default moment timezone - moment.tz.setDefault(defaultTimezone); + results.forEach((item) => { + if (startsWith(item.id, series.id)) { + item.data = item.data.map((row) => [ + (panel.ignore_daylight_time ? moment.utc : moment)(row[0]) + .add(offsetValue, offsetUnit) + .valueOf(), + row[1], + ]); + } + }); + } finally { + if (!panel.ignore_daylight_time) { + // reset default moment timezone + moment.tz.setDefault(defaultTimezone); + } } } } diff --git a/src/plugins/vis_types/timeseries/server/lib/vis_data/response_processors/series/time_shift.test.js b/src/plugins/vis_types/timeseries/server/lib/vis_data/response_processors/series/time_shift.test.js index 7fff2603cf47ab..f3000e5589b6d0 100644 --- a/src/plugins/vis_types/timeseries/server/lib/vis_data/response_processors/series/time_shift.test.js +++ b/src/plugins/vis_types/timeseries/server/lib/vis_data/response_processors/series/time_shift.test.js @@ -128,4 +128,9 @@ describe('timeShift(resp, panel, series)', () => { [dateAfterDST + 1000 * 60 * 60 * 24, 2], ]); }); + + test('processor is sync to avoid timezone setting leakage', () => { + const result = timeShift(resp, panel, series, {})((results) => results)([]); + expect(Array.isArray(result)).toBe(true); + }); }); diff --git a/x-pack/plugins/lens/common/expressions/time_scale/time_scale.test.ts b/x-pack/plugins/lens/common/expressions/time_scale/time_scale.test.ts index d51f2594b42678..9bf647b1942820 100644 --- a/x-pack/plugins/lens/common/expressions/time_scale/time_scale.test.ts +++ b/x-pack/plugins/lens/common/expressions/time_scale/time_scale.test.ts @@ -388,4 +388,34 @@ describe('time_scale', () => { expect(result.rows.map(({ scaledMetric }) => scaledMetric)).toEqual([1, 1, 1, 1, 1]); }); + + it('should be sync except for timezone getter to prevent timezone leakage', async () => { + let resolveTimezonePromise: (value: string | PromiseLike) => void; + const timezonePromise = new Promise((res) => { + resolveTimezonePromise = res; + }); + const timeScaleResolved = jest.fn((x) => x); + const delayedTimeScale = getTimeScale(() => timezonePromise); + const delayedTimeScaleWrapper = functionWrapper(delayedTimeScale); + const result = delayedTimeScaleWrapper( + { + ...emptyTable, + }, + { + ...defaultArgs, + } + ).then(timeScaleResolved) as Promise; + + expect(result instanceof Promise).toBe(true); + // wait a tick + await new Promise((r) => setTimeout(r, 0)); + // time scale is not done yet because it's waiting for the timezone + expect(timeScaleResolved).not.toHaveBeenCalled(); + // resolve timezone + resolveTimezonePromise!('UTC'); + // wait a tick + await new Promise((r) => setTimeout(r, 0)); + // should resolve now without another async dependency + expect(timeScaleResolved).toHaveBeenCalled(); + }); }); diff --git a/x-pack/plugins/lens/common/expressions/time_scale/time_scale_fn.ts b/x-pack/plugins/lens/common/expressions/time_scale/time_scale_fn.ts index 78d2e9896d4c3b..5217d6a7db1676 100644 --- a/x-pack/plugins/lens/common/expressions/time_scale/time_scale_fn.ts +++ b/x-pack/plugins/lens/common/expressions/time_scale/time_scale_fn.ts @@ -9,6 +9,7 @@ import moment from 'moment-timezone'; import { i18n } from '@kbn/i18n'; import { buildResultColumns, + Datatable, ExecutionContext, } from '../../../../../../src/plugins/expressions/common'; import { @@ -76,38 +77,45 @@ export const timeScaleFn = } // the datemath plugin always parses dates by using the current default moment time zone. // to use the configured time zone, we are switching just for the bounds calculation. + + // The code between this call and the reset in the finally block is not allowed to get async, + // otherwise the timezone setting can leak out of this function. const defaultTimezone = moment().zoneName(); - moment.tz.setDefault(timeInfo.timeZone); + let result: Datatable; + try { + moment.tz.setDefault(timeInfo.timeZone); - const timeBounds = timeInfo.timeRange && calculateBounds(timeInfo.timeRange); + const timeBounds = timeInfo.timeRange && calculateBounds(timeInfo.timeRange); - const result = { - ...input, - columns: resultColumns, - rows: input.rows.map((row) => { - const newRow = { ...row }; + result = { + ...input, + columns: resultColumns, + rows: input.rows.map((row) => { + const newRow = { ...row }; - let startOfBucket = moment(row[dateColumnId]); - let endOfBucket = startOfBucket.clone().add(intervalDuration); - if (timeBounds && timeBounds.min) { - startOfBucket = moment.max(startOfBucket, timeBounds.min); - } - if (timeBounds && timeBounds.max) { - endOfBucket = moment.min(endOfBucket, timeBounds.max); - } - const bucketSize = endOfBucket.diff(startOfBucket); - const factor = bucketSize / targetUnitInMs; + let startOfBucket = moment(row[dateColumnId]); + let endOfBucket = startOfBucket.clone().add(intervalDuration); + if (timeBounds && timeBounds.min) { + startOfBucket = moment.max(startOfBucket, timeBounds.min); + } + if (timeBounds && timeBounds.max) { + endOfBucket = moment.min(endOfBucket, timeBounds.max); + } + const bucketSize = endOfBucket.diff(startOfBucket); + const factor = bucketSize / targetUnitInMs; - const currentValue = newRow[inputColumnId]; - if (currentValue != null) { - newRow[outputColumnId] = Number(currentValue) / factor; - } + const currentValue = newRow[inputColumnId]; + if (currentValue != null) { + newRow[outputColumnId] = Number(currentValue) / factor; + } - return newRow; - }), - }; - // reset default moment timezone - moment.tz.setDefault(defaultTimezone); + return newRow; + }), + }; + } finally { + // reset default moment timezone + moment.tz.setDefault(defaultTimezone); + } return result; };