diff --git a/public/app/plugins/datasource/elasticsearch/datasource.test.ts b/public/app/plugins/datasource/elasticsearch/datasource.test.ts index 603aaab2b2c..1e9268aac21 100644 --- a/public/app/plugins/datasource/elasticsearch/datasource.test.ts +++ b/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 = { - getAdhocFilters: () => [{ key: '', operator: '=', value: 'a' }], - } as unknown as TemplateSrv; - const { ds } = getTestContext({ templateSrvMock }); + const { ds, templateSrv } = getTestContext(); + jest.mocked(templateSrv.getAdhocFilters).mockReturnValue([{ key: '', operator: '=', value: 'a', condition: '' }]); const query = ds.addAdHocFilters('foo:"bar"'); expect(query).toBe('foo:"bar"'); }); it('should filter out ad hoc filter without value', () => { - const templateSrvMock = { - getAdhocFilters: () => [{ key: 'a', operator: '=', value: '' }], - } as unknown as TemplateSrv; - const { ds } = getTestContext({ templateSrvMock }); + const { ds, templateSrv } = getTestContext(); + jest.mocked(templateSrv.getAdhocFilters).mockReturnValue([{ key: 'a', operator: '=', value: '', condition: '' }]); const query = ds.addAdHocFilters('foo:"bar"'); expect(query).toBe('foo:"bar"'); }); it('should filter out filter ad hoc filter with invalid operator', () => { - const templateSrvMock = { - getAdhocFilters: () => [{ key: 'a', operator: 'A', value: '' }], - } as unknown as TemplateSrv; - const { ds } = getTestContext({ templateSrvMock }); + const { ds, templateSrv } = getTestContext(); + jest.mocked(templateSrv.getAdhocFilters).mockReturnValue([{ key: 'a', operator: 'A', value: '', condition: '' }]); 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 = { - getAdhocFilters: () => [{ key: 'test', operator: '=', value: 'test1' }], - } as unknown as TemplateSrv; - const { ds } = getTestContext({ templateSrvMock }); + let ds: ElasticDatasource, templateSrvMock: TemplateSrv; + beforeEach(() => { + const { ds: datasource, templateSrv } = getTestContext(); + 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 = { - getAdhocFilters: () => [ - { key: 'bar', operator: '=', value: 'baz' }, - { key: 'job', operator: '!=', value: 'grafana' }, - { key: 'service', operator: '=~', value: 'service' }, - { key: 'count', operator: '>', value: '1' }, - ], - } as unknown as TemplateSrv; - const { ds } = getTestContext({ templateSrvMock }); + let ds: ElasticDatasource; + beforeEach(() => { + const { ds: datasource, templateSrv } = getTestContext(); + ds = datasource; + jest.mocked(templateSrv.getAdhocFilters).mockReturnValue([ + { key: 'bar', operator: '=', value: 'baz', condition: '' }, + { key: 'job', operator: '!=', value: 'grafana', condition: '' }, + { key: 'service', operator: '=~', value: 'service', condition: '' }, + { 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"'); diff --git a/public/app/plugins/datasource/elasticsearch/datasource.ts b/public/app/plugins/datasource/elasticsearch/datasource.ts index 7a6e98f4f62..40a8fc5dfc3 100644 --- a/public/app/plugins/datasource/elasticsearch/datasource.ts +++ b/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}"`; diff --git a/public/app/plugins/datasource/elasticsearch/modifyQuery.test.ts b/public/app/plugins/datasource/elasticsearch/modifyQuery.test.ts index 5f785ced562..e64ec36a954 100644 --- a/public/app/plugins/datasource/elasticsearch/modifyQuery.test.ts +++ b/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(query, 'label', 'value')).toBe(''); + expect(removeFilterFromQuery('label:"value"', '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(''); + }); }); diff --git a/public/app/plugins/datasource/elasticsearch/modifyQuery.ts b/public/app/plugins/datasource/elasticsearch/modifyQuery.ts index cc2622f1d5b..0d843525278 100644 --- a/public/app/plugins/datasource/elasticsearch/modifyQuery.ts +++ b/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, '\\:'); +}