From 10af103db7e4fe914793b04b8f51d1549e3bfb93 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Wed, 11 Sep 2019 14:55:38 +0200 Subject: [PATCH] [897-add-infinite-valid-lifetime] Cql does not use timestamps for expire values --- src/lib/cql/cql_exchange.cc | 15 +------ src/lib/dhcpsrv/cql_lease_mgr.cc | 45 ++++--------------- .../dhcpsrv/tests/cql_lease_mgr_unittest.cc | 10 ----- 3 files changed, 11 insertions(+), 59 deletions(-) diff --git a/src/lib/cql/cql_exchange.cc b/src/lib/cql/cql_exchange.cc index 61d0bb707a..d28183496e 100644 --- a/src/lib/cql/cql_exchange.cc +++ b/src/lib/cql/cql_exchange.cc @@ -742,17 +742,10 @@ void CqlExchange::convertToDatabaseTime(const time_t& cltt, const uint32_t& valid_lifetime, cass_int64_t& expire) { - // Calculate expire time. Store it in the 64-bit value so as we can - // detect overflows. + // Calculate expire time. cass_int64_t expire_time = static_cast(cltt) + static_cast(valid_lifetime); - if (expire_time > DatabaseConnection::MAX_DB_TIME) { - isc_throw(BadValue, - "CqlExchange(): convertToDatabaseTime(): time value " - << expire_time << " is too large"); - } - expire = expire_time; } @@ -760,11 +753,7 @@ void CqlExchange::convertFromDatabaseTime(const cass_int64_t& expire, const cass_int64_t& valid_lifetime, time_t& cltt) { - /// @todo: Although 2037 is still far away, there are people who use infinite - /// lifetimes. Cassandra doesn't have to support it right now, but at least - /// we should be able to detect a problem. - - // Convert to local time + // Convert to local time. cltt = static_cast(expire - valid_lifetime); } diff --git a/src/lib/dhcpsrv/cql_lease_mgr.cc b/src/lib/dhcpsrv/cql_lease_mgr.cc index 9cddf8f25d..e61b2c0b64 100644 --- a/src/lib/dhcpsrv/cql_lease_mgr.cc +++ b/src/lib/dhcpsrv/cql_lease_mgr.cc @@ -449,12 +449,8 @@ CqlLease4Exchange::createBindForInsert(const Lease4Ptr &lease, AnyArray &data) { // For convenience for external tools, this is converted to lease // expiry time (expire). The relationship is given by: // expire = cltt_ + valid_lft_ - // Avoid overflow - uint32_t valid_lft = lease_->valid_lft_; - if (valid_lft == Lease::INFINITY_LFT) { - valid_lft = Lease::FIVEHUNDREDDAYS; - } - CqlExchange::convertToDatabaseTime(lease_->cltt_, valid_lft, expire_); + CqlExchange::convertToDatabaseTime(lease_->cltt_, lease_->valid_lft_, + expire_); // subnet_id: int subnet_id_ = static_cast(lease_->subnet_id_); @@ -556,12 +552,8 @@ CqlLease4Exchange::createBindForUpdate(const Lease4Ptr &lease, AnyArray &data, // For convenience for external tools, this is converted to lease // expiry time (expire). The relationship is given by: // expire = cltt_ + valid_lft_ - // Avoid overflow - uint32_t valid_lft = lease_->valid_lft_; - if (valid_lft == Lease::INFINITY_LFT) { - valid_lft = Lease::FIVEHUNDREDDAYS; - } - CqlExchange::convertToDatabaseTime(lease_->cltt_, valid_lft, expire_); + CqlExchange::convertToDatabaseTime(lease_->cltt_, lease_->valid_lft_, + expire_); // subnet_id: int subnet_id_ = static_cast(lease_->subnet_id_); @@ -705,12 +697,7 @@ CqlLease4Exchange::retrieve() { } time_t cltt = 0; - // Recover from overflow - uint32_t valid_lft = valid_lifetime_; - if (valid_lft == Lease::INFINITY_LFT) { - valid_lft = Lease::FIVEHUNDREDDAYS; - } - CqlExchange::convertFromDatabaseTime(expire_, valid_lft, cltt); + CqlExchange::convertFromDatabaseTime(expire_, valid_lifetime_, cltt); HWAddrPtr hwaddr(new HWAddr(hwaddr_, HTYPE_ETHER)); @@ -1127,12 +1114,7 @@ CqlLease6Exchange::createBindForInsert(const Lease6Ptr &lease, AnyArray &data) { // For convenience for external tools, this is converted to lease // expiry time (expire). The relationship is given by: // expire = cltt_ + valid_lft_ - // Avoid overflow - uint32_t valid_lft = lease_->valid_lft_; - if (valid_lft == Lease::INFINITY_LFT) { - valid_lft = Lease::FIVEHUNDREDDAYS; - } - CqlExchange::convertToDatabaseTime(lease_->cltt_, valid_lft, expire_); + CqlExchange::convertToDatabaseTime(lease_->cltt_, lease_->valid_lft_, expire_); // subnet_id: int subnet_id_ = static_cast(lease_->subnet_id_); @@ -1267,12 +1249,8 @@ CqlLease6Exchange::createBindForUpdate(const Lease6Ptr &lease, AnyArray &data, // For convenience for external tools, this is converted to lease // expiry time (expire). The relationship is given by: // expire = cltt_ + valid_lft_ - // Avoid overflow - uint32_t valid_lft = lease_->valid_lft_; - if (valid_lft == Lease::INFINITY_LFT) { - valid_lft = Lease::FIVEHUNDREDDAYS; - } - CqlExchange::convertToDatabaseTime(lease_->cltt_, valid_lft, expire_); + CqlExchange::convertToDatabaseTime(lease_->cltt_, lease_->valid_lft_, + expire_); // subnet_id: int subnet_id_ = static_cast(lease_->subnet_id_); @@ -1529,12 +1507,7 @@ CqlLease6Exchange::retrieve() { fqdn_fwd_, fqdn_rev_, hostname_, hwaddr, prefix_len_)); time_t cltt = 0; - // Recover from overflow - uint32_t valid_lft = valid_lifetime_; - if (valid_lft == Lease::INFINITY_LFT) { - valid_lft = Lease::FIVEHUNDREDDAYS; - } - CqlExchange::convertFromDatabaseTime(expire_, valid_lft, cltt); + CqlExchange::convertFromDatabaseTime(expire_, valid_lifetime_, cltt); result->cltt_ = cltt; result->state_ = state_; diff --git a/src/lib/dhcpsrv/tests/cql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/cql_lease_mgr_unittest.cc index fa1b2267dc..727c9d644b 100644 --- a/src/lib/dhcpsrv/tests/cql_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/cql_lease_mgr_unittest.cc @@ -503,11 +503,6 @@ TEST_F(CqlLeaseMgrTest, basicLease4) { testBasicLease4(); } -/// @brief Check that Lease4 code safely handles invalid dates. -TEST_F(CqlLeaseMgrTest, maxDate4) { - testMaxDate4(); -} - /// @brief checks that infinite lifetimes do not overflow. TEST_F(CqlLeaseMgrTest, infiniteLifeTime4) { testInfiniteLifeTime4(); @@ -654,11 +649,6 @@ TEST_F(CqlLeaseMgrTest, basicLease6) { testBasicLease6(); } -/// @brief Check that Lease6 code safely handles invalid dates. -TEST_F(CqlLeaseMgrTest, maxDate6) { - testMaxDate6(); -} - /// @brief checks that infinite lifetimes do not overflow. TEST_F(CqlLeaseMgrTest, infiniteLifeTime6) { testInfiniteLifeTime6(); -- 2.47.2