Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lack of minute should not pass the validation #244

Open
lvqq opened this issue Sep 22, 2021 · 3 comments
Open

Lack of minute should not pass the validation #244

lvqq opened this issue Sep 22, 2021 · 3 comments

Comments

@lvqq
Copy link

lvqq commented Sep 22, 2021

  • cron-parser version: 4.0.0
  • node: 14.16.0

as the title, when i use expression like 20 15 * *, it pass the validation and becomes * 20 15 * * by default, i think the expression without minute should not pass the validation because minute is not optional but required.

also, the expression 20 15 * * does not pass the validation in other package or site like https://crontab.guru/

image

here is the sample, the function parseExpression should throw a error but not:

const parser = require('cron-parser');
try {
  const interval = parser.parseExpression('20 15 * *');
  interval.next();  // 2021-10-15 20:00:00
  interval.next();  // 2021-10-15 20:01:00
  interval.next();  // 2021-10-15 20:02:00
} catch(e) {
  console.log(e);
}

@georgms
Copy link

georgms commented Oct 26, 2021

I've stumbled on this too. Similarly, expressions such as just 1 or 1 2 are considered valid while e.g. https://crontab.guru/ considers them invalid.

@harrisiirak
Copy link
Owner

Currently, validation of the input expression is rather lax, and if there isn't value specified for the field, it fills it with wildcard.

Throwing an error probably would make more sense indeed. Not sure if somebody out there is relying on this "undocumented feature", but changing it to throw an error should be rather trivial.

@lvqq
Copy link
Author

lvqq commented Nov 27, 2021

Maybe it's a good idea to be part of parseExpression function option? To determine wheither there should be filled with wildcard

MichaelLeeHobbs added a commit to MichaelLeeHobbs/cron-parser that referenced this issue Apr 28, 2023
harrisiirak added a commit that referenced this issue Feb 13, 2025
* WIP - Phase 1 of converting to TypeScript is mostly complete

* WIP - Phase 1 of converting to TypeScript is mostly complete

* WIP - refactored compactField into CronFields. Clean up on other files

* WIP: cleanup types

* WIP: cleanup types

* WIP: refactored findSchedule in to multiple smaller functions

* WIP: refactored findSchedule in to multiple smaller functions

* WIP: refactored shiftTimezone on to CronDate

* WIP: refactored file names. Testing install from github

* added prepare script

* updated library packaging

* updated the default export to match the current library

* updated exports for compatible with CommonJS-style require

* attempt 2 at cjs combability

* attempt 3 at cjs compatability

* some clean up

* wip - cleaner support for CJS and ESM

* wip - looks like we still need the index.cjs.js to support cjs even with the separate builds

* wip - added notes, and general cleanup

* wip - added note

* wip - init jest setup/work

* WIP - testing precommit lint

* WIP - testing precommit lint

* wip - testing pre-commit hook

* wip - formatted code

* wip - refactoring test...

* wip - refactoring test...

* wip - refactored CronParser stringify test

* wip - Test conversion complete. Overall coverage is at 91%. Lost lots of coverage on CronParser, not sure why yet. Added a lot of FIXME's for things that don't make sense or aren't needed. Phase 2 basically done. Phase 3 will be a code review.

* wip - types cleanup

* wip - cleanup

* wip - simplified code

* wip - simplified code

* wip - fixes #153 and #299. Adds test for #156

* wip - Adds fix for #156 in the form of toString vs stringify as it'd be very difficult to make stringify work with these expressions

* wip - Adds fix for #244 in the form of strict mode which defaults to false

* wip - working on a fix for 284

* wip - working on a fix for 284

* wip - fixed 284, major cleanup

* wip - 99.5% code coverage and mostly documented

* wip - code analysis clean up

* wip - add documentation

* wip - updated notes

* wip - basically done

* Done for now

* fixed typos

* removed tsx

* removed npx from scripts calling jest

* removed old_typesVersions

* update requires node version

* Remove .run folder from repository

* Remove GPT4 PROMPTS from repository

* cleaned up .gitignore

* cleaned up .npmignore

* cleaned up .eslintrc.cjs

* Update src/CronDate.ts

Co-authored-by: Harri Siirak <[email protected]>

* renamed applyDateOperation and made hoursLength optional

* renamed handleMathOp to invokeDateOperation

* cleaned up predefined expressions

* cleaned up Predefined Expressions

* removed pointless defensive code

* added types RawCronFields and SerializedCronFields

* cleaned up asserts on getRawFields

* fixed test

* cleaned up parse options

* cleaned up handleSingleRange, handleMultipleRanges

* commented compactField

