prevent leakage of moment timezone (#123694)

elastic · Jan 31, 2022 · 89fc72f · 89fc72f
1 parent 920b4fc
commit 89fc72f
Show file tree

Hide file tree

Showing 4 changed files with 92 additions and 45 deletions.
diff --git a/...plugins/vis_types/timeseries/server/lib/vis_data/response_processors/series/time_shift.js b/...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/...ns/vis_types/timeseries/server/lib/vis_data/response_processors/series/time_shift.test.js b/...ns/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
@@ -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<string>) => void;
+    const timezonePromise = new Promise<string>((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<Datatable>;
+
+    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
@@ -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;
   };