GothenburgBitFactory / taskwarrior

Taskwarrior - Command line Task Management
https://taskwarrior.org
MIT License
4.44k stars 303 forks source link

Some invalid datetimes are not detected early enough #2996

Open djmitche opened 1 year ago

djmitche commented 1 year ago

./test/datetime-negative.t tests that a bunch of invalid dates result in errors. The tests pass, but many of them only by accident. Consider

⸩ task add due:-12:12:12+01:00 foo
'-1668769932' is not a valid date in the '' format.

this is, indeed, an error, as the test expects. However:

It turns out that there are two places where an identical "is not a valid date in the" is thrown:

many of the cases in this test script are caught in the first spot, which is the expectation. However, when the input evaluates as an arithmetic expression and the result is not 0, ColTypeDate.cpp lets it pass. This date is then added to the new task, and only when that task is written to the backlog, via Task::composeJSON, is the error thrown.

So, I think we need to do a better job of validating the input in ColTypeDate.cpp, preferably by calling the Datetime constructor to raise the exception in only that one spot. However, I don't understand the Variant implementation well enough to know how to do that. Can someone help me out?

djmitche commented 1 year ago

From some discussion, it seems that things like -12:12:12 should not parse as datestamps at all (so, should return a variant with _date == 0), in which case ColTypeDate would reject the input as the tests try to assert.

I skipped these tests for now because what is currently catching the errors (Task::composeJSON) is being removed with the conversion to taskchampion.