* changed output dir to dist

* cleaned up PredefinedExpressionsEnum

* cleaned up assert

* removed collect coverage from .js

* added prettier as a dev dependencies and added object-curly-spacing to the eslint rules

* Renamed enums using PascalCase

* removed prettier, having eslint and prettier at the same time was a bad experience. WebStorm won't pick up object-curly-spacing or ij_typescript_spaces_within_imports but running eslint --fix will handle it. Future dev's will just have to configure their IDE's run eslint --fix on save or by hotkey, or manually configure their IDE to insert the spaces

* refactored out CronConstants.ts and refactored constraints into the fields classes

* disabled failing test that are allowed by current behavior. It's upto @harrisiirak if he wants correct the behavior to be inline with cron specs and which spec or allow a more flexible standard.

* fixed comment

* Fixed bugs for dual builds which broke the tests. Fixed the tests

* refactored casing on enums

* removed debug

* renamed interfaces and cleaned up some code

* freeze instead of cloning to prevent mutation

* optimized applyDateOperation

* renamed CronFields to CronFieldCollection

* removed archive

* fixed github build based on suggestion from harrisiirak

* this commit is to capture DaysInMonth changes before removing it so we can roll back if we want

* refactored out DaysInMonth enum, we had to do silly things to make it work and it was only used in one place

* cleaned up enum names

* cleaned up unneeded mutation protection

* cleaned up tests

* cleaned up var naming

* clean up old todos

* cleaned up unneeded mutation protection

* added documentation for strict mode

* cleaned up the project, 100% coverage, fixed broken test, replaced spread anti-mutation with Object.freeze

* clean up

* remove unsupported Node.js versions from the build matrix.

* removed notes

* removed redundant utc option

* cleaned up comments

* renamed DayOfTheMonthRange to DayOfMonthRange

* renamed DayOfTheWeekRange to DayOfWeekRange

* renamed CronFieldTypes to CronFieldType

* Update src/wrapper-index.cjs

Co-authored-by: Harri Siirak <[email protected]>

* removed ESM support and updated documentation

* Update src/fields/CronMonth.ts

Co-authored-by: Harri Siirak <[email protected]>

* Update src/fields/CronMonth.ts

Co-authored-by: Harri Siirak <[email protected]>

* perf(npm): remove superfluous files from npm package (#328)

* perf(npm): remove superfluous files from npm package

* test: fix TS tests to point to the right file

* Bump package version

* Update package-lock.json

* Add +x flag to .husky/pre-commit

* Update files in package.json

* Use CronUnit enum directly where possible

* Add parseInt radix

* Remove docs from repo

* Array index access formatting

* Rename CronDayOfTheWeek to CronDayOfWeek

* Cleanup CronFields tests

* Remove unnecessary assertion

* Remove unused code path

* Rename test cases

* Change assertion

* Remove Object.freeze usage

* Remove assertions that can create possible performance issues

* Upgrade luxon

* Update dev deps

* Update dev deps

* Ingore duplicate enum values

* Update import order

* Import field classes directly and remove CronFieldCollection module re-exports

* Remove duplicate options interface

* Simplify CronFieldCollection usage

* Remove package.cjs.json

* Remove invalid target from ignorePatterns

* Change allowJs to false

* Update LICENSE

* Include Node.js 20 in build workflow

* Iterators cleanup. Implement E6 compatible iterator for CronExpression class

* Cleanup CronExpression tests

* Cleanup CronDate tests

* Cleanup CronParser tests

* Cleanup CronFields tests

* Cleanup CronExpressionParser tests

* More tests cleanup

* Bump luxon to v3.5.0

* Apply minor performance improvements to findSchedule loop

* Optimize unit add/subtract logic in CronDate

* Refactor types and decouple CronExpression from CronExpressionParser to avoid any internal circular dependencies

* Upgrade eslint deps and config format

* Fix index exports and add missing CronExpressionParser export

* Add CronFieldCollection.from method to simplify interval fields modifications

* Update README.md with relevant changes

* Refactor CronParser class and split file parsing related logic into CronFileParser class

* Remove applyDateOperation check

* Ignore loop guard in coverage report

* Update husky usage

* Remove unnecessary startOf calls for second add/subtract in CronDate

* Add benchmarking tooling

* Bump min Node.js version requirement

* Add documentation generation and publishing GH Actions workflow

* Remove component.json

* Add .npmignore

* Update tsconfig.json

* Add prettier for code linting/formatting

* Reformat files

* Use test instead it

* Update package.json

---------

Co-authored-by: Michael Hobbs <[email protected]>
Co-authored-by: Pierre Cavin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants