BIND 10 #2837: MySQL STRICT mode causes libdhcpsrv unit tests t fail when MySQL STRICT mode is enabled

[BIND 10 Development]

#2837: MySQL STRICT mode causes libdhcpsrv unit tests t fail when MySQL STRICT
mode is enabled
-------------------------------------+-------------------------------------
            Reporter:  tmark         |                        Owner:
                Type:  defect        |  stephen
            Priority:  medium        |                       Status:
           Component:  libdhcp       |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-DHCP-20130411
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  0             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by tmark):

 * owner:  tmark => stephen


Comment:

 Replying to [comment:5 stephen]:
 > '''src/lib/dhcp/hwaddr.cc'''[[BR]]
 > HWAddr constructors: even single-line "if" blocks should use "{" and "}"
 (see the [wiki:CodingGuidelines#CurlyBraces coding guidelines]).
 >

 Fixed.


 > I think you can leave the initializations of the member hwaddr_ as they
 were (i.e. created with the passed arguments) and just have the length
 check in the constructor.  At worst there is additional overhead of
 initializing hwaddr_ with the data if the exception is thrown (as opposed
 to the overhead of constructing hwaddr_ and copying data to it in separate
 steps in all cases when an exception is not thrown).
 >

 Agreed, Fixed.

 > '''src/lib/dhcp/tests/hwaddr_unittest.cc'''[[BR]]
 > HWAddrTest: Please include spaces around the "+" in the initialization
 of big_data.
 >
 > HWAddrTest: It is probably better to initialize big_data explicitly (via
 std::fill or memset); I recall a problem on one system (can't remember
 which, probably Solaris) when initializing an array with fewer elements
 than the size of the array.  In fact, an even better solution is to create
 big_data_vector using:
 > {{{
 > vector<uint8_t> big_data_vector(HWAddr::MAX_HWADDR_LEN + 1, 0);
 > }}}
 > (although the "0" is not really necessary - it's the default) and, when
 testing the constructor that requires a base address and size, pass
 {{{&big_data_vector[0]}}} and {{{big_data_vector.size()}}} through as
 arguments.
 >

 Good catch. Done.

 > '''src/lib/dhcpsrv/mysql_lease_mgr.cc'''[[BR]]
 > openDatabase: Suggest that the "set" in the command setting the session
 variable be uppercase (matches the style of the prepared statements).
 >
 >

 Done.

 > '''src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc'''[[BR]]
 > Line 884/906: not sure about the reformatting here.  The BIND 10
 standard for continuation lines is the
 [wiki:CodingGuidelines#TabsIndentation BSD style].
 >

 Adjusted per guidelines.

 > '''!ChangeLog entry'''[[BR]]
 > Would rewrite the last sentence as:
 > {{{
 > Also, attempts to create a HWAddr (hardware address) object with
 > too long an array of data now throw an exception.
 > }}}
 >
 > Only needs one git commit number, that of the commit in which the change
 has been merged into master.


 Marcin, corrected me on this on an earlier ticket.  Will use only the
 merge commit.

 ChangeLog text will be:

 Changed mysql_lease_mgr to set the SQL mode option to STRICT. This
 causes mysql it to treat invalid input data as an error. Rather than
 "successfully" inserting a too large value by truncating it, the
 insert will fail, and the lease manager will throw an exception.
 Also, attempts to create a HWAddr (hardware address) object with
 too long an array of data now throw an exception.
 (Trac #2387, git TBD)

-- 
Ticket URL: <http://bind10.isc.org/ticket/2837#comment:6>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development