Elasticsearch: escape colons in ad hoc filters (#70611)

* Elasticsearch: escape colons in ad hoc filters

* Escape filter: update regex

* Elasticsearch: escape colons from all filters

public/app/plugins/datasource/elasticsearch/datasource.test.ts

@@ -128,12 +128,12 @@ function getTestContext({
         }
       },
       containsTemplate: jest.fn().mockImplementation((text?: string) => text?.includes('$') ?? false),
-      getAdhocFilters: () => [],
+      getAdhocFilters: jest.fn().mockReturnValue([]),
     } as unknown as TemplateSrv);
 
   const ds = createElasticDatasource(settings, templateSrv);
 
-  return { timeSrv, ds, fetchMock };
+  return { timeSrv, ds, fetchMock, templateSrv };
 }
 
 describe('ElasticDatasource', () => {
@@ -1266,33 +1266,27 @@ describe('modifyQuery', () => {
   });
 });
 
-describe('addAdhocFilters', () => {
+describe('addAdHocFilters', () => {
   describe('with invalid filters', () => {
     it('should filter out ad hoc filter without key', () => {
-      const templateSrvMock = {
+      const { ds, templateSrv } = getTestContext();
-        getAdhocFilters: () => [{ key: '', operator: '=', value: 'a' }],
+      jest.mocked(templateSrv.getAdhocFilters).mockReturnValue([{ key: '', operator: '=', value: 'a', condition: '' }]);
-      } as unknown as TemplateSrv;
-      const { ds } = getTestContext({ templateSrvMock });
 
       const query = ds.addAdHocFilters('foo:"bar"');
       expect(query).toBe('foo:"bar"');
     });
 
     it('should filter out ad hoc filter without value', () => {
-      const templateSrvMock = {
+      const { ds, templateSrv } = getTestContext();
-        getAdhocFilters: () => [{ key: 'a', operator: '=', value: '' }],
+      jest.mocked(templateSrv.getAdhocFilters).mockReturnValue([{ key: 'a', operator: '=', value: '', condition: '' }]);
-      } as unknown as TemplateSrv;
-      const { ds } = getTestContext({ templateSrvMock });
 
       const query = ds.addAdHocFilters('foo:"bar"');
       expect(query).toBe('foo:"bar"');
     });
 
     it('should filter out filter ad hoc filter with invalid operator', () => {
-      const templateSrvMock = {
+      const { ds, templateSrv } = getTestContext();
-        getAdhocFilters: () => [{ key: 'a', operator: 'A', value: '' }],
+      jest.mocked(templateSrv.getAdhocFilters).mockReturnValue([{ key: 'a', operator: 'A', value: '', condition: '' }]);
-      } as unknown as TemplateSrv;
-      const { ds } = getTestContext({ templateSrvMock });
 
       const query = ds.addAdHocFilters('foo:"bar"');
       expect(query).toBe('foo:"bar"');
@@ -1300,10 +1294,15 @@ describe('addAdhocFilters', () => {
   });
 
   describe('with 1 ad hoc filter', () => {
-    const templateSrvMock = {
+    let ds: ElasticDatasource, templateSrvMock: TemplateSrv;
-      getAdhocFilters: () => [{ key: 'test', operator: '=', value: 'test1' }],
+    beforeEach(() => {
-    } as unknown as TemplateSrv;
+      const { ds: datasource, templateSrv } = getTestContext();
-    const { ds } = getTestContext({ templateSrvMock });
+      ds = datasource;
+      templateSrvMock = templateSrv;
+      jest
+        .mocked(templateSrv.getAdhocFilters)
+        .mockReturnValue([{ key: 'test', operator: '=', value: 'test1', condition: '' }]);
+    });
 
     it('should correctly add 1 ad hoc filter when query is not empty', () => {
       const query = ds.addAdHocFilters('foo:"bar"');
@@ -1314,18 +1313,29 @@ describe('addAdhocFilters', () => {
       const query = ds.addAdHocFilters('');
       expect(query).toBe('test:"test1"');
     });
+
+    it('should escape characters in filter keys', () => {
+      jest
+        .mocked(templateSrvMock.getAdhocFilters)
+        .mockReturnValue([{ key: 'field:name', operator: '=', value: 'field:value', condition: '' }]);
+
+      const query = ds.addAdHocFilters('');
+      expect(query).toBe('field\\:name:"field:value"');
+    });
   });
 
   describe('with multiple ad hoc filters', () => {
-    const templateSrvMock = {
+    let ds: ElasticDatasource;
-      getAdhocFilters: () => [
+    beforeEach(() => {
-        { key: 'bar', operator: '=', value: 'baz' },
+      const { ds: datasource, templateSrv } = getTestContext();
-        { key: 'job', operator: '!=', value: 'grafana' },
+      ds = datasource;
-        { key: 'service', operator: '=~', value: 'service' },
+      jest.mocked(templateSrv.getAdhocFilters).mockReturnValue([
-        { key: 'count', operator: '>', value: '1' },
+        { key: 'bar', operator: '=', value: 'baz', condition: '' },
-      ],
+        { key: 'job', operator: '!=', value: 'grafana', condition: '' },
-    } as unknown as TemplateSrv;
+        { key: 'service', operator: '=~', value: 'service', condition: '' },
-    const { ds } = getTestContext({ templateSrvMock });
+        { key: 'count', operator: '>', value: '1', condition: '' },
+      ]);
+    });
 
     it('should correctly add ad hoc filters when query is not empty', () => {
       const query = ds.addAdHocFilters('foo:"bar" AND test:"test1"');

public/app/plugins/datasource/elasticsearch/datasource.ts

@@ -52,7 +52,7 @@ import {
 } from './components/QueryEditor/MetricAggregationsEditor/aggregations';
 import { metricAggregationConfig } from './components/QueryEditor/MetricAggregationsEditor/utils';
 import { isMetricAggregationWithMeta } from './guards';
-import { addFilterToQuery, queryHasFilter, removeFilterFromQuery } from './modifyQuery';
+import { addFilterToQuery, escapeFilter, queryHasFilter, removeFilterFromQuery } from './modifyQuery';
 import { trackAnnotationQuery, trackQuery } from './tracking';
 import {
   Logs,
@@ -938,10 +938,15 @@ export class ElasticDatasource
       return query;
     }
     const esFilters = adhocFilters.map((filter) => {
-      const { key, operator, value } = filter;
+      let { key, operator, value } = filter;
       if (!key || !value) {
         return;
       }
+      /**
+       * Keys and values in ad hoc filters may contain characters such as
+       * colons, which needs to be escaped.
+       */
+      key = escapeFilter(key);
       switch (operator) {
         case '=':
           return `${key}:"${value}"`;

public/app/plugins/datasource/elasticsearch/modifyQuery.test.ts

@@ -30,6 +30,10 @@ describe('queryHasFilter', () => {
     expect(queryHasFilter('label:"value"', 'label', 'otherValue', '-')).toBe(false);
     expect(queryHasFilter('label:"value"', 'label', 'value', '-')).toBe(false);
   });
+  it('should support filters containing colons', () => {
+    expect(queryHasFilter('label\\:name:"value"', 'label:name', 'value')).toBe(true);
+    expect(queryHasFilter('-label\\:name:"value"', 'label:name', 'value', '-')).toBe(true);
+  });
 });
 
 describe('addFilterToQuery', () => {
@@ -45,12 +49,14 @@ describe('addFilterToQuery', () => {
   it('should add a negative filter to the query with other filters', () => {
     expect(addFilterToQuery('label2:"value2"', 'label', 'value', '-')).toBe('label2:"value2" AND -label:"value"');
   });
+  it('should support filters with colons', () => {
+    expect(addFilterToQuery('', 'label:name', 'value')).toBe('label\\:name:"value"');
+  });
 });
 
 describe('removeFilterFromQuery', () => {
   it('should remove filter from query', () => {
-    const query = 'label:"value"';
+    expect(removeFilterFromQuery('label:"value"', 'label', 'value')).toBe('');
-    expect(removeFilterFromQuery(query, 'label', 'value')).toBe('');
   });
   it('should remove filter from query with other filters', () => {
     expect(removeFilterFromQuery('label:"value" AND label2:"value2"', 'label', 'value')).toBe('label2:"value2"');
@@ -71,4 +77,7 @@ describe('removeFilterFromQuery', () => {
       '-label : "value" OR label2:"value2"'
     );
   });
+  it('should support filters with colons', () => {
+    expect(removeFilterFromQuery('label\\:name:"value"', 'label:name', 'value')).toBe('');
+  });
 });

public/app/plugins/datasource/elasticsearch/modifyQuery.ts

@@ -6,6 +6,7 @@ type ModifierType = '' | '-';
  * Checks for the presence of a given label:"value" filter in the query.
  */
 export function queryHasFilter(query: string, key: string, value: string, modifier: ModifierType = ''): boolean {
+  key = escapeFilter(key);
   const regex = getFilterRegex(key, value);
   const matches = query.matchAll(regex);
   for (const match of matches) {
@@ -27,19 +28,21 @@ export function addFilterToQuery(query: string, key: string, value: string, modi
     return query;
   }
 
+  key = escapeFilter(key);
   const filter = `${modifier}${key}:"${value}"`;
 
   return query === '' ? filter : `${query} AND ${filter}`;
 }
 
 function getFilterRegex(key: string, value: string) {
-  return new RegExp(`[-]{0,1}\\s*${key}\\s*:\\s*["']{0,1}${escapeRegex(value)}["']{0,1}`, 'ig');
+  return new RegExp(`[-]{0,1}\\s*${escapeRegex(key)}\\s*:\\s*["']{0,1}${escapeRegex(value)}["']{0,1}`, 'ig');
 }
 
 /**
  * Removes a label:"value" expression from the query.
  */
 export function removeFilterFromQuery(query: string, key: string, value: string, modifier: ModifierType = ''): string {
+  key = escapeFilter(key);
   const regex = getFilterRegex(key, value);
   const matches = query.matchAll(regex);
   const opRegex = new RegExp(`\\s+(?:AND|OR)\\s*$|^\\s*(?:AND|OR)\\s+`, 'ig');
@@ -55,3 +58,11 @@ export function removeFilterFromQuery(query: string, key: string, value: string,
   query = query.replace(/OR\s+AND/gi, 'AND');
   return query;
 }
+
+/**
+ * Filters can possibly contain colons, which are used as a separator in the query.
+ * Use this function to escape filter keys.
+ */
+export function escapeFilter(value: string) {
+  return value.replace(/:/g, '\\:');
+}