Browse Source

[3673] The time conversion method in PgSQL calculates expiry time itself.

This is to prevent the negative overflows when valid_lft is substracted
from the time_t value. It is also consistent with the MySQL implementation.
Marcin Siodelski 10 years ago
parent
commit
ccf7e110c2

+ 3 - 2
src/lib/dhcpsrv/mysql_lease_mgr.cc

@@ -1278,8 +1278,9 @@ MySqlLeaseMgr::~MySqlLeaseMgr() {
 //    from a time read from the database into a local time.
 //    from a time read from the database into a local time.
 
 
 void
 void
-MySqlLeaseMgr::convertToDatabaseTime(time_t cltt, uint32_t valid_lifetime,
-                                    MYSQL_TIME& expire) {
+MySqlLeaseMgr::convertToDatabaseTime(const time_t cltt,
+                                     const uint32_t valid_lifetime,
+                                     MYSQL_TIME& expire) {
 
 
     // Calculate expiry time. Store it in the 64-bit value so as we can detect
     // Calculate expiry time. Store it in the 64-bit value so as we can detect
     // overflows.
     // overflows.

+ 5 - 2
src/lib/dhcpsrv/mysql_lease_mgr.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
 // purpose with or without fee is hereby granted, provided that the above
@@ -423,8 +423,11 @@ public:
     /// @param valid_lifetime Valid lifetime
     /// @param valid_lifetime Valid lifetime
     /// @param expire Reference to MYSQL_TIME object where the expiry time of
     /// @param expire Reference to MYSQL_TIME object where the expiry time of
     ///        the lease will be put.
     ///        the lease will be put.
+    ///
+    /// @throw isc::BadValue if the sum of the calculated expiration time is
+    /// greater than the value of @c LeaseMgr::MAX_DB_TIME.
     static
     static
-    void convertToDatabaseTime(time_t cltt, uint32_t valid_lifetime,
+    void convertToDatabaseTime(const time_t cltt, const uint32_t valid_lifetime,
                                MYSQL_TIME& expire);
                                MYSQL_TIME& expire);
 
 
     /// @brief Convert Database Time to Lease Times
     /// @brief Convert Database Time to Lease Times

+ 21 - 12
src/lib/dhcpsrv/pgsql_lease_mgr.cc

@@ -289,7 +289,11 @@ public:
 
 
     virtual ~PgSqlLeaseExchange(){}
     virtual ~PgSqlLeaseExchange(){}
 
 
-    /// @brief Converts time_t structure to a text representation in local time.
+    /// @brief Converts lease expiration time to a text representation in
+    /// local time.
+    ///
+    /// The expiration time is calculated as a sum of the cltt (client last
+    /// transmit time) and the valid lifetime.
     ///
     ///
     /// The format of the output string is "%Y-%m-%d %H:%M:%S".  Database
     /// The format of the output string is "%Y-%m-%d %H:%M:%S".  Database
     /// table columns using this value should be typed as TIMESTAMP WITH
     /// table columns using this value should be typed as TIMESTAMP WITH
@@ -298,23 +302,30 @@ public:
     /// when stored.  Likewise, these columns are automatically adjusted
     /// when stored.  Likewise, these columns are automatically adjusted
     /// upon retrieval unless fetched via "extract(epoch from <column>))".
     /// upon retrieval unless fetched via "extract(epoch from <column>))".
     ///
     ///
-    /// @param time_val_64 timestamp to be converted. This is given as a
-    /// 64-bit value to avoid overflows on the 32-bit systems where time_t
-    /// is implemented as int32_t.
+    /// @param cltt Client last transmit time
+    /// @param valid_lifetime Valid lifetime
+    ///
     /// @return std::string containing the stringified time
     /// @return std::string containing the stringified time
+    /// @throw isc::BadValue if the sum of the calculated expiration time is
+    /// greater than the value of @c LeaseMgr::MAX_DB_TIME.
     static std::string
     static std::string
-    convertToDatabaseTime(const int64_t& time_val_64) {
+    convertToDatabaseTime(const time_t cltt, const uint32_t valid_lifetime) {
+        // Calculate expiry time. Store it in the 64-bit value so as we can detect
+        // overflows.
+        int64_t expire_time_64 = static_cast<int64_t>(cltt) +
+            static_cast<int64_t>(valid_lifetime);
+
         // PostgreSQL does funny things with time if you get past Y2038.  It
         // PostgreSQL does funny things with time if you get past Y2038.  It
         // will accept the values (unlike MySQL which throws) but it
         // will accept the values (unlike MySQL which throws) but it
         // stops correctly adjusting to local time when reading them back
         // stops correctly adjusting to local time when reading them back
         // out. So lets disallow it here.
         // out. So lets disallow it here.
-        if (time_val_64 > LeaseMgr::MAX_DB_TIME) {
-            isc_throw(BadValue, "Time value is too large: " << time_val_64);
+        if (expire_time_64 > LeaseMgr::MAX_DB_TIME) {
+            isc_throw(isc::BadValue, "Time value is too large: " << expire_time_64);
         }
         }
 
 
         struct tm tinfo;
         struct tm tinfo;
         char buffer[20];
         char buffer[20];
-        const time_t time_val = static_cast<time_t>(time_val_64);
+        const time_t time_val = static_cast<time_t>(expire_time_64);
         localtime_r(&time_val, &tinfo);
         localtime_r(&time_val, &tinfo);
         strftime(buffer, sizeof(buffer), "%Y-%m-%d %H:%M:%S", &tinfo);
         strftime(buffer, sizeof(buffer), "%Y-%m-%d %H:%M:%S", &tinfo);
         return (std::string(buffer));
         return (std::string(buffer));
@@ -636,8 +647,7 @@ public:
                              (lease->valid_lft_);
                              (lease->valid_lft_);
             bind_array.add(valid_lft_str_);
             bind_array.add(valid_lft_str_);
 
 
-            expire_str_ = convertToDatabaseTime(static_cast<int64_t>(lease->valid_lft_) +
-                                                static_cast<int64_t>(lease->cltt_));
+            expire_str_ = convertToDatabaseTime(lease->cltt_, lease_->valid_lft_);
             bind_array.add(expire_str_);
             bind_array.add(expire_str_);
 
 
             subnet_id_str_ = boost::lexical_cast<std::string>
             subnet_id_str_ = boost::lexical_cast<std::string>
@@ -801,8 +811,7 @@ public:
                              (lease->valid_lft_);
                              (lease->valid_lft_);
             bind_array.add(valid_lft_str_);
             bind_array.add(valid_lft_str_);
 
 
-            expire_str_ = convertToDatabaseTime(static_cast<int64_t>(lease->valid_lft_) +
-                                                static_cast<int64_t>(lease->cltt_));
+            expire_str_ = convertToDatabaseTime(lease->cltt_, lease_->valid_lft_);
             bind_array.add(expire_str_);
             bind_array.add(expire_str_);
 
 
             subnet_id_str_ = boost::lexical_cast<std::string>
             subnet_id_str_ = boost::lexical_cast<std::string>