Test for unusual task data (#3770)

* Test for unusual task data

A task might have any combination of keys and values, but Taskwarrior
often assumes that only valid values can occur, and crashes otherwise.
This is highly inconvenient, as it's often impossible to do anything
with the invalid task -- Taskwarrior just fails without modifying it.

So, this is the beginning of some testing for such invalid tasks, with
the goal of making Taskwarrior due something reasonable. In general, an
invalid attribute value is treated as if it was not set. This is not
exhaustive, and there are likely still bugs of this sort, but as we find
them we can fix and add regression tests to this script.

This introduces a new test-only binary that creates a "bare" task using
TaskChampion, avoiding Taskwarrior's efforts to not create "unusual"
tasks.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

src/TDB2.cpp

@@ -57,10 +57,6 @@ void TDB2::open_replica(const std::string& location, bool create_if_missing) {
 ////////////////////////////////////////////////////////////////////////////////
 // Add the new task to the replica.
 void TDB2::add(Task& task) {
-  // Validate a task for addition. This is stricter than `task.validate`, as any
-  // inconsistency is probably user error.
-  task.validate_add();
-
   // Ensure the task is consistent, and provide defaults if necessary.
   // bool argument to validate() is "applyDefault", to apply default values for
   // properties not otherwise given.

src/Task.cpp

@@ -321,8 +321,8 @@ void Task::setStatus(Task::status status) {
 ////////////////////////////////////////////////////////////////////////////////
 // Determines status of a date attribute.
 Task::dateState Task::getDateState(const std::string& name) const {
-  std::string value = get(name);
-  if (value.length()) {
+  time_t value = get_date(name);
+  if (value > 0) {
     Datetime reference(value);
     Datetime now;
     Datetime today("today");
@@ -804,13 +804,16 @@ std::string Task::composeJSON(bool decorate /*= false*/) const {
 
     // Date fields are written as ISO 8601.
     if (type == "date") {
-      Datetime d(i.second);
-      out << '"' << (i.first == "modification" ? "modified" : i.first)
-          << "\":\""
-          // Date was deleted, do not export parsed empty string
-          << (i.second == "" ? "" : d.toISO()) << '"';
+      time_t epoch = get_date(i.first);
+      if (epoch != 0) {
+        Datetime d(i.second);
+        out << '"' << (i.first == "modification" ? "modified" : i.first)
+            << "\":\""
+            // Date was deleted, do not export parsed empty string
+            << (i.second == "" ? "" : d.toISO()) << '"';
 
-      ++attributes_written;
+        ++attributes_written;
+      }
     }
 
     /*

src/commands/CmdAdd.cpp

@@ -56,6 +56,11 @@ int CmdAdd::execute(std::string& output) {
   // the task is empty, but DOM references can refer to earlier parts of the
   // command line, e.g., `task add due:20110101 wait:due`.
   task.modify(Task::modReplace, true);
+
+  // Validate a task for addition. This is stricter than `task.validate`, as any
+  // inconsistency is probably user error.
+  task.validate_add();
+
   Context::getContext().tdb2.add(task);
 
   // Do not display ID 0, users cannot query by that

src/commands/CmdInfo.cpp

@@ -120,9 +120,11 @@ int CmdInfo::execute(std::string& output) {
       description += '\n' + std::string(indent, ' ') +
                      Datetime(anno.first.substr(11)).toString(dateformatanno) + ' ' + anno.second;
 
-    row = view.addRow();
-    view.set(row, 0, "Description");
-    view.set(row, 1, description, c);
+    if (task.has("description")) {
+      row = view.addRow();
+      view.set(row, 0, "Description");
+      view.set(row, 1, description, c);
+    }
 
     // status
     row = view.addRow();
@@ -215,64 +217,76 @@ int CmdInfo::execute(std::string& output) {
     }
 
     // entry
-    row = view.addRow();
-    view.set(row, 0, "Entered");
-    Datetime dt(task.get_date("entry"));
-    std::string entry = dt.toString(dateformat);
+    if (task.has("entry") && task.get_date("entry")) {
+      row = view.addRow();
+      view.set(row, 0, "Entered");
+      Datetime dt(task.get_date("entry"));
+      std::string entry = dt.toString(dateformat);
 
-    std::string age;
-    auto created = task.get("entry");
-    if (created.length()) {
-      Datetime dt(strtoll(created.c_str(), nullptr, 10));
-      age = Duration(now - dt).formatVague();
+      std::string age;
+      auto created = task.get("entry");
+      if (created.length()) {
+        Datetime dt(strtoll(created.c_str(), nullptr, 10));
+        age = Duration(now - dt).formatVague();
+      }
+
+      view.set(row, 1, entry + " (" + age + ')');
     }
 
-    view.set(row, 1, entry + " (" + age + ')');
+    auto validDate = [&](const char* prop) {
+      if (!task.has(prop)) {
+        return false;
+      }
+      if (task.get_date(prop) == 0) {
+        return false;
+      }
+      return true;
+    };
 
     // wait
-    if (task.has("wait")) {
+    if (validDate("wait")) {
       row = view.addRow();
       view.set(row, 0, "Waiting until");
       view.set(row, 1, Datetime(task.get_date("wait")).toString(dateformat));
     }
 
     // scheduled
-    if (task.has("scheduled")) {
+    if (validDate("scheduled")) {
       row = view.addRow();
       view.set(row, 0, "Scheduled");
       view.set(row, 1, Datetime(task.get_date("scheduled")).toString(dateformat));
     }
 
     // start
-    if (task.has("start")) {
+    if (validDate("start")) {
       row = view.addRow();
       view.set(row, 0, "Start");
       view.set(row, 1, Datetime(task.get_date("start")).toString(dateformat));
     }
 
     // due (colored)
-    if (task.has("due")) {
+    if (validDate("due")) {
       row = view.addRow();
       view.set(row, 0, "Due");
       view.set(row, 1, Datetime(task.get_date("due")).toString(dateformat));
     }
 
     // end
-    if (task.has("end")) {
+    if (validDate("end")) {
       row = view.addRow();
       view.set(row, 0, "End");
       view.set(row, 1, Datetime(task.get_date("end")).toString(dateformat));
     }
 
     // until
-    if (task.has("until")) {
+    if (validDate("until")) {
       row = view.addRow();
       view.set(row, 0, "Until");
       view.set(row, 1, Datetime(task.get_date("until")).toString(dateformat));
     }
 
     // modified
-    if (task.has("modified")) {
+    if (validDate("modified")) {
       row = view.addRow();
       view.set(row, 0, "Last modified");
 
@@ -632,7 +646,11 @@ std::optional<std::string> CmdInfo::formatForInfo(const std::vector<Operation>&
       } else {
         // Record the last start time for later duration calculation.
         if (prop == "start") {
-          last_start = Datetime(value.value()).toEpoch();
+          try {
+            last_start = Datetime(value.value()).toEpoch();
+          } catch (std::string) {
+            // ignore invalid dates
+          }
         }
 
         out << format("{1} set to '{2}'.", Lexer::ucFirst(prop),

src/commands/CmdModify.cpp

@@ -119,7 +119,7 @@ void CmdModify::checkConsistency(Task &before, Task &after) {
     throw std::string("You cannot remove the recurrence from a recurring task.");
 
   if ((before.getStatus() == Task::pending) && (after.getStatus() == Task::pending) &&
-      (after.get("end") != ""))
+      (before.get("end") == "") && (after.get("end") != ""))
     throw format("Could not modify task {1}. You cannot set an end date on a pending task.",
                  before.identifier(true));
 }

src/feedback.cpp

@@ -51,7 +51,12 @@ std::string renderAttribute(const std::string& name, const std::string& value,
   if (Context::getContext().columns.find(name) != Context::getContext().columns.end()) {
     Column* col = Context::getContext().columns[name];
     if (col && col->type() == "date" && value != "") {
-      Datetime d((time_t)strtoll(value.c_str(), nullptr, 10));
+      int64_t epoch = strtoll(value.c_str(), nullptr, 10);
+      // Do not try to render an un-parseable value.
+      if (epoch == 0) {
+        return value;
+      }
+      Datetime d((time_t)epoch);
       if (format == "") return d.toString(Context::getContext().config.get("dateformat"));
 
       return d.toString(format);

src/recur.cpp

@@ -288,9 +288,11 @@ std::optional<Datetime> getNextRecurrence(Datetime& current, std::string& period
   else if (unicodeLatinDigit(period[0]) && period[period.length() - 1] == 'q') {
     int increment = strtol(period.substr(0, period.length() - 1).c_str(), nullptr, 10);
 
-    if (increment <= 0)
-      throw format("Recurrence period '{1}' is equivalent to {2} and hence invalid.", period,
-                   increment);
+    if (increment <= 0) {
+      Context::getContext().footnote(format(
+          "Recurrence period '{1}' is equivalent to {2} and hence invalid.", period, increment));
+      return std::nullopt;
+    }
 
     m += 3 * increment;
     while (m > 12) {
@@ -346,8 +348,11 @@ std::optional<Datetime> getNextRecurrence(Datetime& current, std::string& period
   // Add the period to current, and we're done.
   std::string::size_type idx = 0;
   Duration p;
-  if (!p.parse(period, idx))
-    throw std::string(format("The recurrence value '{1}' is not valid.", period));
+  if (!p.parse(period, idx)) {
+    Context::getContext().footnote(
+        format("Warning: The recurrence value '{1}' is not valid.", period));
+    return std::nullopt;
+  }
 
   return checked_add_datetime(current, p.toTime_t());
 }

test/CMakeLists.txt

@@ -188,6 +188,7 @@ set (pythonTests
   undo.test.py
   unicode.test.py
   unique.test.py
+  unusual_task.test.py
   upgrade.test.py
   urgency.test.py
   urgency_inherit.test.py
@@ -207,6 +208,15 @@ foreach (python_Test ${pythonTests})
     )
 endforeach(python_Test)
 
+# Create a `make_tc_task` binary, used for unusual_task.test.py. In order to build this
+# for the tests, it's added as a dependency of `test_runner`.
+add_executable (make_tc_task make_tc_task.cpp)
+target_link_libraries (make_tc_task task commands columns libshared task commands columns libshared task commands columns libshared ${TASK_LIBRARIES})
+if (DARWIN)
+  target_link_libraries (make_tc_task "-framework CoreFoundation -framework Security -framework SystemConfiguration")
+endif (DARWIN)
+add_dependencies(test_runner make_tc_task)
+
 # -- Shell tests
 
 set (shell_SRCS

test/basetest/utils.py

@@ -28,8 +28,11 @@ ON_POSIX = "posix" in sys.builtin_module_names
 # Directory relative to basetest module location
 CURRENT_DIR = os.path.dirname(os.path.abspath(__file__))
 
+# From the CMAKE value of the same name. This is substituted at configure.
+CMAKE_BINARY_DIR = os.path.abspath("${CMAKE_BINARY_DIR}")
+
 # Location of binary files (usually the src/ folder)
-BIN_PREFIX = os.path.abspath(os.path.join("${CMAKE_BINARY_DIR}", "src"))
+BIN_PREFIX = os.path.abspath(os.path.join(CMAKE_BINARY_DIR, "src"))
 
 # Default location of test hooks
 DEFAULT_HOOK_PATH = os.path.abspath(

test/make_tc_task.cpp

@@ -0,0 +1,80 @@
+////////////////////////////////////////////////////////////////////////////////
+//
+// Copyright 2025, Dustin J. Mitchell
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+// THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+// SOFTWARE.
+//
+// https://www.opensource.org/licenses/mit-license.php
+//
+////////////////////////////////////////////////////////////////////////////////
+
+#include <cmake.h>
+// cmake.h include header must come first
+
+#include <CmdInfo.h>
+#include <main.h>
+#include <stdlib.h>
+#include <taskchampion-cpp/lib.h>
+#include <test.h>
+#include <util.h>
+
+#include <iostream>
+#include <limits>
+
+#include "format.h"
+
+namespace {
+
+////////////////////////////////////////////////////////////////////////////////
+int usage() {
+  std::cerr << "USAGE: make_tc_task DATADIR KEY=VALUE ..\n";
+  return 1;
+}
+
+}  // namespace
+
+////////////////////////////////////////////////////////////////////////////////
+int main(int argc, char **argv) {
+  if (!--argc) {
+    return usage();
+  }
+  char *datadir = *++argv;
+
+  auto replica = tc::new_replica_on_disk(datadir, true);
+  auto uuid = tc::uuid_v4();
+  auto operations = tc::new_operations();
+  auto task = tc::create_task(uuid, operations);
+
+  while (--argc) {
+    std::string arg = *++argv;
+    size_t eq_idx = arg.find('=');
+    if (eq_idx == std::string::npos) {
+      return usage();
+    }
+    std::string property = arg.substr(0, eq_idx);
+    std::string value = arg.substr(eq_idx + 1);
+    task->update(property, value, operations);
+  }
+  replica->commit_operations(std::move(operations));
+
+  std::cout << static_cast<std::string>(uuid.to_string()) << "\n";
+  return 0;
+}
+
+////////////////////////////////////////////////////////////////////////////////

test/unusual_task.test.py

@@ -0,0 +1,202 @@
+#!/usr/bin/env python3
+###############################################################################
+#
+# Copyright 2025 Dustin J. Mitchell
+#
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included
+# in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+# OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+# SOFTWARE.
+#
+# https://www.opensource.org/licenses/mit-license.php
+#
+###############################################################################
+
+import sys
+import os
+import re
+import time
+import unittest
+
+# Ensure python finds the local simpletap module
+sys.path.append(os.path.dirname(os.path.abspath(__file__)))
+
+from basetest import Task, TestCase
+from basetest.utils import run_cmd_wait, CMAKE_BINARY_DIR
+
+
+class TestUnusualTasks(TestCase):
+    def setUp(self):
+        """Executed before each test in the class"""
+        self.t = Task()
+        self.t.config(
+            "report.custom-report.columns",
+            "id,description,entry,start,end,due,scheduled,modified,until",
+        )
+        self.t.config("verbose", "nothing")
+
+    def make_task(self, **props):
+        make_tc_task = os.path.abspath(
+            os.path.join(CMAKE_BINARY_DIR, "test", "make_tc_task")
+        )
+        cmd = [make_tc_task, self.t.datadir]
+        for p, v in props.items():
+            cmd.append(f"{p}={v}")
+        _, out, _ = run_cmd_wait(cmd)
+        return out.strip()
+
+    def test_empty_task_info(self):
+        uuid = self.make_task()
+        _, out, _ = self.t(f"{uuid} info")
+        self.assertNotIn("Entered", out)
+        self.assertNotIn("Waiting", out)
+        self.assertNotIn("Last modified", out)
+        self.assertNotIn("Start", out)
+        self.assertNotIn("End", out)
+        self.assertNotIn("Due", out)
+        self.assertNotIn("Until", out)
+        self.assertRegex(out, r"Status\s+Pending")
+
+    def test_modify_empty_task(self):
+        uuid = self.make_task()
+        self.t(f"{uuid} modify a description +taggy due:tomorrow")
+        _, out, _ = self.t(f"{uuid} info")
+        self.assertRegex(out, r"Description\s+a description")
+        self.assertRegex(out, r"Tags\s+taggy")
+
+    def test_empty_task_recurring(self):
+        uuid = self.make_task(status="recurring")
+        _, out, _ = self.t(f"{uuid} info")
+        self.assertRegex(out, r"Status\s+Recurring")
+        _, out, _ = self.t(f"{uuid} custom-report")
+
+    def test_recurring_invalid_rtype(self):
+        uuid = self.make_task(
+            status="recurring", due=str(int(time.time())), rtype="occasional"
+        )
+        _, out, _ = self.t(f"{uuid} info")
+        self.assertRegex(out, r"Status\s+Recurring")
+        self.assertRegex(out, r"Recurrence type\s+occasional")
+        _, out, _ = self.t(f"{uuid} custom-report")
+
+    def test_recurring_invalid_recur(self):
+        uuid = self.make_task(
+            status="recurring",
+            due=str(int(time.time())),
+            rtype="periodic",
+            recur="xxxxx",
+        )
+        _, out, _ = self.t(f"{uuid} info")
+        self.assertRegex(out, r"Status\s+Recurring")
+        self.assertRegex(out, r"Recurrence type\s+periodic")
+        _, out, _ = self.t(f"{uuid} custom-report")
+
+    def test_recurring_bad_quarters_rtype(self):
+        uuid = self.make_task(
+            status="recurring", due=str(int(time.time())), rtype="periodic", recur="9aq"
+        )
+        _, out, _ = self.t(f"{uuid} custom-report")
+
+    def test_invalid_entry_info(self):
+        uuid = self.make_task(entry="abcdef")
+        _, out, _ = self.t(f"{uuid} info")
+        self.assertNotIn("Entered", out)
+
+    def test_invalid_modified_info(self):
+        uuid = self.make_task(modified="abcdef")
+        _, out, _ = self.t(f"{uuid} info")
+        self.assertNotIn(r"Last modified", out)
+
+    def test_invalid_start_info(self):
+        uuid = self.make_task(start="abcdef")
+        _, out, _ = self.t(f"{uuid} info")
+
+    def test_invalid_dates_report(self):
+        uuid = self.make_task(
+            wait="wait",
+            scheduled="scheduled",
+            start="start",
+            due="due",
+            end="end",
+            until="until",
+            modified="modified",
+        )
+        _, out, _ = self.t(f"{uuid} custom-report")
+
+    def test_invalid_dates_stop(self):
+        uuid = self.make_task(
+            wait="wait",
+            scheduled="scheduled",
+            start="start",
+            due="due",
+            end="end",
+            until="until",
+            modified="modified",
+        )
+        _, out, _ = self.t(f"{uuid} stop")
+
+    def test_invalid_dates_modify(self):
+        uuid = self.make_task(
+            wait="wait",
+            scheduled="scheduled",
+            start="start",
+            due="due",
+            end="end",
+            until="until",
+            modified="modified",
+        )
+        _, out, _ = self.t(f"{uuid} mod a description +tag")
+
+    def test_invalid_dates_info(self):
+        uuid = self.make_task(
+            wait="wait",
+            scheduled="scheduled",
+            start="start",
+            due="due",
+            end="end",
+            until="until",
+            modified="modified",
+        )
+        _, out, _ = self.t(f"{uuid} info")
+        self.assertNotRegex("^Entered\s+", out)
+        self.assertNotRegex("^Start\s+", out)
+        self.assertIn(r"Wait set to 'wait'", out)
+        self.assertIn(r"Scheduled set to 'scheduled'", out)
+        self.assertIn(r"Start set to 'start'", out)
+        self.assertIn(r"Due set to 'due'", out)
+        self.assertIn(r"End set to 'end'", out)
+        self.assertIn(r"Until set to 'until'", out)
+        # (note that 'modified' is not shown in the journal)
+
+    def test_invalid_dates_export(self):
+        uuid = self.make_task(
+            wait="wait",
+            scheduled="scheduled",
+            start="start",
+            due="due",
+            end="end",
+            until="until",
+            modified="modified",
+        )
+        _, out, _ = self.t(f"{uuid} export")
+
+
+if __name__ == "__main__":
+    from simpletap import TAPTestRunner
+
+    unittest.main(testRunner=TAPTestRunner())
+
+# vim: ai sts=4 et sw=4 ft=python