jmrozanec / cron-utils

Cron utils for parsing, validations and human readable descriptions as well as date/time interoperability.
http://cron-utils.com
Apache License 2.0
1.15k stars 262 forks source link

Using L, W, and LW with CronBuilder throws IllegalArgumentException #535

Closed andysenn closed 2 years ago

andysenn commented 2 years ago

We have a project that is currently using version 8.0.0. We are trying to upgrade to address a couple of CVEs, but it looks like newer versions (starting around 8.1.1) no longer support L, W, and LW with CronBuilder.

We've reviewed issue https://github.com/jmrozanec/cron-utils/issues/501, which sounded like it was going to address this, but we're still running into issues. We've tested this with versions 8.1.1, 9.1.8, and 9.2.0.

The following are some example test cases that all pass on version 8.0.0, but all fail on the newer versions:

import org.junit.Test;

import com.cronutils.builder.CronBuilder;
import com.cronutils.model.CronType;
import com.cronutils.model.definition.CronDefinitionBuilder;
import com.cronutils.model.field.expression.FieldExpressionFactory;
import com.cronutils.model.field.value.SpecialChar;

public class CronUtilsTests {

    @Test
    public void testDayOfWeek_L() {
        CronBuilder.cron(CronDefinitionBuilder.instanceDefinitionFor(CronType.SPRING))
                .withDoM(FieldExpressionFactory.questionMark())
                .withDoW(FieldExpressionFactory.on(1, SpecialChar.L))
                .instance().asString();
    }

    @Test
    public void testDayOfMonth_L() {
        CronBuilder.cron(CronDefinitionBuilder.instanceDefinitionFor(CronType.SPRING))
                .withDoM(FieldExpressionFactory.on(SpecialChar.L))
                .withDoW(FieldExpressionFactory.questionMark())
                .instance().asString();
    }

    @Test
    public void testDayOfMonth_W() {
        CronBuilder.cron(CronDefinitionBuilder.instanceDefinitionFor(CronType.SPRING))
                .withDoM(FieldExpressionFactory.on(1, SpecialChar.W))
                .withDoW(FieldExpressionFactory.questionMark())
                .instance().asString();
    }

    @Test
    public void testDayOfMonth_LW() {
        CronBuilder.cron(CronDefinitionBuilder.instanceDefinitionFor(CronType.SPRING))
                .withDoM(FieldExpressionFactory.on(SpecialChar.LW))
                .withDoW(FieldExpressionFactory.questionMark())
                .instance().asString();
    }

}

These tests all fail with a message similar to the following:

java.lang.IllegalArgumentException: Invalid chars in expression! Expression: L Invalid chars: L

    at com.cronutils.model.field.expression.visitor.ValidationFieldExpressionVisitor.checkUnsupportedChars(ValidationFieldExpressionVisitor.java:46)
    at com.cronutils.model.field.expression.visitor.ValidationFieldExpressionVisitor.visit(ValidationFieldExpressionVisitor.java:93)
    at com.cronutils.model.field.expression.visitor.ValidationFieldExpressionVisitor.visit(ValidationFieldExpressionVisitor.java:25)
    at com.cronutils.model.field.expression.On.accept(On.java:70)
    at com.cronutils.builder.CronBuilder.addField(CronBuilder.java:226)
    at com.cronutils.builder.CronBuilder.withDoM(CronBuilder.java:59)
    [...]
jmrozanec commented 2 years ago

@andysenn, may we ask for a PR with the abovementioned tests? Thanks! 😄

andysenn commented 2 years ago

@jmrozanec - I apologize. After looking at the code a bit more, I see that CronType.SPRING53 was added in version 9.2.0. If I use this, then it works. Since, at least from my understanding, those characters weren't supported prior to Spring 5.3, I don't think this is an issue.

Thank you for your time and quick response. This can be closed.

jmrozanec commented 2 years ago

@andysenn, we are glad to hear that! 😄