use more accurate method to calculate time interval in should_autodeploy() and should_autorenew() functions

letsencrypt/storage.py

@@ -2,7 +2,6 @@
 import datetime
 import os
 import re
-import time
 
 import configobj
 import parsedatetime
@@ -24,8 +23,8 @@ def config_with_defaults(config=None):
     return defaults_copy
 
 
-def parse_time_interval(interval, textparser=parsedatetime.Calendar()):
-    """Parse the time specified time interval.
+def add_time_interval(base_time, interval, textparser=parsedatetime.Calendar()):
+    """Parse the time specified time interval, and add it to the base_time
 
     The interval can be in the English-language format understood by
     parsedatetime, e.g., '10 days', '3 weeks', '6 months', '9 hours', or
@@ -33,15 +32,19 @@ def parse_time_interval(interval, textparser=parsedatetime.Calendar()):
     hours'. If an integer is found with no associated unit, it is
     interpreted by default as a number of days.
 
+    :param datetime.datetime base_time: The time to be added with the interval.
     :param str interval: The time interval to parse.
 
-    :returns: The interpretation of the time interval.
-    :rtype: :class:`datetime.timedelta`"""
+    :returns: The base_time plus the interpretation of the time interval.
+    :rtype: :class:`datetime.datetime`"""
 
     if interval.strip().isdigit():
         interval += " days"
-    return datetime.timedelta(0, time.mktime(textparser.parse(
-        interval, time.localtime(0))[0]))
+
+    # try to use the same timezone, but fallback to UTC
+    tzinfo = base_time.tzinfo or pytz.UTC
+
+    return textparser.parseDT(interval, base_time, tzinfo=tzinfo)[0]
 
 
 class RenewableCert(object):  # pylint: disable=too-many-instance-attributes
@@ -465,11 +468,9 @@ class RenewableCert(object):  # pylint: disable=too-many-instance-attributes
             if self.has_pending_deployment():
                 interval = self.configuration.get("deploy_before_expiry",
                                                   "5 days")
-                autodeploy_interval = parse_time_interval(interval)
                 expiry = crypto_util.notAfter(self.current_target("cert"))
-                now = datetime.datetime.utcnow().replace(tzinfo=pytz.UTC)
-                remaining = expiry - now
-                if remaining < autodeploy_interval:
+                now = pytz.UTC.fromutc(datetime.datetime.utcnow())
+                if expiry < add_time_interval(now, interval):
                     return True
         return False
 
@@ -532,12 +533,10 @@ class RenewableCert(object):  # pylint: disable=too-many-instance-attributes
 
             # Renewals on the basis of expiry time
             interval = self.configuration.get("renew_before_expiry", "10 days")
-            autorenew_interval = parse_time_interval(interval)
             expiry = crypto_util.notAfter(self.version(
                 "cert", self.latest_common_version()))
-            now = datetime.datetime.utcnow().replace(tzinfo=pytz.UTC)
-            remaining = expiry - now
-            if remaining < autorenew_interval:
+            now = pytz.UTC.fromutc(datetime.datetime.utcnow())
+            if expiry < add_time_interval(now, interval):
                 return True
         return False
 

letsencrypt/tests/renewer_test.py

@@ -1,5 +1,6 @@
 """Tests for letsencrypt.renewer."""
 import datetime
+import pytz
 import os
 import tempfile
 import shutil
@@ -622,18 +623,47 @@ class RenewableCertTests(BaseRenewableCertTest):
         #      OCSP server to test against.
         self.assertFalse(self.test_rc.ocsp_revoked())
 
-    def test_parse_time_interval(self):
+    def test_add_time_interval(self):
         from letsencrypt import storage
-        # XXX: I'm not sure if intervals related to years and months
-        #      take account of the current date (if so, some of these
-        #      may fail in the future, like in leap years or even in
-        #      months of different lengths!)
-        intended = {"": 0, "17 days": 17, "23": 23, "1 month": 31,
-                    "7 weeks": 49, "1 year 1 day": 366, "1 year-1 day": 364,
-                    "4 years": 1461}
-        for time in intended:
-            self.assertEqual(storage.parse_time_interval(time),
-                             datetime.timedelta(intended[time]))
+
+        # this month has 30 days, and the next year is a leap year
+        time_1 = pytz.UTC.fromutc(datetime.datetime(2003, 11, 20, 11, 59, 21))
+
+        # this month has 31 days, and the next year is not a leap year
+        time_2 = pytz.UTC.fromutc(datetime.datetime(2012, 10, 18, 21, 31, 16))
+
+        # in different time zone (GMT+8)
+        time_3 = pytz.timezone('Asia/Shanghai').fromutc(
+            datetime.datetime(2015, 10, 26, 22, 25, 41))
+
+        intended = {
+            (time_1, ""): time_1,
+            (time_2, ""): time_2,
+            (time_3, ""): time_3,
+            (time_1, "17 days"): time_1 + datetime.timedelta(17),
+            (time_2, "17 days"): time_2 + datetime.timedelta(17),
+            (time_1, "30"): time_1 + datetime.timedelta(30),
+            (time_2, "30"): time_2 + datetime.timedelta(30),
+            (time_1, "7 weeks"): time_1 + datetime.timedelta(49),
+            (time_2, "7 weeks"): time_2 + datetime.timedelta(49),
+            # 1 month is always 30 days, no matter which month it is
+            (time_1, "1 month"): time_1 + datetime.timedelta(30),
+            (time_2, "1 month"): time_2 + datetime.timedelta(31),
+            # 1 year could be 365 or 366 days, depends on the year
+            (time_1, "1 year"): time_1 + datetime.timedelta(366),
+            (time_2, "1 year"): time_2 + datetime.timedelta(365),
+            (time_1, "1 year 1 day"): time_1 + datetime.timedelta(367),
+            (time_2, "1 year 1 day"): time_2 + datetime.timedelta(366),
+            (time_1, "1 year-1 day"): time_1 + datetime.timedelta(365),
+            (time_2, "1 year-1 day"): time_2 + datetime.timedelta(364),
+            (time_1, "4 years"): time_1 + datetime.timedelta(1461),
+            (time_2, "4 years"): time_2 + datetime.timedelta(1461),
+        }
+
+        for parameters, excepted in intended.items():
+            base_time, interval = parameters
+            self.assertEqual(storage.add_time_interval(base_time, interval),
+                             excepted)
 
     @mock.patch("letsencrypt.renewer.plugins_disco")
     @mock.patch("letsencrypt.account.AccountFileStorage")