BIND 10 master, updated. cac02e9290600407bd6f3071c6654c1216278616 [master] Merge branch 'trac2837' always use STRICT_SQL mode in mysql_lease_mgr.
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Apr 9 19:19:04 UTC 2013
The branch, master has been updated
via cac02e9290600407bd6f3071c6654c1216278616 (commit)
via 91a0d273da950aac6348bbc900ceca502a6974db (commit)
via 2b7e75de166cb416a60080ad34d8acbed4fd8119 (commit)
via 02fb3337a339f2043183ae66062a774ae6375e83 (commit)
via ab624f466e084edecb4a6299a56f8d33282a1ba0 (commit)
from 403c7178590825e101e0822715fa557403ff5c33 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit cac02e9290600407bd6f3071c6654c1216278616
Merge: 403c717 91a0d27
Author: Thomas Markwalder <tmark at isc.org>
Date: Tue Apr 9 15:16:32 2013 -0400
[master] Merge branch 'trac2837' always use STRICT_SQL mode in
mysql_lease_mgr.
-----------------------------------------------------------------------
Summary of changes:
src/lib/dhcp/hwaddr.cc | 11 ++++-
src/lib/dhcp/hwaddr.h | 2 +
src/lib/dhcp/tests/hwaddr_unittest.cc | 11 ++++-
src/lib/dhcpsrv/lease_mgr.h | 2 -
src/lib/dhcpsrv/mysql_lease_mgr.cc | 16 ++++--
src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc | 54 +++++++++------------
6 files changed, 57 insertions(+), 39 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/dhcp/hwaddr.cc b/src/lib/dhcp/hwaddr.cc
index d19f2ad..eb23b44 100644
--- a/src/lib/dhcp/hwaddr.cc
+++ b/src/lib/dhcp/hwaddr.cc
@@ -14,9 +14,11 @@
#include <dhcp/hwaddr.h>
#include <dhcp/dhcp4.h>
+#include <exceptions/exceptions.h>
#include <iomanip>
#include <sstream>
#include <vector>
+#include <string.h>
namespace isc {
namespace dhcp {
@@ -27,10 +29,17 @@ HWAddr::HWAddr()
HWAddr::HWAddr(const uint8_t* hwaddr, size_t len, uint8_t htype)
:hwaddr_(hwaddr, hwaddr + len), htype_(htype) {
+ if (len > MAX_HWADDR_LEN) {
+ isc_throw(InvalidParameter, "hwaddr length exceeds MAX_HWADDR_LEN");
+ }
}
HWAddr::HWAddr(const std::vector<uint8_t>& hwaddr, uint8_t htype)
:hwaddr_(hwaddr), htype_(htype) {
+ if (hwaddr.size() > MAX_HWADDR_LEN) {
+ isc_throw(InvalidParameter,
+ "address vector size exceeds MAX_HWADDR_LEN");
+ }
}
std::string HWAddr::toText() const {
@@ -50,7 +59,7 @@ std::string HWAddr::toText() const {
}
bool HWAddr::operator==(const HWAddr& other) const {
- return ((this->htype_ == other.htype_) &&
+ return ((this->htype_ == other.htype_) &&
(this->hwaddr_ == other.hwaddr_));
}
diff --git a/src/lib/dhcp/hwaddr.h b/src/lib/dhcp/hwaddr.h
index 93b06a1..13a16bf 100644
--- a/src/lib/dhcp/hwaddr.h
+++ b/src/lib/dhcp/hwaddr.h
@@ -27,6 +27,8 @@ namespace dhcp {
/// @brief Hardware type that represents information from DHCPv4 packet
struct HWAddr {
public:
+ /// @brief Maximum size of a hardware address.
+ static const size_t MAX_HWADDR_LEN = 20;
/// @brief default constructor
HWAddr();
diff --git a/src/lib/dhcp/tests/hwaddr_unittest.cc b/src/lib/dhcp/tests/hwaddr_unittest.cc
index 144d62d..bf2eb9a 100644
--- a/src/lib/dhcp/tests/hwaddr_unittest.cc
+++ b/src/lib/dhcp/tests/hwaddr_unittest.cc
@@ -40,9 +40,11 @@ TEST(HWAddrTest, constructor) {
const uint8_t data1[] = {0, 1, 2, 3, 4, 5, 6};
const uint8_t htype = HTYPE_ETHER;
-
vector<uint8_t> data2(data1, data1 + sizeof(data1));
+ // Over the limit data
+ vector<uint8_t> big_data_vector(HWAddr::MAX_HWADDR_LEN + 1, 0);
+
scoped_ptr<HWAddr> hwaddr1(new HWAddr(data1, sizeof(data1), htype));
scoped_ptr<HWAddr> hwaddr2(new HWAddr(data2, htype));
scoped_ptr<HWAddr> hwaddr3(new HWAddr());
@@ -55,6 +57,13 @@ TEST(HWAddrTest, constructor) {
EXPECT_EQ(0, hwaddr3->hwaddr_.size());
EXPECT_EQ(htype, hwaddr3->htype_);
+
+ // Check that over the limit data length throws exception
+ EXPECT_THROW(HWAddr(&big_data_vector[0], big_data_vector.size(), HTYPE_ETHER),
+ InvalidParameter);
+
+ // Check that over the limit vector throws exception
+ EXPECT_THROW(HWAddr(big_data_vector, HTYPE_ETHER), InvalidParameter);
}
// This test checks if the comparison operators are sane.
diff --git a/src/lib/dhcpsrv/lease_mgr.h b/src/lib/dhcpsrv/lease_mgr.h
index 7d328e7..02e517e 100644
--- a/src/lib/dhcpsrv/lease_mgr.h
+++ b/src/lib/dhcpsrv/lease_mgr.h
@@ -211,8 +211,6 @@ struct Lease {
/// would be required. As this is a critical part of the code that will be used
/// extensively, direct access is warranted.
struct Lease4 : public Lease {
- /// @brief Maximum size of a hardware address
- static const size_t HWADDR_MAX = 20;
/// @brief Address extension
///
diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc
index b9706e4..9828085 100644
--- a/src/lib/dhcpsrv/mysql_lease_mgr.cc
+++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc
@@ -94,9 +94,6 @@ namespace {
/// colon separators.
const size_t ADDRESS6_TEXT_MAX_LEN = 39;
-/// @brief Maximum size of a hardware address.
-const size_t HWADDR_MAX_LEN = 20;
-
/// @brief MySQL True/False constants
///
/// Declare typed values so as to avoid problems of data conversion. These
@@ -533,7 +530,7 @@ private:
std::string columns_[LEASE_COLUMNS];///< Column names
my_bool error_[LEASE_COLUMNS]; ///< Error array
std::vector<uint8_t> hwaddr_; ///< Hardware address
- uint8_t hwaddr_buffer_[HWADDR_MAX_LEN];
+ uint8_t hwaddr_buffer_[HWAddr::MAX_HWADDR_LEN];
///< Hardware address buffer
unsigned long hwaddr_length_; ///< Hardware address length
std::vector<uint8_t> client_id_; ///< Client identification
@@ -1109,6 +1106,17 @@ MySqlLeaseMgr::openDatabase() {
mysql_error(mysql_));
}
+ // Set SQL mode options for the connection: SQL mode governs how what
+ // constitutes insertable data for a given column, and how to handle
+ // invalid data. We want to ensure we get the strictest behavior and
+ // to reject invalid data with an error.
+ const char *sql_mode = "SET SESSION sql_mode ='STRICT_ALL_TABLES'";
+ result = mysql_options(mysql_, MYSQL_INIT_COMMAND, sql_mode);
+ if (result != 0) {
+ isc_throw(DbOpenError, "unable to set SQL mode options: " <<
+ mysql_error(mysql_));
+ }
+
// Open the database.
//
// The option CLIENT_FOUND_ROWS is specified so that in an UPDATE,
diff --git a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc
index 226afe9..5b2fa9e 100644
--- a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc
+++ b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc
@@ -18,6 +18,7 @@
#include <dhcpsrv/lease_mgr_factory.h>
#include <dhcpsrv/mysql_lease_mgr.h>
#include <dhcpsrv/tests/test_utils.h>
+#include <exceptions/exceptions.h>
#include <gtest/gtest.h>
@@ -883,28 +884,23 @@ TEST_F(MySqlLeaseMgrTest, getLease4HwaddrSize) {
vector<Lease4Ptr> leases = createLeases4();
// Now add leases with increasing hardware address size.
- for (uint8_t i = 0; i <= Lease4::HWADDR_MAX; ++i) {
+ for (uint8_t i = 0; i <= HWAddr::MAX_HWADDR_LEN; ++i) {
leases[1]->hwaddr_.resize(i, i);
EXPECT_TRUE(lmptr_->addLease(leases[1]));
// @todo: Simply use HWAddr directly once 2589 is implemented
- Lease4Collection returned = lmptr_->getLease4(HWAddr(leases[1]->hwaddr_, HTYPE_ETHER));
+ Lease4Collection returned =
+ lmptr_->getLease4(HWAddr(leases[1]->hwaddr_, HTYPE_ETHER));
+
ASSERT_EQ(1, returned.size());
detailCompareLease(leases[1], *returned.begin());
(void) lmptr_->deleteLease(leases[1]->addr_);
}
- // Expect some problem when accessing a lease that had too long a hardware
- // address. (The 42 is a random value put in each byte of the address.)
- // In fact the address is stored in a truncated form, so we won't find it
- // when we look.
- // @todo Check if there is some way of detecting that data added
- // to the database is truncated. There does not appear to
- // be any indication in the C API.
- leases[1]->hwaddr_.resize(Lease4::HWADDR_MAX + 100, 42);
- EXPECT_TRUE(lmptr_->addLease(leases[1]));
- // @todo: Simply use HWAddr directly once 2589 is implemented
- Lease4Collection returned = lmptr_->getLease4(HWAddr(leases[1]->hwaddr_, HTYPE_ETHER));
- EXPECT_EQ(0, returned.size());
+ // Database should not let us add one that is too big
+ // (The 42 is a random value put in each byte of the address.)
+ // @todo: 2589 will make this test impossible
+ leases[1]->hwaddr_.resize(HWAddr::MAX_HWADDR_LEN + 100, 42);
+ EXPECT_THROW(lmptr_->addLease(leases[1]), isc::dhcp::DbOperationError);
}
/// @brief Check GetLease4 methods - access by Hardware Address & Subnet ID
@@ -921,8 +917,9 @@ TEST_F(MySqlLeaseMgrTest, getLease4HwaddrSubnetId) {
// Get the leases matching the hardware address of lease 1 and
// subnet ID of lease 1. Result should be a single lease - lease 1.
// @todo: Simply use HWAddr directly once 2589 is implemented
- Lease4Ptr returned = lmptr_->getLease4(HWAddr(leases[1]->hwaddr_, HTYPE_ETHER),
- leases[1]->subnet_id_);
+ Lease4Ptr returned = lmptr_->getLease4(HWAddr(leases[1]->hwaddr_,
+ HTYPE_ETHER), leases[1]->subnet_id_);
+
ASSERT_TRUE(returned);
detailCompareLease(leases[1], returned);
@@ -957,8 +954,9 @@ TEST_F(MySqlLeaseMgrTest, getLease4HwaddrSubnetId) {
leases[1]->addr_ = leases[2]->addr_;
EXPECT_TRUE(lmptr_->addLease(leases[1]));
// @todo: Simply use HWAddr directly once 2589 is implemented
- EXPECT_THROW(returned = lmptr_->getLease4(HWAddr(leases[1]->hwaddr_, HTYPE_ETHER),
- leases[1]->subnet_id_),
+ EXPECT_THROW(returned = lmptr_->getLease4(HWAddr(leases[1]->hwaddr_,
+ HTYPE_ETHER),
+ leases[1]->subnet_id_),
isc::dhcp::MultipleRecords);
// Delete all leases in the database
@@ -979,28 +977,22 @@ TEST_F(MySqlLeaseMgrTest, getLease4HwaddrSubnetIdSize) {
// Now add leases with increasing hardware address size and check
// that they can be retrieved.
- for (uint8_t i = 0; i <= Lease4::HWADDR_MAX; ++i) {
+ for (uint8_t i = 0; i <= HWAddr::MAX_HWADDR_LEN; ++i) {
leases[1]->hwaddr_.resize(i, i);
EXPECT_TRUE(lmptr_->addLease(leases[1]));
// @todo: Simply use HWAddr directly once 2589 is implemented
- Lease4Ptr returned = lmptr_->getLease4(HWAddr(leases[1]->hwaddr_, HTYPE_ETHER),
+ Lease4Ptr returned = lmptr_->getLease4(HWAddr(leases[1]->hwaddr_,
+ HTYPE_ETHER),
leases[1]->subnet_id_);
ASSERT_TRUE(returned);
detailCompareLease(leases[1], returned);
(void) lmptr_->deleteLease(leases[1]->addr_);
}
- // Expect some error when getting a lease with too long a hardware
- // address. Set the contents of each byte to 42, a random value.
- // @todo Check if there is some way of detecting that data added
- // to the database is truncated. There does not appear to
- // be any indication in the C API.
- leases[1]->hwaddr_.resize(Lease4::HWADDR_MAX + 100, 42);
- EXPECT_TRUE(lmptr_->addLease(leases[1]));
- // @todo: Simply use HWAddr directly once 2589 is implemented
- Lease4Ptr returned = lmptr_->getLease4(HWAddr(leases[1]->hwaddr_, HTYPE_ETHER),
- leases[1]->subnet_id_);
- EXPECT_FALSE(returned);
+ // Database should not let us add one that is too big
+ // (The 42 is a random value put in each byte of the address.)
+ leases[1]->hwaddr_.resize(HWAddr::MAX_HWADDR_LEN + 100, 42);
+ EXPECT_THROW(lmptr_->addLease(leases[1]), isc::dhcp::DbOperationError);
}
/// @brief Check GetLease4 methods - access by Client ID
More information about the bind10-changes
mailing list