Loki: Fix wrongly escaped label values when using LabelFilter (#59812)

* change backticks to quotes

* add unescaping to `addFilterAsLabelFilter`

* fix removal of wrong quotes

* change unescape order
This commit is contained in:
Sven Grossmann 2022-12-06 12:30:03 +01:00 committed by GitHub
parent 78ef55eb06
commit 932349b5ab
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 95 additions and 39 deletions

View File

@ -1,4 +1,4 @@
import { isBytesString } from './languageUtils'; import { escapeLabelValueInExactSelector, isBytesString, unescapeLabelValue } from './languageUtils';
describe('isBytesString', () => { describe('isBytesString', () => {
it('correctly matches bytes string with integers', () => { it('correctly matches bytes string with integers', () => {
@ -18,3 +18,27 @@ describe('isBytesString', () => {
expect(isBytesString('1.234')).toBe(false); expect(isBytesString('1.234')).toBe(false);
}); });
}); });
describe('escapeLabelValueInExactSelector', () => {
it.each`
value | escapedValue
${'nothing to escape'} | ${'nothing to escape'}
${'escape quote: "'} | ${'escape quote: \\"'}
${'escape newline: \nend'} | ${'escape newline: \\nend'}
${'escape slash: \\'} | ${'escape slash: \\\\'}
`('when called with $value', ({ value, escapedValue }) => {
expect(escapeLabelValueInExactSelector(value)).toEqual(escapedValue);
});
});
describe('unescapeLabelValueInExactSelector', () => {
it.each`
value | unescapedValue
${'nothing to unescape'} | ${'nothing to unescape'}
${'escape quote: \\"'} | ${'escape quote: "'}
${'escape newline: \\nend'} | ${'escape newline: \nend'}
${'escape slash: \\\\'} | ${'escape slash: \\'}
`('when called with $value', ({ value, unescapedValue }) => {
expect(unescapeLabelValue(value)).toEqual(unescapedValue);
});
});

View File

@ -35,6 +35,10 @@ export function escapeLabelValueInExactSelector(labelValue: string): string {
return labelValue.replace(/\\/g, '\\\\').replace(/\n/g, '\\n').replace(/"/g, '\\"'); return labelValue.replace(/\\/g, '\\\\').replace(/\n/g, '\\n').replace(/"/g, '\\"');
} }
export function unescapeLabelValue(labelValue: string): string {
return labelValue.replace(/\\n/g, '\n').replace(/\\"/g, '"').replace(/\\\\/g, '\\');
}
export function escapeLabelValueInRegexSelector(labelValue: string): string { export function escapeLabelValueInRegexSelector(labelValue: string): string {
return escapeLabelValueInExactSelector(escapeLokiRegexp(labelValue)); return escapeLabelValueInExactSelector(escapeLokiRegexp(labelValue));
} }

View File

@ -8,42 +8,50 @@ import {
describe('addLabelToQuery()', () => { describe('addLabelToQuery()', () => {
it.each` it.each`
query | description | label | operator | value | expectedResult query | description | label | operator | value | expectedResult
${'{x="y"}'} | ${'no label and value'} | ${''} | ${'='} | ${''} | ${''} ${'{x="y"}'} | ${'no label and value'} | ${''} | ${'='} | ${''} | ${''}
${'{x="yy"}'} | ${'simple query'} | ${'bar'} | ${'='} | ${'baz'} | ${'{x="yy", bar="baz"}'} ${'{x="yy"}'} | ${'simple query'} | ${'bar'} | ${'='} | ${'baz'} | ${'{x="yy", bar="baz"}'}
${'{x="yy"}'} | ${'simple query'} | ${'bar'} | ${'='} | ${'baz'} | ${'{x="yy", bar="baz"}'} ${'{x="yy"}'} | ${'simple query'} | ${'bar'} | ${'='} | ${'baz'} | ${'{x="yy", bar="baz"}'}
${'{x="yy"}'} | ${'custom operator'} | ${'bar'} | ${'!='} | ${'baz'} | ${'{x="yy", bar!="baz"}'} ${'{x="yy"}'} | ${'custom operator'} | ${'bar'} | ${'!='} | ${'baz'} | ${'{x="yy", bar!="baz"}'}
${'rate({}[1m])'} | ${'do not modify ranges'} | ${'bar'} | ${'='} | ${'baz'} | ${'rate({bar="baz"}[1m])'} ${'rate({}[1m])'} | ${'do not modify ranges'} | ${'bar'} | ${'='} | ${'baz'} | ${'rate({bar="baz"}[1m])'}
${'sum by (host) (rate({} [1m]))'} | ${'detect in-order function use'} | ${'bar'} | ${'='} | ${'baz'} | ${'sum by (host) (rate({bar="baz"}[1m]))'} ${'sum by (host) (rate({} [1m]))'} | ${'detect in-order function use'} | ${'bar'} | ${'='} | ${'baz'} | ${'sum by (host) (rate({bar="baz"}[1m]))'}
${'{instance="my-host.com:9100"}'} | ${'selectors with punctuation'} | ${'bar'} | ${'='} | ${'baz'} | ${'{instance="my-host.com:9100", bar="baz"}'} ${'{instance="my-host.com:9100"}'} | ${'selectors with punctuation'} | ${'bar'} | ${'='} | ${'baz'} | ${'{instance="my-host.com:9100", bar="baz"}'}
${'{list="a,b,c"}'} | ${'selectors with punctuation'} | ${'bar'} | ${'='} | ${'baz'} | ${'{list="a,b,c", bar="baz"}'} ${'{list="a,b,c"}'} | ${'selectors with punctuation'} | ${'bar'} | ${'='} | ${'baz'} | ${'{list="a,b,c", bar="baz"}'}
${'rate({}[5m]) + rate({}[5m])'} | ${'arithmetical expressions'} | ${'bar'} | ${'='} | ${'baz'} | ${'rate({bar="baz"}[5m]) + rate({bar="baz"}[5m])'} ${'rate({}[5m]) + rate({}[5m])'} | ${'arithmetical expressions'} | ${'bar'} | ${'='} | ${'baz'} | ${'rate({bar="baz"}[5m]) + rate({bar="baz"}[5m])'}
${'avg(rate({x="y"} [$__interval]))+ sum(rate({}[5m]))'} | ${'arithmetical expressions'} | ${'bar'} | ${'='} | ${'baz'} | ${'avg(rate({x="y", bar="baz"} [$__interval]))+ sum(rate({bar="baz"}[5m]))'} ${'avg(rate({x="y"} [$__interval]))+ sum(rate({}[5m]))'} | ${'arithmetical expressions'} | ${'bar'} | ${'='} | ${'baz'} | ${'avg(rate({x="y", bar="baz"} [$__interval]))+ sum(rate({bar="baz"}[5m]))'}
${'rate({x="yy"}[5m]) * rate({y="zz",a="bb"}[5m]) * rate({}[5m])'} | ${'arithmetical expressions'} | ${'bar'} | ${'='} | ${'baz'} | ${'rate({x="yy", bar="baz"}[5m]) * rate({y="zz", a="bb", bar="baz"}[5m]) * rate({bar="baz"}[5m])'} ${'rate({x="yy"}[5m]) * rate({y="zz",a="bb"}[5m]) * rate({}[5m])'} | ${'arithmetical expressions'} | ${'bar'} | ${'='} | ${'baz'} | ${'rate({x="yy", bar="baz"}[5m]) * rate({y="zz", a="bb", bar="baz"}[5m]) * rate({bar="baz"}[5m])'}
${'{x="yy", bar!="baz"}'} | ${'do not add duplicate labels'} | ${'bar'} | ${'!='} | ${'baz'} | ${'{x="yy", bar!="baz"}'} ${'{x="yy", bar!="baz"}'} | ${'do not add duplicate labels'} | ${'bar'} | ${'!='} | ${'baz'} | ${'{x="yy", bar!="baz"}'}
${'rate({bar="baz"}[1m])'} | ${'do not add duplicate labels'} | ${'bar'} | ${'='} | ${'baz'} | ${'rate({bar="baz"}[1m])'} ${'rate({bar="baz"}[1m])'} | ${'do not add duplicate labels'} | ${'bar'} | ${'='} | ${'baz'} | ${'rate({bar="baz"}[1m])'}
${'{list="a,b,c", bar="baz"}'} | ${'do not add duplicate labels'} | ${'bar'} | ${'='} | ${'baz'} | ${'{list="a,b,c", bar="baz"}'} ${'{list="a,b,c", bar="baz"}'} | ${'do not add duplicate labels'} | ${'bar'} | ${'='} | ${'baz'} | ${'{list="a,b,c", bar="baz"}'}
${'avg(rate({bar="baz"} [$__interval]))+ sum(rate({bar="baz"}[5m]))'} | ${'do not add duplicate labels'} | ${'bar'} | ${'='} | ${'baz'} | ${'avg(rate({bar="baz"} [$__interval]))+ sum(rate({bar="baz"}[5m]))'} ${'avg(rate({bar="baz"} [$__interval]))+ sum(rate({bar="baz"}[5m]))'} | ${'do not add duplicate labels'} | ${'bar'} | ${'='} | ${'baz'} | ${'avg(rate({bar="baz"} [$__interval]))+ sum(rate({bar="baz"}[5m]))'}
${'{x="y"} |="yy"'} | ${'do not remove filters'} | ${'bar'} | ${'='} | ${'baz'} | ${'{x="y", bar="baz"} |="yy"'} ${'{x="y"} |="yy"'} | ${'do not remove filters'} | ${'bar'} | ${'='} | ${'baz'} | ${'{x="y", bar="baz"} |="yy"'}
${'{x="y"} |="yy" !~"xx"'} | ${'do not remove filters'} | ${'bar'} | ${'='} | ${'baz'} | ${'{x="y", bar="baz"} |="yy" !~"xx"'} ${'{x="y"} |="yy" !~"xx"'} | ${'do not remove filters'} | ${'bar'} | ${'='} | ${'baz'} | ${'{x="y", bar="baz"} |="yy" !~"xx"'}
${'{x="y"} or {}'} | ${'metric with logical operators'} | ${'bar'} | ${'='} | ${'baz'} | ${'{x="y", bar="baz"} or {bar="baz"}'} ${'{x="y"} or {}'} | ${'metric with logical operators'} | ${'bar'} | ${'='} | ${'baz'} | ${'{x="y", bar="baz"} or {bar="baz"}'}
${'{x="y"} and {}'} | ${'metric with logical operators'} | ${'bar'} | ${'='} | ${'baz'} | ${'{x="y", bar="baz"} and {bar="baz"}'} ${'{x="y"} and {}'} | ${'metric with logical operators'} | ${'bar'} | ${'='} | ${'baz'} | ${'{x="y", bar="baz"} and {bar="baz"}'}
${'sum(rate({job="foo"}[2m])) by (value $variable)'} | ${'template variables'} | ${'bar'} | ${'='} | ${'baz'} | ${'sum(rate({job="foo", bar="baz"}[2m])) by (value $variable)'} ${'sum(rate({job="foo"}[2m])) by (value $variable)'} | ${'template variables'} | ${'bar'} | ${'='} | ${'baz'} | ${'sum(rate({job="foo", bar="baz"}[2m])) by (value $variable)'}
${'rate({x="y"}[${__range_s}s])'} | ${'metric query with range grafana variable'} | ${'bar'} | ${'='} | ${'baz'} | ${'rate({x="y", bar="baz"}[${__range_s}s])'} ${'rate({x="y"}[${__range_s}s])'} | ${'metric query with range grafana variable'} | ${'bar'} | ${'='} | ${'baz'} | ${'rate({x="y", bar="baz"}[${__range_s}s])'}
${'max by (id, name, type) ({type=~"foo|bar|baz-test"}) * on(id) group_right(id, type, name) sum by (id) (rate({} [5m])) * 1000'} | ${'metric query with labels in label list with the group modifier'} | ${'bar'} | ${'='} | ${'baz'} | ${'max by (id, name, type) ({type=~"foo|bar|baz-test", bar="baz"}) * on(id) group_right(id, type, name) sum by (id) (rate({bar="baz"}[5m])) * 1000'} ${'max by (id, name, type) ({type=~"foo|bar|baz-test"}) * on(id) group_right(id, type, name) sum by (id) (rate({} [5m])) * 1000'} | ${'metric query with labels in label list with the group modifier'} | ${'bar'} | ${'='} | ${'baz'} | ${'max by (id, name, type) ({type=~"foo|bar|baz-test", bar="baz"}) * on(id) group_right(id, type, name) sum by (id) (rate({bar="baz"}[5m])) * 1000'}
${'{foo="bar"} | logfmt'} | ${'query with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'{foo="bar"} | logfmt | bar=`baz`'} ${'{foo="bar"} | logfmt'} | ${'query with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'{foo="bar"} | logfmt | bar=`baz`'}
${'{foo="bar"} | logfmt | json'} | ${'query with multiple parsers'} | ${'bar'} | ${'='} | ${'baz'} | ${'{foo="bar"} | logfmt | json | bar=`baz`'} ${'{foo="bar"} | logfmt | json'} | ${'query with multiple parsers'} | ${'bar'} | ${'='} | ${'baz'} | ${'{foo="bar"} | logfmt | json | bar=`baz`'}
${'{foo="bar"} | logfmt | x="y"'} | ${'query with parser and label filter'} | ${'bar'} | ${'='} | ${'baz'} | ${'{foo="bar"} | logfmt | x="y" | bar=`baz`'} ${'{foo="bar"} | logfmt | x="y"'} | ${'query with parser and label filter'} | ${'bar'} | ${'='} | ${'baz'} | ${'{foo="bar"} | logfmt | x="y" | bar=`baz`'}
${'rate({foo="bar"} | logfmt [5m])'} | ${'metric query with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'rate({foo="bar"} | logfmt | bar=`baz` [5m])'} ${'rate({foo="bar"} | logfmt [5m])'} | ${'metric query with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'rate({foo="bar"} | logfmt | bar=`baz` [5m])'}
${'sum by(host) (rate({foo="bar"} | logfmt | x="y" | line_format "{{.status}}" [5m]))'} | ${'metric query with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'sum by(host) (rate({foo="bar"} | logfmt | x="y" | bar=`baz` | line_format "{{.status}}" [5m]))'} ${'sum by(host) (rate({foo="bar"} | logfmt | x="y" | line_format "{{.status}}" [5m]))'} | ${'metric query with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'sum by(host) (rate({foo="bar"} | logfmt | x="y" | bar=`baz` | line_format "{{.status}}" [5m]))'}
${'{foo="bar"} | logfmt | line_format "{{.status}}"'} | ${'do not add filter to line_format expressions in query with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'{foo="bar"} | logfmt | bar=`baz` | line_format "{{.status}}"'} ${'{foo="bar"} | logfmt | line_format "{{.status}}"'} | ${'do not add filter to line_format expressions in query with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'{foo="bar"} | logfmt | bar=`baz` | line_format "{{.status}}"'}
${'{foo="bar"} | logfmt | line_format "{{status}}"'} | ${'do not add filter to line_format expressions in query with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'{foo="bar"} | logfmt | bar=`baz` | line_format "{{status}}"'} ${'{foo="bar"} | logfmt | line_format "{{status}}"'} | ${'do not add filter to line_format expressions in query with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'{foo="bar"} | logfmt | bar=`baz` | line_format "{{status}}"'}
${'{}'} | ${'query without stream selector'} | ${'bar'} | ${'='} | ${'baz'} | ${'{bar="baz"}'} ${'{}'} | ${'query without stream selector'} | ${'bar'} | ${'='} | ${'baz'} | ${'{bar="baz"}'}
${'{} | logfmt'} | ${'query without stream selector and with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'{bar="baz"}| logfmt'} ${'{} | logfmt'} | ${'query without stream selector and with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'{bar="baz"}| logfmt'}
${'{} | x="y"'} | ${'query without stream selector and with label filter'} | ${'bar'} | ${'='} | ${'baz'} | ${'{bar="baz"}| x="y"'} ${'{} | x="y"'} | ${'query without stream selector and with label filter'} | ${'bar'} | ${'='} | ${'baz'} | ${'{bar="baz"}| x="y"'}
${'{} | logfmt | x="y"'} | ${'query without stream selector and with parser and label filter'} | ${'bar'} | ${'='} | ${'baz'} | ${'{bar="baz"}| logfmt | x="y"'} ${'{} | logfmt | x="y"'} | ${'query without stream selector and with parser and label filter'} | ${'bar'} | ${'='} | ${'baz'} | ${'{bar="baz"}| logfmt | x="y"'}
${'sum(rate({x="y"} [5m])) + sum(rate({} | logfmt [5m]))'} | ${'metric query with 1 empty and 1 not empty stream selector with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'sum(rate({x="y", bar="baz"} [5m])) + sum(rate({bar="baz"}| logfmt [5m]))'} ${'sum(rate({x="y"} [5m])) + sum(rate({} | logfmt [5m]))'} | ${'metric query with 1 empty and 1 not empty stream selector with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'sum(rate({x="y", bar="baz"} [5m])) + sum(rate({bar="baz"}| logfmt [5m]))'}
${'sum(rate({x="y"} | logfmt [5m])) + sum(rate({} [5m]))'} | ${'metric query with 1 non-empty and 1 not empty stream selector with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'sum(rate({x="y", bar="baz"} | logfmt [5m])) + sum(rate({bar="baz"}[5m]))'} ${'sum(rate({x="y"} | logfmt [5m])) + sum(rate({} [5m]))'} | ${'metric query with 1 non-empty and 1 not empty stream selector with parser'} | ${'bar'} | ${'='} | ${'baz'} | ${'sum(rate({x="y", bar="baz"} | logfmt [5m])) + sum(rate({bar="baz"}[5m]))'}
${'{x="yy"}'} | ${'simple query with escaped value'} | ${'bar'} | ${'='} | ${'"baz"'} | ${'{x="yy", bar=""baz""}'}
${'{x="yy"}'} | ${'simple query with escaped value'} | ${'bar'} | ${'='} | ${'\\"baz\\"'} | ${'{x="yy", bar="\\"baz\\""}'}
${'{x="yy"}'} | ${'simple query with an other escaped value'} | ${'bar'} | ${'='} | ${'baz\\\\'} | ${'{x="yy", bar="baz\\\\"}'}
${'{x="yy"}'} | ${'simple query with escaped value and regex operator'} | ${'bar'} | ${'~='} | ${'baz\\\\'} | ${'{x="yy", bar~="baz\\\\"}'}
${'{foo="bar"} | logfmt'} | ${'query with parser with escaped value'} | ${'bar'} | ${'='} | ${'\\"baz\\"'} | ${'{foo="bar"} | logfmt | bar=`"baz"`'}
${'{foo="bar"} | logfmt'} | ${'query with parser with an other escaped value'} | ${'bar'} | ${'='} | ${'baz\\\\'} | ${'{foo="bar"} | logfmt | bar=`baz\\`'}
${'{foo="bar"} | logfmt'} | ${'query with parser with escaped value and regex operator'} | ${'bar'} | ${'~='} | ${'\\"baz\\"'} | ${'{foo="bar"} | logfmt | bar~=`"baz"`'}
${'{foo="bar"} | logfmt'} | ${'query with parser with escaped value and regex operator'} | ${'bar'} | ${'~='} | ${'\\"baz\\"'} | ${'{foo="bar"} | logfmt | bar~=`"baz"`'}
`( `(
'should add label to query: $query, description: $description', 'should add label to query: $query, description: $description',
({ query, description, label, operator, value, expectedResult }) => { ({ query, description, label, operator, value, expectedResult }) => {

View File

@ -18,6 +18,7 @@ import {
import { QueryBuilderLabelFilter } from '../prometheus/querybuilder/shared/types'; import { QueryBuilderLabelFilter } from '../prometheus/querybuilder/shared/types';
import { unescapeLabelValue } from './languageUtils';
import { LokiQueryModeller } from './querybuilder/LokiQueryModeller'; import { LokiQueryModeller } from './querybuilder/LokiQueryModeller';
import { buildVisualQueryFromString } from './querybuilder/parsing'; import { buildVisualQueryFromString } from './querybuilder/parsing';
@ -329,7 +330,9 @@ function addFilterAsLabelFilter(
const start = query.substring(prev, match.to); const start = query.substring(prev, match.to);
const end = isLast ? query.substring(match.to) : ''; const end = isLast ? query.substring(match.to) : '';
const labelFilter = ` | ${filter.label}${filter.op}\`${filter.value}\``; // we now unescape all escaped values again, because we are using backticks which can handle those cases.
// we also don't care about the operator here, because we need to unescape for both, regex and equal.
const labelFilter = ` | ${filter.label}${filter.op}\`${unescapeLabelValue(filter.value)}\``;
newQuery += start + labelFilter + end; newQuery += start + labelFilter + end;
prev = match.to; prev = match.to;
} }

View File

@ -571,6 +571,21 @@ describe('buildVisualQueryFromString', () => {
}) })
); );
}); });
it('parses simple query with quotes in label value', () => {
expect(buildVisualQueryFromString('{app="\\"frontend\\""}')).toEqual(
noErrors({
labels: [
{
op: '=',
value: '\\"frontend\\"',
label: 'app',
},
],
operations: [],
})
);
});
}); });
function noErrors(query: LokiVisualQuery) { function noErrors(query: LokiVisualQuery) {

View File

@ -212,7 +212,9 @@ function getLabel(expr: string, node: SyntaxNode): QueryBuilderLabelFilter {
const labelNode = node.getChild(Identifier); const labelNode = node.getChild(Identifier);
const label = getString(expr, labelNode); const label = getString(expr, labelNode);
const op = getString(expr, labelNode!.nextSibling); const op = getString(expr, labelNode!.nextSibling);
const value = getString(expr, node.getChild(String)).replace(/"/g, ''); let value = getString(expr, node.getChild(String));
// `value` is wrapped in double quotes, so we need to remove them. As a value can contain double quotes, we can't use RegEx here.
value = value.substring(1, value.length - 1);
return { return {
label, label,