commit c1d96358d49a4583b8aa9bdb1e8d73c70f9d8d06
Author: Nick Mathewson <ni...@torproject.org>
Date:   Tue Jun 1 16:18:23 2021 -0400

    Use native timegm when available.
    
    Continue having a tor_gmtime_impl() unit test so that we can detect
    any problems in our replacement function; add a new test function to
    make sure that gmtime<->timegm are a round-trip on now-ish times.
    
    This is a fix for bug #40383, wherein we ran into trouble because
    tor_timegm() does not believe that time_t should include a count of
    leap seconds, but FreeBSD's gmtime believes that it should.  This
    disagreement meant that for a certain amount of time each day,
    instead of calculating the most recent midnight, our voting-schedule
    functions would calculate the second-most-recent midnight, and lead
    to an assertion failure.
    
    I am calling this a bugfix on 0.2.0.3-alpha when we first started
    calculating our voting schedule in this way.
---
 changes/bug40383            |  7 +++++++
 configure.ac                |  1 +
 src/lib/encoding/time_fmt.c | 35 +++++++++++++++++++++++++++++++++--
 src/lib/encoding/time_fmt.h |  6 ++++++
 src/test/test_util.c        | 26 +++++++++++++++++++++++++-
 5 files changed, 72 insertions(+), 3 deletions(-)

