CFE-612: Made signal name matching case-insensitive

cf-agent/verify_processes.c

@@ -69,7 +69,7 @@
 
     if (a->restart_class)
     {
-        if ((RlistKeyIn(a->signals, "term")) || (RlistKeyIn(a->signals, "kill")))
+        if ((RlistKeyIn_IgnoreCase(a->signals, "term")) || (RlistKeyIn_IgnoreCase(a->signals, "kill")))
         {
             Log(LOG_LEVEL_WARNING, "Promise '%s' kills then restarts - never strictly converges",
                 pp->promiser);
@@ -387,7 +387,7 @@
         {
             if (pid == 1)
             {
-                if (RlistLen(a->signals) == 1 && RlistKeyIn(a->signals, "hup"))
+                if (RlistLen(a->signals) == 1 && RlistKeyIn_IgnoreCase(a->signals, "hup"))
                 {
                     Log(LOG_LEVEL_VERBOSE, "Okay to send only HUP to init");
                 }

libpromises/conversion.c

@@ -229,10 +229,14 @@ int SignalFromString(const char *s)
        SIGHUP, SIGINT, SIGTRAP, SIGKILL, SIGPIPE, SIGCONT, SIGABRT, SIGSTOP,
        SIGQUIT, SIGTERM, SIGCHLD, SIGUSR1, SIGUSR2, SIGBUS, SIGSEGV
     };
+    if (s == NULL)
+    {
+        return -1;
+    }
 
     for (size_t i = 0; i < 15; i++)
     {
-        if (StringEqual(s, signal_names[i]))
+        if (StringEqual_IgnoreCase(s, signal_names[i]))
         {
             return signals[i];
         }

libpromises/mod_process.c

@@ -72,7 +72,7 @@ static const ConstraintSyntax processes_constraints[] =
     ConstraintSyntaxNewString("process_stop", CF_ABSPATHRANGE, "A command used to stop a running process", SYNTAX_STATUS_NORMAL),
     ConstraintSyntaxNewString("restart_class", CF_IDRANGE,
      "A class to be defined globally if the process is not running, so that a command: rule can be referred to restart the process", SYNTAX_STATUS_NORMAL),
-    ConstraintSyntaxNewStringList("signals", "(hup|int|trap|kill|pipe|cont|abrt|stop|quit|term|child|usr1|usr2|bus|segv|[0-9]+s?)",
+    ConstraintSyntaxNewStringList("signals", "((?i:hup|int|trap|kill|pipe|cont|abrt|stop|quit|term|child|usr1|usr2|bus|segv)|[0-9]+s?)",
                                   "A list of strings representing signals to be sent to a process or sleep periods", SYNTAX_STATUS_NORMAL),
     ConstraintSyntaxNewNull()
 };

libpromises/rlist.c

@@ -209,6 +209,24 @@ Rlist *RlistKeyIn(Rlist *list, const char *key)
     return NULL;
 }
 
+Rlist *RlistKeyIn_IgnoreCase(Rlist *list, const char *key)
+{
+    for (Rlist *rp = list; rp != NULL; rp = rp->next)
+    {
+        if (rp->val.type == RVAL_TYPE_SCALAR)
+        {
+            const char *scalar = RlistScalarValue(rp);
+            if (scalar != NULL &&
+                StringEqual_IgnoreCase(scalar, key))
+            {
+                return rp;
+            }
+        }
+    }
+
+    return NULL;
+}
+
 /*******************************************************************/
 
 bool RlistMatchesRegexRlist(const Rlist *list, const Rlist *search)

libpromises/rlist.h

@@ -82,6 +82,7 @@ Rlist *RlistRlistValue(const Rlist *rlist);
 Rlist *RlistParseShown(const char *string);
 Rlist *RlistParseString(const char *string);
 Rlist *RlistKeyIn(Rlist *list, const char *key);
+Rlist *RlistKeyIn_IgnoreCase(Rlist *list, const char *key);
 int RlistLen(const Rlist *start);
 bool RlistMatchesRegexRlist(const Rlist *list, const Rlist *search);
 bool RlistMatchesRegex(const Rlist *list, const char *str);

tests/acceptance/05_processes/01_matching/signals_case_insensitive.cf

@@ -0,0 +1,112 @@
+#######################################################
+#
+# CFE-612: processes "signals" accepts signal names case-insensitively.
+# Signal a backgrounded victim with the UPPERCASE "TERM"; its SIGTERM
+# trap records the delivered signal to a sentinel that check verifies.
+# Pre-fix, uppercase fails policy validation. SIGKILL/-1 wouldn't fire
+# the trap, so the sentinel also proves it was specifically SIGTERM.
+#
+#######################################################
+body common control
+{
+  inputs => { "../../default.sub.cf" };
+  bundlesequence => { default("$(this.promise_filename)") };
+  version => "1.0";
+}
+
+#######################################################
+body action background
+{
+  background => "true";
+}
+
+#######################################################
+bundle agent init
+{
+  meta:
+    # Backgrounding + POSIX trap/wait + ps matching need a real POSIX shell.
+    "test_skip_needs_work" string => "!linux";
+
+  vars:
+    "victim" string => "$(G.testdir)/cfe612_signal_victim.sh";
+    "sentinel" string => "$(G.testdir)/cfe612_signal_received";
+    "ready" string => "$(G.testdir)/cfe612_signal_ready";
+    "poll_script" string => "$(G.testdir)/cfe612_poll.sh";
+
+  files:
+    # Remove any artifacts left over from a previous run.
+    "$(sentinel)" delete => tidy;
+    "$(ready)" delete => tidy;
+    "$(poll_script)" delete => tidy;
+
+    # Polling helper: wait up to ~10s for a file to become non-empty.
+    "$(poll_script)"
+      create => "true",
+      content => '#!/bin/sh
+file="$1"
+for _ in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100; do
+  [ -s "$file" ] && exit 0
+  sleep 0.1
+done
+exit 1';
+
+    # Victim: "sleep & wait" so the SIGTERM trap fires promptly; the trap
+    # records the signal then kills its sleep child (a lingering descendant
+    # would hold the harness's output pipe open and stall the run). 30s is
+    # just a safety bound so a never-signalled victim can't run away.
+    "$(victim)"
+      create => "true",
+      content => "#!/bin/sh
+trap 'printf TERM > $(sentinel); kill $sleeppid 2>/dev/null; exit 0' TERM
+printf ready > $(ready)
+sleep 30 &
+sleeppid=$!
+wait
+";
+
+  commands:
+    # Launch the victim, then poll until its ready-file appears (cap ~10s).
+    "$(G.sh) $(victim)" action => background;
+    "$(G.sh) $(poll_script) \"$(ready)\"";
+}
+
+#######################################################
+bundle agent test
+{
+  meta:
+    "description" -> { "CFE-612" }
+      string => "processes 'signals' attribute accepts signal names case-insensitively";
+
+  processes:
+    # UPPERCASE signal name; the unique token only matches our victim.
+    "cfe612_signal_victim" signals => { "TERM" };
+
+  commands:
+    # Poll for the sentinel becoming non-empty (cap ~10s).
+    # processes run before commands, so this runs after the signal.
+    "$(G.sh) $(init.poll_script) \"$(init.sentinel)\"";
+}
+
+#######################################################
+bundle agent check
+{
+  vars:
+    "expected" string => "TERM";
+
+    "actual"
+      string => readfile("$(init.sentinel)"),
+      if => fileexists("$(init.sentinel)");
+
+  classes:
+    "ok" expression => strcmp("$(expected)", "$(actual)");
+
+  reports:
+    DEBUG::
+      "Expected sentinel '$(expected)', found '$(actual)'";
+
+    ok::
+      "$(this.promise_filename) Pass";
+
+    !ok::
+      "$(this.promise_filename) FAIL";
+}

tests/unit/conversion_test.c

@@ -2,6 +2,7 @@
 
 #include <cmockery.h>
 #include <conversion.h>
+#include <signal.h>
 
 static void test_string_is_boolean(void)
 {
@@ -180,6 +181,31 @@ static void test_double_from_string(void)
     assert_true(val == old_val);
 }
 
+static void test_signal_from_string(void)
+{
+    /* Lowercase (baseline -- should always work) */
+    assert_int_equal(SignalFromString("hup"), SIGHUP);
+    assert_int_equal(SignalFromString("int"), SIGINT);
+    assert_int_equal(SignalFromString("term"), SIGTERM);
+    assert_int_equal(SignalFromString("kill"), SIGKILL);
+
+    /* Uppercase (CFE-612: should work but currently fails) */
+    assert_int_equal(SignalFromString("HUP"), SIGHUP);
+    assert_int_equal(SignalFromString("INT"), SIGINT);
+    assert_int_equal(SignalFromString("TERM"), SIGTERM);
+    assert_int_equal(SignalFromString("KILL"), SIGKILL);
+
+    /* Mixed case */
+    assert_int_equal(SignalFromString("Term"), SIGTERM);
+    assert_int_equal(SignalFromString("HuP"), SIGHUP);
+    assert_int_equal(SignalFromString("KiLl"), SIGKILL);
+
+    /* Invalid */
+    assert_int_equal(SignalFromString("invalid"), -1);
+    assert_int_equal(SignalFromString(""), -1);
+    assert_int_equal(SignalFromString(NULL), -1);
+}
+
 static void test_CommandArg0_bound(void)
 {
     char dst[128];
@@ -284,6 +310,7 @@ int main()
         unit_test(test_int_from_string),
         unit_test(test_double_from_string),
         unit_test(test_CommandArg0_bound),
+        unit_test(test_signal_from_string),
     };
 
     return run_tests(tests);

tests/unit/rlist_test.c

@@ -760,6 +760,30 @@ static void test_rval_write_raw()
     WriterClose(writer);
 }
 
+static void test_key_in_ignore_case()
+{
+    Rlist *list = NULL;
+
+    RlistAppendScalar(&list, "TERM");
+    RlistAppendScalar(&list, "HUP");
+    RlistAppendScalar(&list, "KILL");
+
+    Rlist *match = RlistKeyIn_IgnoreCase(list, "term");
+    assert_true(match != NULL);
+    assert_string_equal(RlistScalarValue(match), "TERM");
+
+    match = RlistKeyIn_IgnoreCase(list, "HuP");
+    assert_true(match != NULL);
+    assert_string_equal(RlistScalarValue(match), "HUP");
+
+    match = RlistKeyIn_IgnoreCase(list, "PIPE");
+    assert_true(match == NULL);
+
+    RlistDestroy(list);
+
+    assert_true(RlistKeyIn_IgnoreCase(NULL, "term") == NULL);
+}
+
 int main()
 {
     PRINT_TEST_BANNER();
@@ -796,6 +820,7 @@ int main()
         unit_test(test_rval_write),
         unit_test(test_rval_write_quoted),
         unit_test(test_rval_write_raw),
+        unit_test(test_key_in_ignore_case),
     };
 
     return run_tests(tests);