Commit cac02e92 authored by Thomas Markwalder's avatar Thomas Markwalder
Browse files

[master] Merge branch 'trac2837' always use STRICT_SQL mode in

mysql_lease_mgr.
parents 403c7178 91a0d273
......@@ -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_));
}
......
......@@ -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();
......
......@@ -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.
......
......@@ -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
///
......
......@@ -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,
......
......@@ -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
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment