Issue #31: Fix cron parsing accepting incorrect values

xrutayisire · Sep 3, 2022 · 2e89cd1 · 2e89cd1
1 parent 3c74dbc
commit 2e89cd1
Show file tree

Hide file tree

Showing 3 changed files with 173 additions and 9 deletions.
diff --git a/src/converter.ts b/src/converter.ts
@@ -1,7 +1,7 @@
 import { MutableRefObject } from 'react'
 
 import { UNITS, SUPPORTED_SHORTCUTS } from './constants'
-import { range, sort, dedup, setError } from './utils'
+import { range, sort, dedup, setError, convertStringToNumber } from './utils'
 import {
   Unit,
   PeriodType,
@@ -338,10 +338,6 @@ function parsePartString(str: string, unit: Unit) {
             const step = parseStep(right, unit)
             const intervalValues = applyInterval(parsedValues, step)
 
-            if (!intervalValues.length) {
-              throw new Error(`Empty interval value "${str}" for ${unit.type}`)
-            }
-
             return intervalValues
           })
           .flat(),
@@ -403,16 +399,20 @@ function parseRange(rangeStr: string, context: string, unit: Unit) {
   const subparts = rangeStr.split('-')
 
   if (subparts.length === 1) {
-    const value = parseInt(subparts[0], 10)
+    const value = convertStringToNumber(subparts[0])
 
     if (isNaN(value)) {
       throw new Error(`Invalid value "${context}" for ${unit.type}`)
     }
 
     return [value]
   } else if (subparts.length === 2) {
-    const minValue = parseInt(subparts[0], 10)
-    const maxValue = parseInt(subparts[1], 10)
+    const minValue = convertStringToNumber(subparts[0])
+    const maxValue = convertStringToNumber(subparts[1])
+
+    if (isNaN(minValue) || isNaN(maxValue)) {
+      throw new Error(`Invalid value "${context}" for ${unit.type}`)
+    }
 
     // Fix to allow equal min and max range values
     // cf: https://github.com/roccivic/cron-converter/pull/15
@@ -449,7 +449,7 @@ function outOfRange(values: number[], unit: Unit) {
  */
 function parseStep(step: string, unit: Unit) {
   if (typeof step !== 'undefined') {
-    const parsedStep = parseInt(step, 10)
+    const parsedStep = convertStringToNumber(step)
 
     if (isNaN(parsedStep) || parsedStep < 1) {
       throw new Error(`Invalid interval step value "${step}" for ${unit.type}`)

diff --git a/src/tests/Cron.defaultValue.test.tsx b/src/tests/Cron.defaultValue.test.tsx
@@ -554,6 +554,160 @@ describe('Cron defaultValue test suite', () => {
       hoursSelect: '2,4,6,19,21,23',
       minutesSelect: 'every minute',
     },
+    {
+      title: 'wrong end of value with text throw an error',
+      defaultValue: '1-4/2crash * * * *',
+      periodSelect: 'day',
+      monthsSelect: undefined,
+      monthDaysSelect: undefined,
+      weekDaysSelect: undefined,
+      hoursSelect: 'every hour',
+      minutesSelect: 'every minute',
+      error: defaultError,
+    },
+    {
+      title: 'wrong end of value with # throw an error',
+      defaultValue: '* 1#3 * * *',
+      periodSelect: 'day',
+      monthsSelect: undefined,
+      monthDaysSelect: undefined,
+      weekDaysSelect: undefined,
+      hoursSelect: 'every hour',
+      minutesSelect: 'every minute',
+      error: defaultError,
+    },
+    {
+      title: 'wrong end of value with # and week-day throw an error',
+      defaultValue: '* * * * Sun#3',
+      periodSelect: 'day',
+      monthsSelect: undefined,
+      monthDaysSelect: undefined,
+      weekDaysSelect: undefined,
+      hoursSelect: 'every hour',
+      minutesSelect: 'every minute',
+      error: defaultError,
+    },
+    {
+      title: 'wrong end of multiple values with # throw an error',
+      defaultValue: '* * * 1,4#3 *',
+      periodSelect: 'day',
+      monthsSelect: undefined,
+      monthDaysSelect: undefined,
+      weekDaysSelect: undefined,
+      hoursSelect: 'every hour',
+      minutesSelect: 'every minute',
+      error: defaultError,
+    },
+    {
+      title: 'wrong value with # in the middle throw an error',
+      defaultValue: '* 1#2 * * *',
+      periodSelect: 'day',
+      monthsSelect: undefined,
+      monthDaysSelect: undefined,
+      weekDaysSelect: undefined,
+      hoursSelect: 'every hour',
+      minutesSelect: 'every minute',
+      error: defaultError,
+    },
+    {
+      title: 'wrong value with text in the middle throw an error',
+      defaultValue: '1crash5 * * * *',
+      periodSelect: 'day',
+      monthsSelect: undefined,
+      monthDaysSelect: undefined,
+      weekDaysSelect: undefined,
+      hoursSelect: 'every hour',
+      minutesSelect: 'every minute',
+      error: defaultError,
+    },
+    {
+      title: 'wrong null value throw an error',
+      defaultValue: 'null * * * *',
+      periodSelect: 'day',
+      monthsSelect: undefined,
+      monthDaysSelect: undefined,
+      weekDaysSelect: undefined,
+      hoursSelect: 'every hour',
+      minutesSelect: 'every minute',
+      error: defaultError,
+    },
+    {
+      title: 'wrong empty value throw an error',
+      defaultValue: '1,,2 * * * *',
+      periodSelect: 'day',
+      monthsSelect: undefined,
+      monthDaysSelect: undefined,
+      weekDaysSelect: undefined,
+      hoursSelect: 'every hour',
+      minutesSelect: 'every minute',
+      error: defaultError,
+    },
+    {
+      title: 'wrong interval value throw an error',
+      defaultValue: '1-/4 * * * *',
+      periodSelect: 'day',
+      monthsSelect: undefined,
+      monthDaysSelect: undefined,
+      weekDaysSelect: undefined,
+      hoursSelect: 'every hour',
+      minutesSelect: 'every minute',
+      error: defaultError,
+    },
+    {
+      title: 'wrong false value throw an error',
+      defaultValue: 'false * * * *',
+      periodSelect: 'day',
+      monthsSelect: undefined,
+      monthDaysSelect: undefined,
+      weekDaysSelect: undefined,
+      hoursSelect: 'every hour',
+      minutesSelect: 'every minute',
+      error: defaultError,
+    },
+    {
+      title: 'wrong true value throw an error',
+      defaultValue: 'true * * * *',
+      periodSelect: 'day',
+      monthsSelect: undefined,
+      monthDaysSelect: undefined,
+      weekDaysSelect: undefined,
+      hoursSelect: 'every hour',
+      minutesSelect: 'every minute',
+      error: defaultError,
+    },
+    {
+      title: 'wrong number with e value throw an error',
+      defaultValue: '2e1 * * * *',
+      periodSelect: 'day',
+      monthsSelect: undefined,
+      monthDaysSelect: undefined,
+      weekDaysSelect: undefined,
+      hoursSelect: 'every hour',
+      minutesSelect: 'every minute',
+      error: defaultError,
+    },
+    {
+      title: 'wrong number with x value throw an error',
+      defaultValue: '0xF * * * *',
+      periodSelect: 'day',
+      monthsSelect: undefined,
+      monthDaysSelect: undefined,
+      weekDaysSelect: undefined,
+      hoursSelect: 'every hour',
+      minutesSelect: 'every minute',
+      error: defaultError,
+    },
+    {
+      title: 'leading 0 in value is accepted',
+      defaultValue: '010 * * * *',
+      expectedValue: '10 * * * *',
+      periodSelect: 'hour',
+      monthsSelect: undefined,
+      monthDaysSelect: undefined,
+      weekDaysSelect: undefined,
+      hoursSelect: undefined,
+      minutesSelect: '10',
+    },
   ]
 
   test.each(cases)(

diff --git a/src/utils.ts b/src/utils.ts
@@ -76,3 +76,13 @@ export function usePrevious(value: any) {
 
   return ref.current
 }
+
+/**
+ * Convert a string to number but fail if not valid for cron
+ */
+export function convertStringToNumber(str: string) {
+  const parseIntValue = parseInt(str, 10)
+  const numberValue = Number(str)
+
+  return parseIntValue === numberValue ? numberValue : NaN
+}