From 712c4073a71538b15784dd68a93dce8b1df0cf55 Mon Sep 17 00:00:00 2001 From: Sascha Date: Thu, 29 Apr 2021 18:25:48 +0200 Subject: Fix quoted filter string with bug counter The parse function dropped user given quotes. This resulted into a wrong query to the backend, which lead to an error. This error prevented the webui from displaying the proper bug count. --- webui/src/__tests__/query.ts | 61 ++++++++++++++++++++++++---------- webui/src/pages/list/Filter.tsx | 44 ++++++++++++------------ webui/src/pages/list/FilterToolbar.tsx | 13 ++++++-- 3 files changed, 76 insertions(+), 42 deletions(-) (limited to 'webui') diff --git a/webui/src/__tests__/query.ts b/webui/src/__tests__/query.ts index 2f04817c..f8ff6360 100644 --- a/webui/src/__tests__/query.ts +++ b/webui/src/__tests__/query.ts @@ -7,49 +7,76 @@ it('parses a simple query', () => { }); it('parses a query with multiple filters', () => { - expect(parse('foo:bar baz:foo-bar')).toEqual({ + expect(parse(`foo:bar baz:foo-bar`)).toEqual({ foo: ['bar'], baz: ['foo-bar'], }); }); it('parses a quoted query', () => { - expect(parse('foo:"bar"')).toEqual({ - foo: ['bar'], + expect(parse(`foo:"bar"`)).toEqual({ + foo: [`"bar"`], }); - expect(parse("foo:'bar'")).toEqual({ - foo: ['bar'], + expect(parse(`foo:'bar'`)).toEqual({ + foo: [`'bar'`], + }); + + expect(parse(`label:abc freetext`)).toEqual({ + label: [`abc`], + freetext: [''], + }); + + expect(parse(`label:abc with "quotes" in freetext`)).toEqual({ + label: [`abc`], + with: [''], + quotes: [''], + in: [''], + freetext: [''], + }); + + expect(parse(`label:'multi word label'`)).toEqual({ + label: [`'multi word label'`], + }); + + expect(parse(`label:"multi word label"`)).toEqual({ + label: [`"multi word label"`], + }); + + expect(parse(`label:'multi word label with "nested" quotes'`)).toEqual({ + label: [`'multi word label with "nested" quotes'`], }); - expect(parse('foo:\'bar "nested" quotes\'')).toEqual({ - foo: ['bar "nested" quotes'], + expect(parse(`label:"multi word label with 'nested' quotes"`)).toEqual({ + label: [`"multi word label with 'nested' quotes"`], }); - expect(parse("foo:'escaped\\' quotes'")).toEqual({ - foo: ["escaped' quotes"], + expect(parse(`foo:'escaped\\' quotes'`)).toEqual({ + foo: [`'escaped\\' quotes'`], }); }); it('parses a query with repetitions', () => { - expect(parse('foo:bar foo:baz')).toEqual({ + expect(parse(`foo:bar foo:baz`)).toEqual({ foo: ['bar', 'baz'], }); }); it('parses a complex query', () => { - expect(parse('foo:bar foo:baz baz:"foobar" idont:\'know\'')).toEqual({ + expect(parse(`foo:bar foo:baz baz:"foobar" idont:'know'`)).toEqual({ foo: ['bar', 'baz'], - baz: ['foobar'], - idont: ['know'], + baz: [`"foobar"`], + idont: [`'know'`], }); }); it('quotes values', () => { - expect(quote('foo')).toEqual('foo'); - expect(quote('foo bar')).toEqual('"foo bar"'); - expect(quote('foo "bar"')).toEqual(`'foo "bar"'`); - expect(quote(`foo "bar" 'baz'`)).toEqual(`"foo \\"bar\\" 'baz'"`); + expect(quote(`foo`)).toEqual(`foo`); + expect(quote(`foo bar`)).toEqual(`"foo bar"`); + expect(quote(`foo "bar"`)).toEqual(`"foo "bar""`); + expect(quote(`foo 'bar'`)).toEqual(`"foo "bar""`); + expect(quote(`'foo'`)).toEqual(`"foo"`); + expect(quote(`foo "bar" 'baz'`)).toEqual(`"foo "bar" "baz""`); }); it('stringifies params', () => { diff --git a/webui/src/pages/list/Filter.tsx b/webui/src/pages/list/Filter.tsx index 3559b3ce..84b08029 100644 --- a/webui/src/pages/list/Filter.tsx +++ b/webui/src/pages/list/Filter.tsx @@ -35,52 +35,50 @@ const ITEM_HEIGHT = 48; export type Query = { [key: string]: string[] }; function parse(query: string): Query { - // TODO: extract the rest of the query? const params: Query = {}; - - // TODO: support escaping without quotes - const re = /(\w+):([A-Za-z0-9-]+|(["'])(([^\3]|\\.)*)\3)+/g; + let re = new RegExp(/(\w+)(:('.*'|".*"|\S*))?/, 'g'); let matches; while ((matches = re.exec(query)) !== null) { if (!params[matches[1]]) { params[matches[1]] = []; } - - let value; - if (matches[4]) { - value = matches[4]; + if (matches[3] !== undefined) { + params[matches[1]].push(matches[3]); } else { - value = matches[2]; + params[matches[1]].push(''); } - value = value.replace(/\\(.)/g, '$1'); - params[matches[1]].push(value); } return params; } function quote(value: string): string { - const hasSingle = value.includes("'"); - const hasDouble = value.includes('"'); const hasSpaces = value.includes(' '); - if (!hasSingle && !hasDouble && !hasSpaces) { - return value; - } + const isDoubleQuotedRegEx = RegExp(/^'.*'$/); + const isSingleQuotedRegEx = RegExp(/^".*"$/); + const isQuoted = () => + isDoubleQuotedRegEx.test(value) || isSingleQuotedRegEx.test(value); - if (!hasDouble) { - return `"${value}"`; + //Test if label name contains whitespace between quotes. If no quoates but + //whitespace, then quote string. + if (!isQuoted() && hasSpaces) { + value = `"${value}"`; } - if (!hasSingle) { - return `'${value}'`; + //Convert single quote (tick) to double quote. This way quoting is always + //uniform and can be relied upon by the label menu + const hasSingle = value.includes(`'`); + if (hasSingle) { + value = value.replace(/'/g, `"`); } - value = value.replace(/"/g, '\\"'); - return `"${value}"`; + return value; } function stringify(params: Query): string { const parts: string[][] = Object.entries(params).map(([key, values]) => { - return values.map((value) => `${key}:${quote(value)}`); + return values.map((value) => + value.length > 0 ? `${key}:${quote(value)}` : key + ); }); return new Array().concat(...parts).join(' '); } diff --git a/webui/src/pages/list/FilterToolbar.tsx b/webui/src/pages/list/FilterToolbar.tsx index e109578d..4ac579f5 100644 --- a/webui/src/pages/list/FilterToolbar.tsx +++ b/webui/src/pages/list/FilterToolbar.tsx @@ -56,6 +56,16 @@ function CountingFilter({ query, children, ...props }: CountingFilterProps) { ); } +function quoteLabel(value: string) { + const hasUnquotedColon = RegExp(/^[^'"].*:.*[^'"]$/); + if (hasUnquotedColon.test(value)) { + //quote values which contain a colon but are not quoted. + //E.g. abc:abc becomes "abc:abc" + return `"${value}"`; + } + return value; +} + type Props = { query: string; queryLocation: (query: string) => LocationDescriptor; @@ -87,7 +97,7 @@ function FilterToolbar({ query, queryLocation }: Props) { labelsData.repository.validLabels.nodes ) { labels = labelsData.repository.validLabels.nodes.map((node) => [ - node.name, + quoteLabel(node.name), node.name, node.color, ]); @@ -131,7 +141,6 @@ function FilterToolbar({ query, queryLocation }: Props) { [key]: [], }); - // TODO: author/label filters return ( Date: Mon, 3 May 2021 15:15:21 +0200 Subject: Add test cases Key:Value parsing test for quoated colon. E.g. label:"foo:bar" Key:Value:Value parsing test. E.g. metadata:key:"https://www.example.com/" --- webui/src/__tests__/query.ts | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) (limited to 'webui') diff --git a/webui/src/__tests__/query.ts b/webui/src/__tests__/query.ts index f8ff6360..7abcecd2 100644 --- a/webui/src/__tests__/query.ts +++ b/webui/src/__tests__/query.ts @@ -11,16 +11,6 @@ it('parses a query with multiple filters', () => { foo: ['bar'], baz: ['foo-bar'], }); -}); - -it('parses a quoted query', () => { - expect(parse(`foo:"bar"`)).toEqual({ - foo: [`"bar"`], - }); - - expect(parse(`foo:'bar'`)).toEqual({ - foo: [`'bar'`], - }); expect(parse(`label:abc freetext`)).toEqual({ label: [`abc`], @@ -35,6 +25,17 @@ it('parses a quoted query', () => { freetext: [''], }); +}); + +it('parses a quoted query', () => { + expect(parse(`foo:"bar"`)).toEqual({ + foo: [`"bar"`], + }); + + expect(parse(`foo:'bar'`)).toEqual({ + foo: [`'bar'`], + }); + expect(parse(`label:'multi word label'`)).toEqual({ label: [`'multi word label'`], }); @@ -51,6 +52,10 @@ it('parses a quoted query', () => { label: [`"multi word label with 'nested' quotes"`], }); + expect(parse(`label:"with:quoated:colon"`)).toEqual({ + label: [`"with:quoated:colon"`], + }); + expect(parse(`foo:'escaped\\' quotes'`)).toEqual({ foo: [`'escaped\\' quotes'`], }); @@ -70,6 +75,12 @@ it('parses a complex query', () => { }); }); +it('parses a key:value:value query', () => { + expect(parse(`meta:github:"https://github.com/MichaelMure/git-bug"`)).toEqual({ + meta: [`github:"https://github.com/MichaelMure/git-bug"`], + }); +}); + it('quotes values', () => { expect(quote(`foo`)).toEqual(`foo`); expect(quote(`foo bar`)).toEqual(`"foo bar"`); -- cgit From 77d36247b3b761e21ef38a764d315cd4d22e7668 Mon Sep 17 00:00:00 2001 From: Sascha Date: Mon, 3 May 2021 15:23:42 +0200 Subject: Update escape quote test --- webui/src/__tests__/query.ts | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) (limited to 'webui') diff --git a/webui/src/__tests__/query.ts b/webui/src/__tests__/query.ts index 7abcecd2..69e2f766 100644 --- a/webui/src/__tests__/query.ts +++ b/webui/src/__tests__/query.ts @@ -24,7 +24,6 @@ it('parses a query with multiple filters', () => { in: [''], freetext: [''], }); - }); it('parses a quoted query', () => { @@ -55,9 +54,23 @@ it('parses a quoted query', () => { expect(parse(`label:"with:quoated:colon"`)).toEqual({ label: [`"with:quoated:colon"`], }); +}); + +it('should not escape nested quotes', () => { + expect(parse(`foo:'do not escape this ->'<- quote'`)).toEqual({ + foo: [`'do not escape this ->'<- quote'`], + }); + + expect(parse(`foo:'do not escape this ->"<- quote'`)).toEqual({ + foo: [`'do not escape this ->"<- quote'`], + }); + + expect(parse(`foo:"do not escape this ->"<- quote"`)).toEqual({ + foo: [`"do not escape this ->"<- quote"`], + }); - expect(parse(`foo:'escaped\\' quotes'`)).toEqual({ - foo: [`'escaped\\' quotes'`], + expect(parse(`foo:"do not escape this ->'<- quote"`)).toEqual({ + foo: [`"do not escape this ->'<- quote"`], }); }); @@ -76,9 +89,11 @@ it('parses a complex query', () => { }); it('parses a key:value:value query', () => { - expect(parse(`meta:github:"https://github.com/MichaelMure/git-bug"`)).toEqual({ - meta: [`github:"https://github.com/MichaelMure/git-bug"`], - }); + expect(parse(`meta:github:"https://github.com/MichaelMure/git-bug"`)).toEqual( + { + meta: [`github:"https://github.com/MichaelMure/git-bug"`], + } + ); }); it('quotes values', () => { -- cgit From 7446a20db8907acedbd14ddcc6a99de577268c1c Mon Sep 17 00:00:00 2001 From: Sascha Date: Wed, 5 May 2021 12:49:11 +0200 Subject: Do not greedy match quotes --- webui/src/__tests__/query.ts | 44 ++++++++++++++++++++++++++++++++++++----- webui/src/pages/list/Filter.tsx | 6 +++--- 2 files changed, 42 insertions(+), 8 deletions(-) (limited to 'webui') diff --git a/webui/src/__tests__/query.ts b/webui/src/__tests__/query.ts index 69e2f766..97ec75a6 100644 --- a/webui/src/__tests__/query.ts +++ b/webui/src/__tests__/query.ts @@ -17,11 +17,11 @@ it('parses a query with multiple filters', () => { freetext: [''], }); - expect(parse(`label:abc with "quotes" in freetext`)).toEqual({ + expect(parse(`label:abc with "quotes" 'in' freetext`)).toEqual({ label: [`abc`], with: [''], - quotes: [''], - in: [''], + '"quotes"': [''], + "'in'": [''], freetext: [''], }); }); @@ -54,11 +54,44 @@ it('parses a quoted query', () => { expect(parse(`label:"with:quoated:colon"`)).toEqual({ label: [`"with:quoated:colon"`], }); + + expect(parse(`label:'name ends after this ->' quote'`)).toEqual({ + label: [`'name ends after this ->'`], + "quote'": [``], + }); + + expect(parse(`label:"name ends after this ->" quote"`)).toEqual({ + label: [`"name ends after this ->"`], + 'quote"': [``], + }); + + expect(parse(`label:'this ->"<- quote belongs to label name'`)).toEqual({ + label: [`'this ->"<- quote belongs to label name'`], + }); + + expect(parse(`label:"this ->'<- quote belongs to label name"`)).toEqual({ + label: [`"this ->'<- quote belongs to label name"`], + }); + + expect(parse(`label:'names end with'whitespace not with quotes`)).toEqual({ + label: [`'names end with'whitespace`], + not: [``], + with: [``], + quotes: [``], + }); + + expect(parse(`label:"names end with"whitespace not with quotes`)).toEqual({ + label: [`"names end with"whitespace`], + not: [``], + with: [``], + quotes: [``], + }); }); it('should not escape nested quotes', () => { expect(parse(`foo:'do not escape this ->'<- quote'`)).toEqual({ - foo: [`'do not escape this ->'<- quote'`], + foo: [`'do not escape this ->'<-`], + "quote'": [``], }); expect(parse(`foo:'do not escape this ->"<- quote'`)).toEqual({ @@ -66,7 +99,8 @@ it('should not escape nested quotes', () => { }); expect(parse(`foo:"do not escape this ->"<- quote"`)).toEqual({ - foo: [`"do not escape this ->"<- quote"`], + foo: [`"do not escape this ->"<-`], + 'quote"': [``], }); expect(parse(`foo:"do not escape this ->'<- quote"`)).toEqual({ diff --git a/webui/src/pages/list/Filter.tsx b/webui/src/pages/list/Filter.tsx index 84b08029..496fb3ba 100644 --- a/webui/src/pages/list/Filter.tsx +++ b/webui/src/pages/list/Filter.tsx @@ -36,7 +36,7 @@ export type Query = { [key: string]: string[] }; function parse(query: string): Query { const params: Query = {}; - let re = new RegExp(/(\w+)(:('.*'|".*"|\S*))?/, 'g'); + let re = new RegExp(/([^:\s]+)(:('[^']*'\S*|"[^"]*"\S*|\S*))?/, 'g'); let matches; while ((matches = re.exec(query)) !== null) { if (!params[matches[1]]) { @@ -53,8 +53,8 @@ function parse(query: string): Query { function quote(value: string): string { const hasSpaces = value.includes(' '); - const isDoubleQuotedRegEx = RegExp(/^'.*'$/); - const isSingleQuotedRegEx = RegExp(/^".*"$/); + const isSingleQuotedRegEx = RegExp(/^'.*'$/); + const isDoubleQuotedRegEx = RegExp(/^".*"$/); const isQuoted = () => isDoubleQuotedRegEx.test(value) || isSingleQuotedRegEx.test(value); -- cgit