From 9bdee12ad900d85e51df96afddd850eb14224a17 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Thu, 14 Sep 2023 09:56:23 +0200 Subject: [PATCH] feat: stop regexes with whitespace (#4681) This PR stops regexes with whitespace from being added as feature naming patterns. --- .../Project/ProjectForm/ProjectForm.tsx | 31 ++++++++++--- .../feature-naming-validation.test.ts | 26 +++++++++++ .../feature-naming-validation.ts | 44 +++++++++++++------ 3 files changed, 80 insertions(+), 21 deletions(-) diff --git a/frontend/src/component/project/Project/ProjectForm/ProjectForm.tsx b/frontend/src/component/project/Project/ProjectForm/ProjectForm.tsx index 9380b3c295..b8d5d15f79 100644 --- a/frontend/src/component/project/Project/ProjectForm/ProjectForm.tsx +++ b/frontend/src/component/project/Project/ProjectForm/ProjectForm.tsx @@ -210,14 +210,31 @@ const ProjectForm: React.FC = ({ }; const onSetFeatureNamingPattern = (regex: string) => { - try { - new RegExp(regex); - setFeatureNamingPattern && setFeatureNamingPattern(regex); - delete errors.featureNamingPattern; - } catch (e) { - errors.featureNamingPattern = 'Invalid regular expression'; - setFeatureNamingPattern && setFeatureNamingPattern(regex); + const disallowedStrings = [ + ' ', + '\\t', + '\\s', + '\\n', + '\\r', + '\\f', + '\\v', + ]; + if ( + disallowedStrings.some(blockedString => + regex.includes(blockedString) + ) + ) { + errors.featureNamingPattern = + 'Whitespace is not allowed in the expression'; + } else { + try { + new RegExp(regex); + delete errors.featureNamingPattern; + } catch (e) { + errors.featureNamingPattern = 'Invalid regular expression'; + } } + setFeatureNamingPattern && setFeatureNamingPattern(regex); updateNamingExampleError({ pattern: regex, example: featureNamingExample || '', diff --git a/src/lib/features/feature-naming-pattern/feature-naming-validation.test.ts b/src/lib/features/feature-naming-pattern/feature-naming-validation.test.ts index fe9a04ff98..f0d099c30f 100644 --- a/src/lib/features/feature-naming-pattern/feature-naming-validation.test.ts +++ b/src/lib/features/feature-naming-pattern/feature-naming-validation.test.ts @@ -61,6 +61,19 @@ describe('validate incoming feature naming data', () => { } }); + test.each([' ', '\\t', '\\n'])( + 'patterns with illegal characters (%s) are invalid', + (string) => { + const pattern = `-${string}[0-9]+`; + + expect( + checkFeatureNamingData({ + pattern, + }), + ).toMatchObject({ state: 'invalid' }); + }, + ); + test('feature naming data with a non-empty example but an empty pattern is invalid', () => { expect( checkFeatureNamingData({ @@ -78,6 +91,19 @@ describe('validate incoming feature naming data', () => { }), ).toMatchObject({ state: 'invalid' }); }); + + test('if the pattern contains disallowed characters, a match is not attempted against the example', () => { + const result = checkFeatureNamingData({ + pattern: 'a. [0-9]+', + example: 'obviously-not-a-match', + }); + + if (result.state === 'valid') { + fail('Expected invalid result'); + } + + expect(result.reasons.length).toBe(1); + }); }); describe('validate feature flag names against a pattern', () => { diff --git a/src/lib/features/feature-naming-pattern/feature-naming-validation.ts b/src/lib/features/feature-naming-pattern/feature-naming-validation.ts index 3bb7df0efc..fb162d773f 100644 --- a/src/lib/features/feature-naming-pattern/feature-naming-validation.ts +++ b/src/lib/features/feature-naming-pattern/feature-naming-validation.ts @@ -10,22 +10,38 @@ export const checkFeatureNamingData = ( if (featureNaming) { const { pattern, example, description } = featureNaming; const errors: string[] = []; - if (pattern && example && !example.match(compileRegex(pattern))) { - errors.push( - `You've provided a feature flag naming example ("${example}") that doesn't match your feature flag naming pattern ("${pattern}"). Please provide an example that matches your supplied pattern. Bear in mind that the pattern must match the whole example, as if it were surrounded by '^' and "$".`, - ); - } + const disallowedStrings = [ + ' ', + '\\t', + '\\s', + '\\n', + '\\r', + '\\f', + '\\v', + ]; - if (!pattern && example) { - errors.push( - "You've provided a feature flag naming example, but no feature flag naming pattern. You must specify a pattern to use an example.", - ); - } + if (pattern) { + if (disallowedStrings.some((str) => pattern.includes(str))) { + errors.push( + `Feature flag names can not contain whitespace. You've provided a feature flag naming pattern that contains a whitespace character: "${pattern}". Remove any whitespace characters from your pattern.`, + ); + } else if (example && !example.match(compileRegex(pattern))) { + errors.push( + `You've provided a feature flag naming example ("${example}") that doesn't match your feature flag naming pattern ("${pattern}"). Please provide an example that matches your supplied pattern. Bear in mind that the pattern must match the whole example, as if it were surrounded by '^' and "$".`, + ); + } + } else { + if (example) { + errors.push( + "You've provided a feature flag naming example, but no feature flag naming pattern. You must specify a pattern to use an example.", + ); + } - if (!pattern && description) { - errors.push( - "You've provided a feature flag naming pattern description, but no feature flag naming pattern. You must have a pattern to use a description.", - ); + if (description) { + errors.push( + "You've provided a feature flag naming pattern description, but no feature flag naming pattern. You must have a pattern to use a description.", + ); + } } const [first, ...rest] = errors;