From 70b42e3a7570b758adef45a9d1a704ebd6f6600b Mon Sep 17 00:00:00 2001 From: Naz Date: Tue, 10 Nov 2020 13:33:01 +1300 Subject: [PATCH] Switched cron validation library to cron-validate no issue - Previous library was relyting on try/catch block to check if the expression is valid. Flow control through error catching is not considered a good practice and can effect performance (https://riptutorial.com/javascript/example/5297/avoid-try-catch-in-performance-critical-functions) --- ghost/job-manager/lib/is-cron-expression.js | 17 ++++++++--------- ghost/job-manager/package.json | 2 +- .../job-manager/test/is-cron-expression.test.js | 4 ++-- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/ghost/job-manager/lib/is-cron-expression.js b/ghost/job-manager/lib/is-cron-expression.js index ff7d6d79a8..052bc615bd 100644 --- a/ghost/job-manager/lib/is-cron-expression.js +++ b/ghost/job-manager/lib/is-cron-expression.js @@ -1,7 +1,10 @@ -const parser = require('cron-parser'); +const cronValidate = require('cron-validate'); /** - * Checks if expression follows supported CRON format as follows + * Checks if expression follows supported crontab format + * reference: https://www.adminschoice.com/crontab-quick-reference + * builder: https://crontab.guru/ + * * e.g.: * "2 * * * *" where: * @@ -15,18 +18,14 @@ const parser = require('cron-parser'); * │ └──────────────────── minute (0 - 59) * └───────────────────────── second (0 - 59, optional) * - * @param {String} expression in CRON format + * @param {String} expression in crontab format (https://www.gnu.org/software/mcron/manual/html_node/Crontab-file.html) * * @returns {boolean} wheather or not the expression is valid */ const isCronExpression = (expression) => { - try { - parser.parseExpression(expression); + let cronResult = cronValidate(expression); - return true; - } catch (err) { - return false; - } + return cronResult.isValid(); }; module.exports = isCronExpression; diff --git a/ghost/job-manager/package.json b/ghost/job-manager/package.json index acf9093ae3..961785456a 100644 --- a/ghost/job-manager/package.json +++ b/ghost/job-manager/package.json @@ -25,7 +25,7 @@ }, "dependencies": { "@breejs/later": "4.0.2", - "cron-parser": "2.17.0", + "cron-validate": "1.4.0", "fastq": "1.9.0", "p-wait-for": "3.1.0" } diff --git a/ghost/job-manager/test/is-cron-expression.test.js b/ghost/job-manager/test/is-cron-expression.test.js index 08a467277d..dc8e3d6ac7 100644 --- a/ghost/job-manager/test/is-cron-expression.test.js +++ b/ghost/job-manager/test/is-cron-expression.test.js @@ -8,12 +8,12 @@ describe('Is cron expression', function () { it('valid cron expressions', function () { should(isCronExpression('* * * * *')).be.true(); should(isCronExpression('1 * * * *')).be.true(); - should(isCronExpression('* * 12-15 * * *'), 'Range should be 0-23').be.true(); + should(isCronExpression('* 13-23 * * *'), 'Range should be 0-23').be.true(); }); it('invalid cron expressions', function () { should(isCronExpression('123 * * * *')).not.be.true(); should(isCronExpression('a * * * *')).not.be.true(); - should(isCronExpression('* * 12-36 * * *'), 'Invalid range should be 0-23').not.be.true(); + should(isCronExpression('* 13-24 * * *'), 'Invalid range should be 0-23').not.be.true(); }); });