Skip to content

Commit

Permalink
fix: fix infinite loop on expressions resolving only inside a DST for…
Browse files Browse the repository at this point in the history
…ward jump (#938)

## Description

Removes a superfluous `+ 1` modifying the behavior unexpectedly. Using
the debugger I've located the bug to this instruction, and it can easily
be confirmed that this `+ 1` is the issue, since `date.month` &
`this.month` are both indexed starting from `1` (and not `0`).

Not sure how long this bug has been around, but it was [already present
when we aligned with the UNIX
standard](https://github.com/kelektiv/node-cron/pull/667/files#diff-c14c2dca8456f15417b39cfbd9758009f8eb4f3a190a415768d6e4ae6ae9dceeL473-L477).

## Related Issue

Fixes #919.

## Motivation and Context

Fixes an infinite loop when a cron expression only resolves to inside a
DST jump. See #919 for more.

## How Has This Been Tested?

Added two new test cases, and [made sure they triggered the infinite
loop](https://github.com/kelektiv/node-cron/actions/runs/12539141178/attempts/1?pr=938)
before writing the fix.

## Screenshots (if appropriate):

## Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)

## Checklist:

- [X] My code follows the code style of this project.
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [X] I have added tests to cover my changes.
- [X] All new and existing tests passed.
- [ ] If my change introduces a breaking change, I have added a `!`
after the type/scope in the title (see the Conventional Commits
standard).
  • Loading branch information
sheerlox authored Dec 30, 2024
1 parent 70c3339 commit efb8df5
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 2 deletions.
2 changes: 1 addition & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"name": "Jest Debug",
"env": { "NODE_ENV": "test" },
"program": "${workspaceFolder}/node_modules/.bin/jest",
"args": [],
"args": ["--runInBand"],
"console": "integratedTerminal",
"windows": {
"program": "${workspaceFolder}/node_modules/jest/bin/jest"
Expand Down
2 changes: 1 addition & 1 deletion src/time.ts
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ export class CronTime {
const beforeJumpingPoint = afterJumpingPoint.minus({ second: 1 });

if (
date.month + 1 in this.month &&
date.month in this.month &&
date.day in this.dayOfMonth &&
this._getWeekDay(date) in this.dayOfWeek
) {
Expand Down
49 changes: 49 additions & 0 deletions tests/cron.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1299,4 +1299,53 @@ describe('cron', () => {
expect(job.running).toBe(false);
});
});

describe('Daylight Saving Time', () => {
// https://github.com/kelektiv/node-cron/issues/919
it('should not get into an infinite loop on Lisbon DST forward jump', () => {
const d = DateTime.fromISO('2024-03-30T00:59:59', {
zone: 'Europe/Lisbon'
}).toJSDate();
const clock = sinon.useFakeTimers(d.getTime());

console.debug({ d });

const job = new CronJob(
'0 1 30 3 *',
callback,
null,
true,
'Europe/Lisbon'
);

clock.tick(1000);
expect(callback).toHaveBeenCalledTimes(1);

clock.restore();
job.stop();
});

it('should not get into an infinite loop on Paris DST forward jump', () => {
const d = DateTime.fromISO('2024-03-31T01:59:59', {
zone: 'Europe/Paris'
}).toJSDate();
const clock = sinon.useFakeTimers(d.getTime());

console.debug({ d });

const job = new CronJob(
'0 2 31 3 *',
callback,
null,
true,
'Europe/Paris'
);

clock.tick(1000);
expect(callback).toHaveBeenCalledTimes(1);

clock.restore();
job.stop();
});
});
});

0 comments on commit efb8df5

Please sign in to comment.