Merge branch 'bug33104_041'

4 files changed, 57 insertions, 22 deletions

diff --git a/changes/bug33104 b/changes/bug33104
new file mode 100644
index 0000000000..b5478df108
--- /dev/null
+++ b/changes/bug33104
@@ -0,0 +1,4 @@
+  o Minor bugfixes (controller):
+    - When receiving "ACTIVE" or "DORMANT" signals on the control port,
+      report them as SIGNAL events. Fixes bug 33104; bugfix on
+      0.4.0.1-alpha.
diff --git a/src/feature/control/control.c b/src/feature/control/control.c
index b4f27cb820..ee1026359d 100644
--- a/src/feature/control/control.c
+++ b/src/feature/control/control.c
@@ -158,6 +158,10 @@ control_ports_write_to_file(void)
 }
 
 const struct signal_name_t signal_table[] = {
+  /* NOTE: this table is used for handling SIGNAL commands and generating
+   * SIGNAL events.  Order is significant: if there are two entries for the
+   * same numeric signal, the first one is the canonical name generated
+   * for the events. */
   { SIGHUP, "RELOAD" },
   { SIGHUP, "HUP" },
   { SIGINT, "SHUTDOWN" },
diff --git a/src/feature/control/control_events.c b/src/feature/control/control_events.c
index b0a1a3f242..916ccea875 100644
--- a/src/feature/control/control_events.c
+++ b/src/feature/control/control_events.c
@@ -1559,29 +1559,17 @@ control_event_signal(uintptr_t signal_num)
   if (!control_event_is_interesting(EVENT_GOT_SIGNAL))
     return 0;
 
-  switch (signal_num) {
-    case SIGHUP:
-      signal_string = "RELOAD";
+  for (unsigned i = 0; signal_table[i].signal_name != NULL; ++i) {
+    if ((int)signal_num == signal_table[i].sig) {
+      signal_string = signal_table[i].signal_name;
       break;
-    case SIGUSR1:
-      signal_string = "DUMP";
-      break;
-    case SIGUSR2:
-      signal_string = "DEBUG";
-      break;
-    case SIGNEWNYM:
-      signal_string = "NEWNYM";
-      break;
-    case SIGCLEARDNSCACHE:
-      signal_string = "CLEARDNSCACHE";
-      break;
-    case SIGHEARTBEAT:
-      signal_string = "HEARTBEAT";
-      break;
-    default:
-      log_warn(LD_BUG, "Unrecognized signal %lu in control_event_signal",
-               (unsigned long)signal_num);
-      return -1;
+    }
+  }
+
+  if (signal_string == NULL) {
+    log_warn(LD_BUG, "Unrecognized signal %lu in control_event_signal",
+             (unsigned long)signal_num);
+    return -1;
   }
 
   send_control_event(EVENT_GOT_SIGNAL,  "650 SIGNAL %s\r\n",
diff --git a/src/test/test_controller_events.c b/src/test/test_controller_events.c
index e9d09fbeef..60dfbd630a 100644
--- a/src/test/test_controller_events.c
+++ b/src/test/test_controller_events.c
@@ -19,6 +19,7 @@
 #include "feature/control/control_fmt.h"
 #include "test/test.h"
 #include "test/test_helpers.h"
+#include "test/log_test_helpers.h"
 
 #include "core/or/entry_connection_st.h"
 #include "core/or/or_circuit_st.h"
@@ -399,6 +400,43 @@ test_cntev_dirboot_defer_orconn(void *arg)
 }
 
 static void
+test_cntev_signal(void *arg)
+{
+  (void)arg;
+  int rv;
+
+  MOCK(queue_control_event_string, mock_queue_control_event_string);
+
+  /* Nothing is listening for signals, so no event should be queued. */
+  rv = control_event_signal(SIGHUP);
+  tt_int_op(0, OP_EQ, rv);
+  tt_ptr_op(saved_event_str, OP_EQ, NULL);
+
+  /* Now try with signals included in the event mask. */
+  control_testing_set_global_event_mask(EVENT_MASK_(EVENT_GOT_SIGNAL));
+  rv = control_event_signal(SIGHUP);
+  tt_int_op(0, OP_EQ, rv);
+  tt_str_op(saved_event_str, OP_EQ, "650 SIGNAL RELOAD\r\n");
+
+  rv = control_event_signal(SIGACTIVE);
+  tt_int_op(0, OP_EQ, rv);
+  tt_str_op(saved_event_str, OP_EQ, "650 SIGNAL ACTIVE\r\n");
+
+  /* Try a signal that doesn't exist. */
+  setup_full_capture_of_logs(LOG_WARN);
+  tor_free(saved_event_str);
+  rv = control_event_signal(99999);
+  tt_int_op(-1, OP_EQ, rv);
+  tt_ptr_op(saved_event_str, OP_EQ, NULL);
+  expect_single_log_msg_containing("Unrecognized signal 99999");
+
+ done:
+  tor_free(saved_event_str);
+ teardown_capture_of_logs();
+  UNMOCK(queue_control_event_string);
+}
+
+static void
 setup_orconn_state(orconn_state_msg_t *msg, uint64_t gid, uint64_t chan,
                    int proxy_type)
 {
@@ -679,6 +717,7 @@ struct testcase_t controller_event_tests[] = {
   TEST(format_cell_stats, TT_FORK),
   TEST(event_mask, TT_FORK),
   TEST(format_stream, TT_FORK),
+  TEST(signal, TT_FORK),
   T_PUBSUB(dirboot_defer_desc, TT_FORK),
   T_PUBSUB(dirboot_defer_orconn, TT_FORK),
   T_PUBSUB(orconn_state, TT_FORK),