Fix and test handling of backslashes in hooks (#3909)

* Update hooks to use `read -r` and `printf`

This portably avoids any interpretation of backslash escapes by the
shell.

* Support running make_tc_task elsewhere

* Add a test for backslashes in task descriptions

test/basetest/task.py

@@ -8,7 +8,13 @@ import tempfile
 import unittest
 from .exceptions import CommandError
 from .hooks import Hooks
-from .utils import run_cmd_wait, run_cmd_wait_nofail, which, task_binary_location
+from .utils import (
+    run_cmd_wait,
+    run_cmd_wait_nofail,
+    which,
+    task_binary_location,
+    CMAKE_BINARY_DIR,
+)
 from .compat import STRING_TYPE
 
 
@@ -300,5 +306,21 @@ class Task(object):
             # Use advanced time format
             self._command = [cmd, "-f", faketime] + self._command
 
+    def make_tc_task(self, **props):
+        """Create a task directly in TaskChampion, bypassing TaskWarrior
+        entirely, and returning the UUID. The properties are not interpreted by
+        the shell.
+        """
+        # Generate the path to the `make_tc_task` binary, which is a dependency of the
+        # test runner.
+        make_tc_task = os.path.abspath(
+            os.path.join(CMAKE_BINARY_DIR, "test", "make_tc_task")
+        )
+        cmd = [make_tc_task, self.datadir]
+        for p, v in props.items():
+            cmd.append(f"{p}={v}")
+        _, out, _ = run_cmd_wait(cmd)
+        return out.strip()
+
 
 # vim: ai sts=4 et sw=4

test/hooks.on-modify.test.py

@@ -176,6 +176,34 @@ class TestHooksOnModify(TestCase):
         hook.assertTriggeredCount(1)
         hook.assertExitcode(0)
 
+    def test_onmodify_escaped_backslash(self):
+        """on-modify-accept - a well-behaved, successful, on-modify hook."""
+        # Create a task with a slash-escaped tab in it, avoiding TaskWarrior to ensure
+        # the slash exists in the DB.
+        uuid = self.t.make_tc_task(description=r"tab\ttab", status="pending")
+
+        hookname = "on-modify-accept"
+        self.t.hooks.add_default(hookname, log=True)
+
+        # `task _get` shows the backslash-escape.
+        code, out, err = self.t(f"_get 1.description")
+        self.assertEqual(out.strip(), r"tab\ttab")
+
+        code, out, err = self.t(f"{uuid} append foo")
+
+        hook = self.t.hooks[hookname]
+        hook.assertTriggeredCount(1)
+        hook.assertExitcode(0)
+
+        logs = hook.get_logs()
+        for msg in logs["output"]["msgs"]:
+            print(msg)
+        self.assertEqual(logs["output"]["msgs"][0], "FEEDBACK")
+
+        # `task _get` still shows the backslash-escape.
+        code, out, err = self.t(f"_get 1.description")
+        self.assertEqual(out.strip(), r"tab\ttab foo")
+
 
 if __name__ == "__main__":
     from simpletap import TAPTestRunner

test/test_hooks/on-add-accept

@@ -5,12 +5,12 @@
 
 # Input:
 # - Line of JSON for proposed new task.
-read new_task
+read -r new_task
 
 # Output:
 # - JSON, modified or unmodified.
 # - Optional feedback/error.
-echo $new_task
+printf "%s\n" "$new_task"
 echo 'FEEDBACK'
 
 # Status:

test/test_hooks/on-add-misbehave2

@@ -5,7 +5,7 @@
 
 # Input:
 # - Line of JSON for proposed new task.
-read new_task
+read -r new_task
 
 # Output:
 # - JSON, modified or unmodified.

test/test_hooks/on-add-misbehave3

@@ -5,12 +5,12 @@
 
 # Input:
 # - Line of JSON for proposed new task.
-read new_task
+read -r new_task
 
 # Output:
 # - JSON, modified or unmodified.
 # - Optional feedback/error.
-echo $new_task
+printf "%s\n" "$new_task"
 echo '{"description":"extra","status":"pending"}'
 echo 'FEEDBACK'
 

test/test_hooks/on-add-misbehave4

@@ -5,7 +5,7 @@
 
 # Input:
 # - Line of JSON for proposed new task.
-read new_task
+read -r new_task
 
 # Output:
 # - JSON, modified or unmodified.

test/test_hooks/on-add-misbehave5

@@ -5,7 +5,7 @@
 
 # Input:
 # - Line of JSON for proposed new task.
-read new_task
+read -r new_task
 
 # Output:
 # - JSON, modified or unmodified.

test/test_hooks/on-add-misbehave6

@@ -5,7 +5,7 @@
 
 # Input:
 # - Line of JSON for proposed new task.
-read new_task
+read -r new_task
 
 # Output:
 # - JSON, modified or unmodified.

test/test_hooks/on-add-modify

@@ -2,17 +2,17 @@
 
 # Input:
 # - Line of JSON for proposed new task.
-read new_task
+read -r new_task
 
-if (echo $new_task | grep -qE '[tT]eh');
+if (printf "%s\n" "$new_task" | grep -qE '[tT]eh');
 then
-  new_task=$(echo $new_task | sed -r 's/([tT])eh/\1he/g')
+  new_task=$(printf "%s\n" "$new_task" | sed -r 's/([tT])eh/\1he/g')
 fi
 
 # Output:
 # - JSON, modified
 # - Optional feedback/error.
-echo $new_task
+printf "%s\n" "$new_task"
 echo 'FEEDBACK'
 
 # Status:

test/test_hooks/on-add-reject

@@ -5,12 +5,12 @@
 
 # Input:
 # - Line of JSON for proposed new task.
-read new_task
+read -r new_task
 
 # Output:
 # - JSON, modified or unmodified.
 # - Optional feedback/error.
-echo $new_task
+printf "%s\n" "$new_task"
 echo 'FEEDBACK'
 
 # Status:

test/test_hooks/on-add.dummy

@@ -2,9 +2,9 @@
 
 echo "on-add executed"
 
-while read TASK; do
+while read -r TASK; do
     echo "New task $TASK"
-    echo $TASK
+    printf "%s\n" "$TASK"
 done
 
 exit 0

test/test_hooks/on-exit-bad

@@ -5,7 +5,7 @@
 
 # Input:
 # - Read-only line of JSON for each task added/modified
-while read modified_task
+while read -r modified_task
 do
   echo 'CHANGED TASK'
 done

test/test_hooks/on-exit-good

@@ -5,7 +5,7 @@
 
 # Input:
 # - Read-only line of JSON for each task added/modified
-while read modified_task
+while read -r modified_task
 do
   echo 'CHANGED TASK'
 done

test/test_hooks/on-exit.dummy

@@ -2,7 +2,7 @@
 
 echo "on-exit executed"
 
-while read TASK; do
+while read -r TASK; do
     echo "New/modified task $TASK"
 done
 

test/test_hooks/on-modify-accept

@@ -6,13 +6,13 @@
 # Input:
 # - line of JSON for the original task
 # - line of JSON for the modified task, the diff being the modification
-read original_task
-read modified_task
+read -r original_task
+read -r modified_task
 
 # Output:
 # - JSON, modified or unmodified.
 # - Optional feedback/error.
-echo $modified_task
+printf "%s\n" "$modified_task"
 echo 'FEEDBACK'
 
 # Status:

test/test_hooks/on-modify-misbehave2

@@ -6,8 +6,8 @@
 # Input:
 # - line of JSON for the original task
 # - line of JSON for the modified task, the diff being the modification
-read original_task
-read modified_task
+read -r original_task
+read -r modified_task
 
 # Output:
 # - JSON, modified or unmodified.

test/test_hooks/on-modify-misbehave3

@@ -6,13 +6,13 @@
 # Input:
 # - line of JSON for the original task
 # - line of JSON for the modified task, the diff being the modification
-read original_task
-read modified_task
+read -r original_task
+read -r modified_task
 
 # Output:
 # - JSON, modified or unmodified.
 # - Optional feedback/error.
-echo $modified_task
+printf "%s\n" "$modified_task"
 echo '{"description":"extra","status":"pending"}'
 echo 'FEEDBACK'
 

test/test_hooks/on-modify-misbehave4

@@ -6,8 +6,8 @@
 # Input:
 # - line of JSON for the original task
 # - line of JSON for the modified task, the diff being the modification
-read original_task
-read modified_task
+read -r original_task
+read -r modified_task
 
 # Output:
 # - JSON, modified or unmodified.

test/test_hooks/on-modify-misbehave5

@@ -6,8 +6,8 @@
 # Input:
 # - line of JSON for the original task
 # - line of JSON for the modified task, the diff being the modification
-read original_task
-read modified_task
+read -r original_task
+read -r modified_task
 
 # Output:
 # - JSON, modified or unmodified.

test/test_hooks/on-modify-misbehave6

@@ -6,8 +6,8 @@
 # Input:
 # - line of JSON for the original task
 # - line of JSON for the modified task, the diff being the modification
-read original_task
-read modified_task
+read -r original_task
+read -r modified_task
 
 # Output:
 # - JSON, modified or unmodified.

test/test_hooks/on-modify-reject

@@ -6,13 +6,13 @@
 # Input:
 # - line of JSON for the original task
 # - line of JSON for the modified task, the diff being the modification
-read original_task
-read modified_task
+read -r original_task
+read -r modified_task
 
 # Output:
 # - JSON, modified or unmodified.
 # - Optional feedback/error.
-echo $modified_task
+printf "%s\n" "$modified_task"
 echo 'FEEDBACK'
 
 # Status:

test/test_hooks/on-modify-revert

@@ -6,13 +6,13 @@
 # Input:
 # - line of JSON for the original task
 # - line of JSON for the modified task, the diff being the modification
-read original_task
-read modified_task
+read -r original_task
+read -r modified_task
 
 # Output:
 # - JSON, modified or unmodified.
 # - Optional feedback/error.
-echo $original_task
+printf "%s\n" "$original_task"
 
 # Status:
 # - 0:     JSON accepted, non-JSON is feedback.

test/test_hooks/on-modify.dummy

@@ -2,9 +2,10 @@
 
 echo "on-modify executed"
 
-while read TASK MODTASK; do
-    echo "Existing task $TASK modified to $MODTASK"
-    echo $MODTASK
-done
+read -r TASK
+read -r MODTASK
+
+echo "Existing task $TASK modified to $MODTASK"
+printf "%s\n" "$MODTASK"
 
 exit 0

test/unusual_task.test.py

@@ -36,7 +36,6 @@ import unittest
 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):
