From 32b0816ecbad635160d60ab9d1d7b9e26358aea5 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 3 Feb 2021 11:41:33 +0000 Subject: [PATCH 1/4] Quick fix for string-templates, was being a bit too fuzzy in its lookup of possible helper names. --- packages/string-templates/src/index.js | 4 +++- packages/string-templates/src/processors/preprocessor.js | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/string-templates/src/index.js b/packages/string-templates/src/index.js index 9cedcde061..6992eb839e 100644 --- a/packages/string-templates/src/index.js +++ b/packages/string-templates/src/index.js @@ -91,7 +91,9 @@ module.exports.processStringSync = (string, context) => { } string = processors.preprocess(string) // this does not throw an error when template can't be fulfilled, have to try correct beforehand - const template = hbsInstance.compile(string) + const template = hbsInstance.compile(string, { + strict: false, + }) return processors.postprocess(template(clonedContext)) } diff --git a/packages/string-templates/src/processors/preprocessor.js b/packages/string-templates/src/processors/preprocessor.js index af6d656eaa..ee3a3a9730 100644 --- a/packages/string-templates/src/processors/preprocessor.js +++ b/packages/string-templates/src/processors/preprocessor.js @@ -63,7 +63,7 @@ module.exports.processors = [ return statement } } - if (HelperNames().some(option => possibleHelper.includes(option))) { + if (HelperNames().some(option => option.includes(possibleHelper))) { insideStatement = `(${insideStatement})` } return `{{ all ${insideStatement} }}` From 4f2fd656c5f5a7318b50fa3f8cc4dcaa19b21cc8 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 3 Feb 2021 12:10:39 +0000 Subject: [PATCH 2/4] Fixing an issue with the new validity checking being too lenient. --- packages/string-templates/src/index.js | 37 +++++++++++++------ .../string-templates/test/helpers.spec.js | 11 ++++++ 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/packages/string-templates/src/index.js b/packages/string-templates/src/index.js index 6992eb839e..fb5c13be26 100644 --- a/packages/string-templates/src/index.js +++ b/packages/string-templates/src/index.js @@ -83,18 +83,25 @@ module.exports.processObjectSync = (object, context) => { * @returns {string} The enriched string, all templates should have been replaced if they can be. */ module.exports.processStringSync = (string, context) => { + const input = cloneDeep(string) let clonedContext = removeNull(cloneDeep(context)) clonedContext = addConstants(clonedContext) // remove any null/undefined properties if (typeof string !== "string") { throw "Cannot process non-string types." } - string = processors.preprocess(string) - // this does not throw an error when template can't be fulfilled, have to try correct beforehand - const template = hbsInstance.compile(string, { - strict: false, - }) - return processors.postprocess(template(clonedContext)) + try { + string = processors.preprocess(string) + // this does not throw an error when template can't be fulfilled, have to try correct beforehand + const template = hbsInstance.compile(string, { + strict: false, + }) + return processors.postprocess(template(clonedContext)) + } catch (err) { + // suggested that we should always return input if an error occurs, incase string wasn't supposed to + // contain any handlebars statements + return input + } } /** @@ -112,19 +119,27 @@ module.exports.makePropSafe = property => { * @returns {boolean} Whether or not the input string is valid. */ module.exports.isValid = string => { - const specialCases = ["string", "number", "object", "array"] + const validCases = ["string", "number", "object", "array"] + // this is a portion of a specific string always output by handlebars in the case of a syntax error + const invalidCases = [`expecting 'id', 'string', 'number'`] // don't really need a real context to check if its valid const context = {} try { hbsInstance.compile(processors.preprocess(string, false))(context) return true } catch (err) { - const msg = err ? err.message : "" - const foundCase = specialCases.find(spCase => - msg.toLowerCase().includes(spCase) + const msg = err && err.message ? err.message : err + if (!msg) { + return false + } + const invalidCase = invalidCases.some(invalidCase => + msg.toLowerCase().includes(invalidCase) + ) + const validCase = validCases.some(validCase => + msg.toLowerCase().includes(validCase) ) // special case for maths functions - don't have inputs yet - return !!foundCase + return validCase && !invalidCase } } diff --git a/packages/string-templates/test/helpers.spec.js b/packages/string-templates/test/helpers.spec.js index 4a4ed6592a..438a9047c3 100644 --- a/packages/string-templates/test/helpers.spec.js +++ b/packages/string-templates/test/helpers.spec.js @@ -316,4 +316,15 @@ describe("Cover a few complex use cases", () => { const validity = isValid("{{ subtract [c390c23a7f1b6441c98d2fe2a51248ef3].[total profit] [c390c23a7f1b6441c98d2fe2a51248ef3].[total revenue] }}") expect(validity).toBe(true) }) + + it("should confirm an invalid string", () => { + const validity = isValid("{{ awdd () ") + expect(validity).toBe(false) + }) + + it("input a garbage string, expect it to be returned", async () => { + const input = `{{{{{{ } {{ ]] ] ] }}} {{ ] {{ { } { dsa { dddddd }}}}}}} }DDD` + const output = await processString(input, {}) + expect(output).toBe(input) + }) }) \ No newline at end of file From 9aaf6b4883c53fc18260e333a0182f7d9e496211 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 3 Feb 2021 12:38:06 +0000 Subject: [PATCH 3/4] Some more fixes, getting a balance of validity checking, not letting package output anything non-sensical. --- packages/string-templates/src/index.js | 26 ++++++++----------- .../string-templates/test/helpers.spec.js | 9 ++++--- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/packages/string-templates/src/index.js b/packages/string-templates/src/index.js index fb5c13be26..480ed6825f 100644 --- a/packages/string-templates/src/index.js +++ b/packages/string-templates/src/index.js @@ -83,25 +83,21 @@ module.exports.processObjectSync = (object, context) => { * @returns {string} The enriched string, all templates should have been replaced if they can be. */ module.exports.processStringSync = (string, context) => { - const input = cloneDeep(string) + if (!exports.isValid(string)) { + return string + } let clonedContext = removeNull(cloneDeep(context)) clonedContext = addConstants(clonedContext) // remove any null/undefined properties if (typeof string !== "string") { throw "Cannot process non-string types." } - try { - string = processors.preprocess(string) - // this does not throw an error when template can't be fulfilled, have to try correct beforehand - const template = hbsInstance.compile(string, { - strict: false, - }) - return processors.postprocess(template(clonedContext)) - } catch (err) { - // suggested that we should always return input if an error occurs, incase string wasn't supposed to - // contain any handlebars statements - return input - } + string = processors.preprocess(string) + // this does not throw an error when template can't be fulfilled, have to try correct beforehand + const template = hbsInstance.compile(string, { + strict: false, + }) + return processors.postprocess(template(clonedContext)) } /** @@ -119,9 +115,9 @@ module.exports.makePropSafe = property => { * @returns {boolean} Whether or not the input string is valid. */ module.exports.isValid = string => { - const validCases = ["string", "number", "object", "array"] + const validCases = ["string", "number", "object", "array", "cannot read property"] // this is a portion of a specific string always output by handlebars in the case of a syntax error - const invalidCases = [`expecting 'id', 'string', 'number'`] + const invalidCases = [`expecting '`] // don't really need a real context to check if its valid const context = {} try { diff --git a/packages/string-templates/test/helpers.spec.js b/packages/string-templates/test/helpers.spec.js index 438a9047c3..cd659fcc78 100644 --- a/packages/string-templates/test/helpers.spec.js +++ b/packages/string-templates/test/helpers.spec.js @@ -317,9 +317,12 @@ describe("Cover a few complex use cases", () => { expect(validity).toBe(true) }) - it("should confirm an invalid string", () => { - const validity = isValid("{{ awdd () ") - expect(validity).toBe(false) + it("should confirm a bunch of invalid strings", () => { + const invalids = ["{{ awd )", "{{ awdd () ", "{{ awdwad ", "{{ awddawd }"] + for (let invalid of invalids) { + const validity = isValid(invalid) + expect(validity).toBe(false) + } }) it("input a garbage string, expect it to be returned", async () => { From febad5ad9d39ab96e0ed12eb4ebc859d0174c084 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 3 Feb 2021 13:04:19 +0000 Subject: [PATCH 4/4] Adding some more changes to make it more obvious when a binding hasn't worked. --- packages/string-templates/src/index.js | 20 +++++++++++++------- packages/string-templates/src/utilities.js | 13 +++++++++++++ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/packages/string-templates/src/index.js b/packages/string-templates/src/index.js index 480ed6825f..e7d9b9d7e7 100644 --- a/packages/string-templates/src/index.js +++ b/packages/string-templates/src/index.js @@ -2,7 +2,7 @@ const handlebars = require("handlebars") const { registerAll } = require("./helpers/index") const processors = require("./processors") const { cloneDeep } = require("lodash/fp") -const { removeNull, addConstants } = require("./utilities") +const { removeNull, addConstants, removeHandlebarsStatements } = require("./utilities") const manifest = require("../manifest.json") const hbsInstance = handlebars.create() @@ -86,18 +86,24 @@ module.exports.processStringSync = (string, context) => { if (!exports.isValid(string)) { return string } + // take a copy of input incase error + const input = string let clonedContext = removeNull(cloneDeep(context)) clonedContext = addConstants(clonedContext) // remove any null/undefined properties if (typeof string !== "string") { throw "Cannot process non-string types." } - string = processors.preprocess(string) - // this does not throw an error when template can't be fulfilled, have to try correct beforehand - const template = hbsInstance.compile(string, { - strict: false, - }) - return processors.postprocess(template(clonedContext)) + try { + string = processors.preprocess(string) + // this does not throw an error when template can't be fulfilled, have to try correct beforehand + const template = hbsInstance.compile(string, { + strict: false, + }) + return processors.postprocess(template(clonedContext)) + } catch (err) { + return removeHandlebarsStatements(input) + } } /** diff --git a/packages/string-templates/src/utilities.js b/packages/string-templates/src/utilities.js index da3aa6ee94..b33e8bfe1d 100644 --- a/packages/string-templates/src/utilities.js +++ b/packages/string-templates/src/utilities.js @@ -32,3 +32,16 @@ module.exports.addConstants = obj => { } return obj } + +module.exports.removeHandlebarsStatements = string => { + let regexp = new RegExp(exports.FIND_HBS_REGEX) + let matches = string.match(regexp) + if (matches == null) { + return string + } + for (let match of matches) { + const idx = string.indexOf(match) + string = exports.swapStrings(string, idx, match.length, "Invalid Binding") + } + return string +}