Clean up vega event handler code (#133557)

* better fallback

* fix

* Fixes normalizeString failure when index is undefined

* Fix typo causing normalizeDate to not work properly

* Adds recursive function to check for the toJSON property on a nested object

* Apply PR comments

* Check if object exists

* Move check on the top

* Some refactoring

* Check object for function property

* Address PR comment

* Address Pr comments

* Improve the NaN check

* Address more PR comments

* Add some unit tests

* Update src/plugins/vis_types/vega/public/vega_view/utils.ts

Co-authored-by: Aleh Zasypkin <aleh.zasypkin@gmail.com>

Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>
Co-authored-by: Stratoula Kalafateli <stratoula1@gmail.com>
Co-authored-by: Aleh Zasypkin <aleh.zasypkin@gmail.com>

src/plugins/vis_types/vega/public/vega_view/utils.test.ts

@@ -0,0 +1,70 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { normalizeString, normalizeObject, normalizeDate } from './utils';
+
+describe('normalizeString', () => {
+  test('should return undefined for non string input', async () => {
+    expect(normalizeString({})).toBe(undefined);
+    expect(normalizeString(12344)).toBe(undefined);
+    expect(normalizeString(null)).toBe(undefined);
+  });
+
+  test('should return the string for string input', async () => {
+    expect(normalizeString('logstash')).toBe('logstash');
+  });
+});
+
+describe('normalizeDate', () => {
+  test('should return timestamp if timestamp is given', async () => {
+    expect(normalizeDate(1654702414)).toBe(1654702414);
+  });
+
+  test('should return null if NaN is given', async () => {
+    expect(normalizeDate(NaN)).toBe(null);
+  });
+
+  test('should return date if a date object is given', async () => {
+    const date = Date.now();
+    expect(normalizeDate(date)).toBe(date);
+  });
+
+  test('should return undefined for a string', async () => {
+    expect(normalizeDate('test')).toBe('test');
+  });
+
+  test('should return the object if object is given', async () => {
+    expect(normalizeDate({ test: 'test' })).toStrictEqual({ test: 'test' });
+  });
+});
+
+describe('normalizeObject', () => {
+  test('should throw if a function is given as the object property', async () => {
+    expect(() => {
+      normalizeObject({ toJSON: () => alert('gotcha') });
+    }).toThrow('a function cannot be used as a property name');
+  });
+
+  test('should throw if a function is given on a nested object', async () => {
+    expect(() => {
+      normalizeObject({ test: { toJSON: () => alert('gotcha') } });
+    }).toThrow('a function cannot be used as a property name');
+  });
+
+  test('should return null for null', async () => {
+    expect(normalizeObject(null)).toBe(null);
+  });
+
+  test('should return null for undefined', async () => {
+    expect(normalizeObject(undefined)).toBe(null);
+  });
+
+  test('should return the object', async () => {
+    expect(normalizeObject({ test: 'test' })).toStrictEqual({ test: 'test' });
+  });
+});

src/plugins/vis_types/vega/public/vega_view/utils.ts

@@ -0,0 +1,58 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+import { ensureNoUnsafeProperties } from '@kbn/std';
+
+export function normalizeDate(date: unknown) {
+  if (typeof date === 'number') {
+    return !isNaN(date) ? date : null;
+  } else if (date instanceof Date) {
+    return date;
+  } else {
+    return normalizeObject(date);
+  }
+}
+
+/*
+Recursive function to check a nested object for a function property
+This function should run before JSON.stringify to ensure that functions such as toJSON
+are not invoked. We dont use the replacer function as it doesnt catch the toJSON function
+*/
+export function checkObjectForFunctionProperty(object: unknown): boolean {
+  if (object === null || object === undefined) {
+    return false;
+  }
+  if (typeof object === 'function') {
+    return true;
+  }
+  if (object && typeof object === 'object') {
+    return Object.values(object).some(
+      (value) => typeof value === 'function' || checkObjectForFunctionProperty(value)
+    );
+  }
+
+  return false;
+}
+/*
+  We want to be strict here when an object is passed to a Vega function
+  - NaN (will be converted to null)
+  - undefined (key will be removed)
+  - Date (will be replaced by its toString value)
+  - will throw an error when a function is found
+  */
+export function normalizeObject(object: unknown) {
+  if (checkObjectForFunctionProperty(object)) {
+    throw new Error('a function cannot be used as a property name');
+  }
+  const normalizedObject = object ? JSON.parse(JSON.stringify(object)) : null;
+  ensureNoUnsafeProperties(normalizedObject);
+  return normalizedObject;
+}
+
+export function normalizeString(string: unknown) {
+  return typeof string === 'string' ? string : undefined;
+}

src/plugins/vis_types/vega/public/vega_view/vega_base_view.js

@@ -20,6 +20,7 @@ import { TooltipHandler } from './vega_tooltip';
 
 import { getEnableExternalUrls, getDataViews } from '../services';
 import { extractIndexPatternsFromSpec } from '../lib/extract_index_pattern';
+import { normalizeDate, normalizeString, normalizeObject } from './utils';
 
 scheme('elastic', euiPaletteColorBlind());
 
@@ -350,8 +351,11 @@ export class VegaBaseView {
    * @param {string} Elastic Query DSL's Custom label for kibanaAddFilter, as used in '+ Add Filter'
    */
   async addFilterHandler(query, index, alias) {
-    const indexId = await this.findIndex(index);
-    const filter = buildQueryFilter(query, indexId, alias);
+    const normalizedQuery = normalizeObject(query);
+    const normalizedIndex = normalizeString(index);
+    const normalizedAlias = normalizeString(alias);
+    const indexId = await this.findIndex(normalizedIndex);
+    const filter = buildQueryFilter(normalizedQuery, indexId, normalizedAlias);
 
     this._fireEvent({ name: 'applyFilter', data: { filters: [filter] } });
   }
@@ -361,8 +365,10 @@ export class VegaBaseView {
    * @param {string} [index] as defined in Kibana, or default if missing
    */
   async removeFilterHandler(query, index) {
-    const indexId = await this.findIndex(index);
-    const filterToRemove = buildQueryFilter(query, indexId);
+    const normalizedQuery = normalizeObject(query);
+    const normalizedIndex = normalizeString(index);
+    const indexId = await this.findIndex(normalizedIndex);
+    const filterToRemove = buildQueryFilter(normalizedQuery, indexId);
 
     const currentFilters = this._filterManager.getFilters();
     const existingFilter = currentFilters.find((filter) => compareFilters(filter, filterToRemove));
@@ -386,7 +392,9 @@ export class VegaBaseView {
    * @param {number|string|Date} end
    */
   setTimeFilterHandler(start, end) {
-    const { from, to, mode } = VegaBaseView._parseTimeRange(start, end);
+    const normalizedStart = normalizeDate(start);
+    const normalizedEnd = normalizeDate(end);
+    const { from, to, mode } = VegaBaseView._parseTimeRange(normalizedStart, normalizedEnd);
 
     this._fireEvent({
       name: 'applyFilter',