History | Log In     View a printable version of the current page.  
Issue Details (XML | Word | Printable)

Key: QUARTZ-574
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: James House
Reporter: Nick Menere
Votes: 0
Watchers: 1
Operations

If you were logged in you would be able to see more operations.
Quartz Scheduler

CronExpression.storeExpressionVals miscalculates month number for invalid month names

Created: 13/Apr/07 02:18 AM   Updated: 18/Apr/07 12:56 PM
Component/s: Triggers
Affects Version/s: 1.4.5
Fix Version/s: 1.6.1

File Attachments: 1. Text File QUARTZ-574.patch (2 kb)



 Description  « Hide
In CronExpression.storeExpressionVals, it tries to calculate month number in the follow snippet:


            if (type == MONTH) {
                sval = getMonthNumber(sub) + 1;
                if (sval < 0) {
                    throw new ParseException("Invalid Month value: '" + sub + "'", i);
                }
                if (s.length() > i + 3) {
                    c = s.charAt(i + 3);
                    if (c == '-') {
                        i += 4;
                        sub = s.substring(i, i + 3);
                        eval = getMonthNumber(sub) + 1;
                        if (eval < 0) {
                            throw new ParseException("Invalid Month value: '" + sub + "'", i);
                        }
                    }
                }
            }


The method getMonthNumber returns -1 for invalid values.
1 is added to the returned value and then a check for less than zero is done. This will never be less than zero.

This is not critical as it fails later with the exception: "Month values must be between 1 and 12"
Though the other message is more appropriate.

Prior to splitting out of the CronExpression, this fails in CronTrigger.java


 All   Comments   Change History      Sort Order:
Henri Yandell - [13/Apr/07 11:11 AM ]
Seems like a simple fix; < 0 to == 0. I'll work on a unit test (unless Nick does one first :) ).

Henri Yandell - [18/Apr/07 12:54 PM ]
Test/Fix attached. The comment that this is not critical as an exception is thrown anyway is only true when its a single month. If the illegal month name is the latter part of a range (Jan-Foo) then no Exception was being thrown.

Henri Yandell - [18/Apr/07 12:56 PM ]
To the branch:

svn ci -m "Fixing QUARTZ-574 by switching the < 0 checks on getMonthNumber to <= 0. This means that the r
ight Exception will be thrown for a month of FOO, and an Exception will now be thrown for a range with an illegal end month, ie) JAN-FOO" src/
Sending src/java/org/quartz/CronExpression.java
Sending src/test/java/org/quartz/CronExpressionTest.java
Transmitting file data ..
Committed revision 695.

And to trunk:

svn ci -m "Fixing QUARTZ-574 by switching the < 0 checks on getMonthNumber to <= 0. This means that the r
ight Exception will be thrown for a month of FOO, and an Exception will now be thrown for a range with an illegal end month, ie) JAN-FOO" src/
Sending src/java/org/quartz/CronExpression.java
Sending src/test/java/org/quartz/CronExpressionTest.java
Transmitting file data ..
Committed revision 696.