diff --git a/changes/bug40383 b/changes/bug40383
new file mode 100644
index 0000000000..c4ca46fac7
--- /dev/null
+++ b/changes/bug40383
@@ -0,0 +1,7 @@
+  o Minor bugfixes (timekeeping):
+    - Calculate the time of day correctly on systems where the time_t
+      type includes leap seconds. (This is not the case on most
+      operating systems, but on those where it occurs, our tor_timegm
+      function did not correctly invert the system's gmtime function,
+      which could result in assertion failures when calculating
+      voting schedules.)  Fixes bug 40383; bugfix on 0.2.0.3-alpha.
diff --git a/configure.ac b/configure.ac
index 6d7cd1dd6a..5a01c28bde 100644
--- a/configure.ac
+++ b/configure.ac
@@ -793,6 +793,7 @@ AC_CHECK_FUNCS(
        strtoull \
        sysconf \
        sysctl \
+        timegm \
        truncate \
        uname \
        usleep \
diff --git a/src/lib/encoding/time_fmt.c b/src/lib/encoding/time_fmt.c
index 573dfaad82..5e58d36698 100644
--- a/src/lib/encoding/time_fmt.c
+++ b/src/lib/encoding/time_fmt.c
@@ -13,6 +13,7 @@
  * and handles a larger variety of types.  It converts between different time
  * formats, and encodes and decodes them from strings.
  **/
+#define TIME_FMT_PRIVATE
 
 #include "lib/encoding/time_fmt.h"
 #include "lib/log/log.h"
@@ -25,6 +26,7 @@
 
 #include <string.h>
 #include <time.h>
+#include <errno.h>
 
 #ifdef HAVE_SYS_TIME_H
 #include <sys/time.h>
@@ -92,8 +94,8 @@ static const int days_per_month[] =
 /** Compute a time_t given a struct tm.  The result is given in UTC, and
  * does not account for leap seconds.  Return 0 on success, -1 on failure.
  */
-int
-tor_timegm(const struct tm *tm, time_t *time_out)
+ATTR_UNUSED STATIC int
+tor_timegm_impl(const struct tm *tm, time_t *time_out)
 {
   /* This is a pretty ironclad timegm implementation, snarfed from Python2.2.
    * It's way more brute-force than fiddling with tzset().
@@ -162,6 +164,35 @@ tor_timegm(const struct tm *tm, time_t *time_out)
   return 0;
 }
 
+/** Compute a time_t given a struct tm.  The result here should be an inverse
+ * of the system's gmtime() function.  Return 0 on success, -1 on failure.
+ */
+int
+tor_timegm(const struct tm *tm, time_t *time_out)
+{
+#ifdef HAVE_TIMEGM
+  /* If the system gives us a timegm(), use it: if the system's time_t
+   * includes leap seconds, then we can hope that its timegm() knows too.
+   *
+   * https://k5wiki.kerberos.org/wiki/Leap_second_handling says the in
+   * general we can rely on any system with leap seconds also having a
+   * timegm implementation.  Let's hope it's right!
+   * */
+  time_t result = timegm((struct tm *) tm);
+  if (result == -1) {
+    log_warn(LD_BUG, "timegm() could not convert time: %s", strerror(errno));
+    *time_out = 0;
+    return -1;
+  } else {
+    *time_out = result;
+    return 0;
+  }
+#else
+  /* The system doesn't have timegm; we'll have to use our own. */
+  return tor_timegm_impl(tm, time_out);
+#endif
+}
+
 /* strftime is locale-specific, so we need to replace those parts */
 
 /** A c-locale array of 3-letter names of weekdays, starting with Sun. */
diff --git a/src/lib/encoding/time_fmt.h b/src/lib/encoding/time_fmt.h
index 80e47c5332..4adccb5990 100644
--- a/src/lib/encoding/time_fmt.h
+++ b/src/lib/encoding/time_fmt.h
@@ -18,6 +18,8 @@
 #include <sys/types.h>
 #endif
 
+#include "lib/testsupport/testsupport.h"
+
 struct tm;
 struct timeval;
 
@@ -41,4 +43,8 @@ int parse_iso_time_nospace(const char *cp, time_t *t);
 int parse_http_time(const char *buf, struct tm *tm);
 int format_time_interval(char *out, size_t out_len, long interval);
 
+#ifdef TIME_FMT_PRIVATE
+STATIC int tor_timegm_impl(const struct tm *tm, time_t *time_out);
+#endif
+
 #endif /* !defined(TOR_TIME_FMT_H) */
diff --git a/src/test/test_util.c b/src/test/test_util.c
index d43bf781f2..ab63344806 100644
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -7,6 +7,7 @@
 #define COMPAT_TIME_PRIVATE
 #define UTIL_MALLOC_PRIVATE
 #define PROCESS_WIN32_PRIVATE
+#define TIME_FMT_PRIVATE
 #include "lib/testsupport/testsupport.h"
 #include "core/or/or.h"
 #include "lib/buf/buffers.h"
@@ -111,7 +112,7 @@ static time_t
 tor_timegm_wrapper(const struct tm *tm)
 {
   time_t t;
-  if (tor_timegm(tm, &t) < 0)
+  if (tor_timegm_impl(tm, &t) < 0)
     return -1;
   return t;
 }
@@ -1501,6 +1502,28 @@ test_util_parse_http_time(void *arg)
   teardown_capture_of_logs();
 }
 
+static void
+test_util_timegm_real(void *arg)
+{
+  (void)arg;
+  /* Get the real timegm again!  We're not testing our impl; we want the
+   * one that will actually get called. */
+#undef tor_timegm
+
+  /* Now check: is timegm the real inverse of gmtime? */
+  time_t now = time(NULL), time2=0;
+  struct tm tm, *p;
+  p = tor_gmtime_r(&now, &tm);
+  tt_ptr_op(p, OP_NE, NULL);
+
+  int r = tor_timegm(&tm, &time2);
+  tt_int_op(r, OP_EQ, 0);
+  tt_i64_op((int64_t) now, OP_EQ, (int64_t) time2);
+
+ done:
+  ;
+}
+
 static void
 test_util_config_line(void *arg)
 {
@@ -7043,6 +7066,7 @@ struct testcase_t util_tests[] = {
   UTIL_TEST(monotonic_time_ratchet, TT_FORK),
   UTIL_TEST(monotonic_time_zero, 0),
   UTIL_TEST(monotonic_time_add_msec, 0),
+  UTIL_TEST(timegm_real, 0),
   UTIL_TEST(htonll, 0),
   UTIL_TEST(get_unquoted_path, 0),
   UTIL_TEST(map_anon, 0),



_______________________________________________
tor-commits mailing list
tor-commits@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits

Reply via email to