@@ -49,18 +48,8 @@ class TestUnusualTasks(TestCase):
         )
         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()
+        uuid = self.t.make_tc_task()
         _, out, _ = self.t(f"{uuid} info")
         self.assertNotIn("Entered", out)
         self.assertNotIn("Waiting", out)
@@ -72,20 +61,20 @@ class TestUnusualTasks(TestCase):
         self.assertRegex(out, r"Status\s+Pending")
 
     def test_modify_empty_task(self):
-        uuid = self.make_task()
+        uuid = self.t.make_tc_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")
+        uuid = self.t.make_tc_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(
+        uuid = self.t.make_tc_task(
             status="recurring", due=str(int(time.time())), rtype="occasional"
         )
         _, out, _ = self.t(f"{uuid} info")
@@ -94,7 +83,7 @@ class TestUnusualTasks(TestCase):
         _, out, _ = self.t(f"{uuid} custom-report")
 
     def test_recurring_invalid_recur(self):
-        uuid = self.make_task(
+        uuid = self.t.make_tc_task(
             status="recurring",
             due=str(int(time.time())),
             rtype="periodic",
@@ -106,27 +95,27 @@ class TestUnusualTasks(TestCase):
         _, out, _ = self.t(f"{uuid} custom-report")
 
     def test_recurring_bad_quarters_rtype(self):
-        uuid = self.make_task(
+        uuid = self.t.make_tc_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")
+        uuid = self.t.make_tc_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")
+        uuid = self.t.make_tc_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")
+        uuid = self.t.make_tc_task(start="abcdef")
         _, out, _ = self.t(f"{uuid} info")
 
     def test_invalid_dates_report(self):
-        uuid = self.make_task(
+        uuid = self.t.make_tc_task(
             wait="wait",
             scheduled="scheduled",
             start="start",
@@ -138,7 +127,7 @@ class TestUnusualTasks(TestCase):
         _, out, _ = self.t(f"{uuid} custom-report")
 
     def test_invalid_dates_stop(self):
-        uuid = self.make_task(
+        uuid = self.t.make_tc_task(
             wait="wait",
             scheduled="scheduled",
             start="start",
@@ -150,7 +139,7 @@ class TestUnusualTasks(TestCase):
         _, out, _ = self.t(f"{uuid} stop")
 
     def test_invalid_dates_modify(self):
-        uuid = self.make_task(
+        uuid = self.t.make_tc_task(
             wait="wait",
             scheduled="scheduled",
             start="start",
@@ -162,7 +151,7 @@ class TestUnusualTasks(TestCase):
         _, out, _ = self.t(f"{uuid} mod a description +tag")
 
     def test_invalid_dates_info(self):
-        uuid = self.make_task(
+        uuid = self.t.make_tc_task(
             wait="wait",
             scheduled="scheduled",
             start="start",
@@ -183,7 +172,7 @@ class TestUnusualTasks(TestCase):
         # (note that 'modified' is not shown in the journal)
 
     def test_invalid_dates_export(self):
-        uuid = self.make_task(
+        uuid = self.t.make_tc_task(
             wait="wait",
             scheduled="scheduled",
             start="start",