From 8ef961bba5e081ff1e43436c84a0be7846dd79b2 Mon Sep 17 00:00:00 2001
From: Charlie Briggs <charliebriggs@gmail.com>
Date: Tue, 4 Jun 2019 12:07:39 +0100
Subject: [PATCH] Prometheus: Correctly escape '|' literals in interpolated
 PromQL variables (#16932)

This escape is currently not happening which breaks multi-value queries with
'|' characters in the variable value.

Added tests for 'interpolateQueryExpr' which should possibly be private, but is difficult to
test due to TemplateSrv calling it.

Closes #16840
---
 .../datasource/prometheus/datasource.ts       |  2 +-
 .../prometheus/specs/datasource.test.ts       | 53 ++++++++++++++++++-
 2 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/public/app/plugins/datasource/prometheus/datasource.ts b/public/app/plugins/datasource/prometheus/datasource.ts
index 6a0137ecfcb..ee8e8dfc48f 100644
--- a/public/app/plugins/datasource/prometheus/datasource.ts
+++ b/public/app/plugins/datasource/prometheus/datasource.ts
@@ -701,7 +701,7 @@ export function prometheusRegularEscape(value) {
 
 export function prometheusSpecialRegexEscape(value) {
   if (typeof value === 'string') {
-    return prometheusRegularEscape(value.replace(/\\/g, '\\\\\\\\').replace(/[$^*{}\[\]+?.()]/g, '\\\\$&'));
+    return prometheusRegularEscape(value.replace(/\\/g, '\\\\\\\\').replace(/[$^*{}\[\]+?.()|]/g, '\\\\$&'));
   }
   return value;
 }
diff --git a/public/app/plugins/datasource/prometheus/specs/datasource.test.ts b/public/app/plugins/datasource/prometheus/specs/datasource.test.ts
index 275671e0283..77da9948122 100644
--- a/public/app/plugins/datasource/prometheus/specs/datasource.test.ts
+++ b/public/app/plugins/datasource/prometheus/specs/datasource.test.ts
@@ -12,6 +12,7 @@ import { DataSourceInstanceSettings } from '@grafana/ui';
 import { PromOptions } from '../types';
 import { TemplateSrv } from 'app/features/templating/template_srv';
 import { TimeSrv } from 'app/features/dashboard/services/TimeSrv';
+import { CustomVariable } from 'app/features/templating/custom_variable';
 
 jest.mock('../metric_find_query');
 
@@ -287,7 +288,7 @@ describe('PrometheusDatasource', () => {
     it('should not escape simple string', () => {
       expect(prometheusSpecialRegexEscape('cryptodepression')).toEqual('cryptodepression');
     });
-    it('should escape $^*+?.()\\', () => {
+    it('should escape $^*+?.()|\\', () => {
       expect(prometheusSpecialRegexEscape("looking'glass")).toEqual("looking\\\\'glass");
       expect(prometheusSpecialRegexEscape('looking{glass')).toEqual('looking\\\\{glass');
       expect(prometheusSpecialRegexEscape('looking}glass')).toEqual('looking\\\\}glass');
@@ -302,12 +303,60 @@ describe('PrometheusDatasource', () => {
       expect(prometheusSpecialRegexEscape('looking(glass')).toEqual('looking\\\\(glass');
       expect(prometheusSpecialRegexEscape('looking)glass')).toEqual('looking\\\\)glass');
       expect(prometheusSpecialRegexEscape('looking\\glass')).toEqual('looking\\\\\\\\glass');
+      expect(prometheusSpecialRegexEscape('looking|glass')).toEqual('looking\\\\|glass');
     });
     it('should escape multiple special characters', () => {
       expect(prometheusSpecialRegexEscape('+looking$glass?')).toEqual('\\\\+looking\\\\$glass\\\\?');
     });
   });
 
+  describe('When interpolating variables', () => {
+    beforeEach(() => {
+      ctx.ds = new PrometheusDatasource(instanceSettings, q, ctx.backendSrvMock, ctx.templateSrvMock, ctx.timeSrvMock);
+      ctx.variable = new CustomVariable({}, {});
+    });
+
+    describe('and value is a string', () => {
+      it('should only escape single quotes', () => {
+        expect(ctx.ds.interpolateQueryExpr("abc'$^*{}[]+?.()|", ctx.variable)).toEqual("abc\\\\'$^*{}[]+?.()|");
+      });
+    });
+
+    describe('and value is a number', () => {
+      it('should return a number', () => {
+        expect(ctx.ds.interpolateQueryExpr(1000, ctx.variable)).toEqual(1000);
+      });
+    });
+
+    describe('and variable allows multi-value', () => {
+      beforeEach(() => {
+        ctx.variable.multi = true;
+      });
+
+      it('should regex escape values if the value is a string', () => {
+        expect(ctx.ds.interpolateQueryExpr('looking*glass', ctx.variable)).toEqual('looking\\\\*glass');
+      });
+
+      it('should return pipe separated values if the value is an array of strings', () => {
+        expect(ctx.ds.interpolateQueryExpr(['a|bc', 'de|f'], ctx.variable)).toEqual('a\\\\|bc|de\\\\|f');
+      });
+    });
+
+    describe('and variable allows all', () => {
+      beforeEach(() => {
+        ctx.variable.includeAll = true;
+      });
+
+      it('should regex escape values if the array is a string', () => {
+        expect(ctx.ds.interpolateQueryExpr('looking*glass', ctx.variable)).toEqual('looking\\\\*glass');
+      });
+
+      it('should return pipe separated values if the value is an array of strings', () => {
+        expect(ctx.ds.interpolateQueryExpr(['a|bc', 'de|f'], ctx.variable)).toEqual('a\\\\|bc|de\\\\|f');
+      });
+    });
+  });
+
   describe('metricFindQuery', () => {
     beforeEach(() => {
       const query = 'query_result(topk(5,rate(http_request_duration_microseconds_count[$__interval])))';
@@ -418,7 +467,7 @@ describe('PrometheusDatasource', () => {
       expect(results.data[0].target).toBe('test{job="testjob"}');
     });
   });
-  describe('When querying prometheus with one target which return multiple series', () => {
+  describe('When querying prometheus with one target which returns multiple series', () => {
     let results;
     const start = 60;
     const end = 360;