From b79d6b712b2104c0cdd215e5a3907a5db94bf7d6 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 3 Feb 2021 11:41:33 +0000 Subject: [PATCH 1/8] 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 96acfc6563bea6b9ff65963f545cd29db52ff8ee Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 3 Feb 2021 12:10:39 +0000 Subject: [PATCH 2/8] 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 c10cd53eb607a2dc3eae2072a855711118292ccb Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 3 Feb 2021 12:38:06 +0000 Subject: [PATCH 3/8] 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 ae54a420ba3c5b9e96ccbef4bf4c977c736b9437 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 3 Feb 2021 13:04:19 +0000 Subject: [PATCH 4/8] 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 +} From 9ae53caa4dbabbea2139a34ff8a406d5020059ae Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 3 Feb 2021 13:55:33 +0000 Subject: [PATCH 5/8] Removing the use of helper-date which was causing some problems, instead took what we needed of it out of the package and updated to use dayjs instead of moment. --- packages/string-templates/package.json | 2 +- packages/string-templates/src/helpers/date.js | 76 +++++++++++++++++++ .../string-templates/src/helpers/external.js | 2 +- .../string-templates/test/helpers.spec.js | 8 ++ packages/string-templates/yarn.lock | 5 ++ 5 files changed, 91 insertions(+), 2 deletions(-) create mode 100644 packages/string-templates/src/helpers/date.js diff --git a/packages/string-templates/package.json b/packages/string-templates/package.json index 8bbcccf3fc..dc2bcf7f54 100644 --- a/packages/string-templates/package.json +++ b/packages/string-templates/package.json @@ -15,9 +15,9 @@ }, "dependencies": { "@budibase/handlebars-helpers": "^0.11.3", + "dayjs": "^1.10.4", "handlebars": "^4.7.6", "handlebars-utils": "^1.0.6", - "helper-date": "^1.0.1", "lodash": "^4.17.20" }, "devDependencies": { diff --git a/packages/string-templates/src/helpers/date.js b/packages/string-templates/src/helpers/date.js new file mode 100644 index 0000000000..6751724608 --- /dev/null +++ b/packages/string-templates/src/helpers/date.js @@ -0,0 +1,76 @@ +const dayjs = require("dayjs") + +/** + * This file was largely taken from the helper-date package - we did this for two reasons: + * 1. It made use of both moment of date.js - this caused some weird bugs with some relatively simple + * syntax and didn't offer much in return. + * 2. Replacing moment with dayjs helps massively reduce bundle size. + * The original package can be found here: + * https://github.com/helpers/helper-date + */ + +function isOptions(val) { + return typeof(val) === "object" && typeof(val.hash) === "object" +} + +function isApp(thisArg) { + return typeof(thisArg) === "object" + && typeof(thisArg.options) === "object" + && typeof(thisArg.app) === "object" +} + +function getContext(thisArg, locals, options) { + if (isOptions(thisArg)) { + return getContext({}, locals, thisArg) + } + // ensure args are in the correct order + if (isOptions(locals)) { + return getContext(thisArg, options, locals) + } + const appContext = isApp(thisArg) ? thisArg.context : {} + options = options || {} + + // if "options" is not handlebars options, merge it onto locals + if (!isOptions(options)) { + locals = Object.assign({}, locals, options) + } + // merge handlebars root data onto locals if specified on the hash + if (isOptions(options) && options.hash.root === true) { + locals = Object.assign({}, options.data.root, locals) + } + let context = Object.assign({}, appContext, locals, options.hash) + if (!isApp(thisArg)) { + context = Object.assign({}, thisArg, context) + } + if (isApp(thisArg) && thisArg.view && thisArg.view.data) { + context = Object.assign({}, context, thisArg.view.data) + } + return context +} + +module.exports = function dateHelper(str, pattern, options) { + if (isOptions(pattern)) { + options = pattern + pattern = null + } + + if (isOptions(str)) { + options = str + pattern = null + str = null + } + + // if no args are passed, return a formatted date + if (str == null && pattern == null) { + dayjs.locale("en") + return dayjs().format("MMMM DD, YYYY") + } + + const defaults = {lang: "en", date: new Date(str)} + const opts = getContext(this, defaults, options) + + // set the language to use + dayjs.locale(opts.lang || opts.language) + + return dayjs(new Date(str)).format(pattern) +} \ No newline at end of file diff --git a/packages/string-templates/src/helpers/external.js b/packages/string-templates/src/helpers/external.js index 3256134371..138565889d 100644 --- a/packages/string-templates/src/helpers/external.js +++ b/packages/string-templates/src/helpers/external.js @@ -1,5 +1,5 @@ const helpers = require("@budibase/handlebars-helpers") -const dateHelper = require("helper-date") +const dateHelper = require("./date") const { HelperFunctionBuiltin } = require("./constants") /** diff --git a/packages/string-templates/test/helpers.spec.js b/packages/string-templates/test/helpers.spec.js index cd659fcc78..b8b44a0929 100644 --- a/packages/string-templates/test/helpers.spec.js +++ b/packages/string-templates/test/helpers.spec.js @@ -1,5 +1,6 @@ const { processString, + processObject, isValid, } = require("../src/index") @@ -330,4 +331,11 @@ describe("Cover a few complex use cases", () => { const output = await processString(input, {}) expect(output).toBe(input) }) + + it("getting a nice date from the user", async () => { + const input = {text: `{{ date user.subscriptionDue "DD-MM" }}`} + const context = JSON.parse(`{"user":{"email":"test@test.com","roleId":"ADMIN","type":"user","tableId":"ta_users","subscriptionDue":"2021-01-12T12:00:00.000Z","_id":"ro_ta_users_us_test@test.com","_rev":"2-24cc794985eb54183ecb93e148563f3d"}}`) + const output = await processObject(input, context) + expect(output.text).toBe("12-01") + }) }) \ No newline at end of file diff --git a/packages/string-templates/yarn.lock b/packages/string-templates/yarn.lock index a3d5ac638d..cb96e4e81c 100644 --- a/packages/string-templates/yarn.lock +++ b/packages/string-templates/yarn.lock @@ -1596,6 +1596,11 @@ date.js@^0.3.1: dependencies: debug "~3.1.0" +dayjs@^1.10.4: + version "1.10.4" + resolved "https://registry.yarnpkg.com/dayjs/-/dayjs-1.10.4.tgz#8e544a9b8683f61783f570980a8a80eaf54ab1e2" + integrity sha512-RI/Hh4kqRc1UKLOAf/T5zdMMX5DQIlDxwUe3wSyMMnEbGunnpENCdbUgM+dW7kXidZqCttBrmw7BhN4TMddkCw== + debug@^2.2.0, debug@^2.3.3: version "2.6.9" resolved "https://registry.yarnpkg.com/debug/-/debug-2.6.9.tgz#5d128515df134ff327e90a4c93f4e077a536341f" From 6fd080269b9e03459cfe10dafecd3a9d536d1d7e Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 3 Feb 2021 13:56:01 +0000 Subject: [PATCH 6/8] linting. --- packages/string-templates/src/helpers/date.js | 14 ++++++++------ packages/string-templates/src/index.js | 14 ++++++++++++-- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/packages/string-templates/src/helpers/date.js b/packages/string-templates/src/helpers/date.js index 6751724608..1c929a758f 100644 --- a/packages/string-templates/src/helpers/date.js +++ b/packages/string-templates/src/helpers/date.js @@ -10,13 +10,15 @@ const dayjs = require("dayjs") */ function isOptions(val) { - return typeof(val) === "object" && typeof(val.hash) === "object" + return typeof val === "object" && typeof val.hash === "object" } function isApp(thisArg) { - return typeof(thisArg) === "object" - && typeof(thisArg.options) === "object" - && typeof(thisArg.app) === "object" + return ( + typeof thisArg === "object" && + typeof thisArg.options === "object" && + typeof thisArg.app === "object" + ) } function getContext(thisArg, locals, options) { @@ -66,11 +68,11 @@ module.exports = function dateHelper(str, pattern, options) { return dayjs().format("MMMM DD, YYYY") } - const defaults = {lang: "en", date: new Date(str)} + const defaults = { lang: "en", date: new Date(str) } const opts = getContext(this, defaults, options) // set the language to use dayjs.locale(opts.lang || opts.language) return dayjs(new Date(str)).format(pattern) -} \ No newline at end of file +} diff --git a/packages/string-templates/src/index.js b/packages/string-templates/src/index.js index e7d9b9d7e7..e662f253c6 100644 --- a/packages/string-templates/src/index.js +++ b/packages/string-templates/src/index.js @@ -2,7 +2,11 @@ const handlebars = require("handlebars") const { registerAll } = require("./helpers/index") const processors = require("./processors") const { cloneDeep } = require("lodash/fp") -const { removeNull, addConstants, removeHandlebarsStatements } = require("./utilities") +const { + removeNull, + addConstants, + removeHandlebarsStatements, +} = require("./utilities") const manifest = require("../manifest.json") const hbsInstance = handlebars.create() @@ -121,7 +125,13 @@ module.exports.makePropSafe = property => { * @returns {boolean} Whether or not the input string is valid. */ module.exports.isValid = string => { - const validCases = ["string", "number", "object", "array", "cannot read property"] + 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 '`] // don't really need a real context to check if its valid From 39ea16975269ef715cc8d41b29c4aa352f3502ab Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 3 Feb 2021 14:00:58 +0000 Subject: [PATCH 7/8] Updated script and manifest - minor spelling error. --- packages/string-templates/manifest.json | 4 ++-- packages/string-templates/scripts/gen-collection-info.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/string-templates/manifest.json b/packages/string-templates/manifest.json index 294eeeec5e..af1f4e3d54 100644 --- a/packages/string-templates/manifest.json +++ b/packages/string-templates/manifest.json @@ -1097,8 +1097,8 @@ "format" ], "numArgs": 2, - "example": "{{date now \"YYYY\"}}", - "description": "

Format a date using moment.js data formatting.

\n" + "example": "{{date now \"DD-MM-YYYY\"}}", + "description": "

Format a date using moment.js date formatting.

\n" } } } \ No newline at end of file diff --git a/packages/string-templates/scripts/gen-collection-info.js b/packages/string-templates/scripts/gen-collection-info.js index 413e870519..76b5c89e0c 100644 --- a/packages/string-templates/scripts/gen-collection-info.js +++ b/packages/string-templates/scripts/gen-collection-info.js @@ -139,8 +139,8 @@ function run() { date: { args: ["datetime", "format"], numArgs: 2, - example: '{{date now "YYYY"}}', - description: "Format a date using moment.js data formatting.", + example: '{{date now "DD-MM-YYYY"}}', + description: "Format a date using moment.js date formatting.", }, } // convert all markdown to HTML From f182a65547ea28c4efc80d0de0f6b834b321c146 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 3 Feb 2021 17:19:41 +0000 Subject: [PATCH 8/8] Adding a check to the link utils which should make sure the correct link doc is always picked. --- packages/server/src/db/linkedRows/linkUtils.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/server/src/db/linkedRows/linkUtils.js b/packages/server/src/db/linkedRows/linkUtils.js index 6549f9b61b..cb669cf5c7 100644 --- a/packages/server/src/db/linkedRows/linkUtils.js +++ b/packages/server/src/db/linkedRows/linkUtils.js @@ -84,6 +84,13 @@ exports.getLinkDocuments = async function({ // filter to get unique entries const foundIds = [] linkRows = linkRows.filter(link => { + // make sure anything unique is the correct key + if ( + (tableId && link.key[0] !== tableId) || + (rowId && link.key[1] !== rowId) + ) { + return false + } const unique = foundIds.indexOf(link.id) === -1 if (unique) { foundIds.push(link.id)