Opened 17 months ago

Closed 7 months ago

#4540 closed defect (complete)

Kea should accept hex values for integer options

Reported by: tomek Owned by: fdupont
Priority: low Milestone: Kea1.2-final
Component: remote-management Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: Mozilla Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 1
Total Hours: 1 Internal?: no

Description

Currently Kea is not able to handle hex values (e.g. 0x12) when an integer is expected.

An example config file that defined lw4over6 option is attached. It defines a new custom option type:

    "option-def": [
        {
            "name": "s46-cont-lw",
            "code": 96,
            "space": "dhcp6",
            "type": "record",
            "array": false,
            "record-types": "uint32, ipv6-address, uint32, ipv4-address, uint8, uint32, uint32, uint32, uint8, uint8, uint16",
            "encapsulate": ""
        },
    ],

And then tries to specify values:

"option-data": [
   {
        "name": "s46-cont-lw",
        "data": "0x005a0010,2001:db8::,0x005c0015,2.84.63.1,64,0x20010db8,0x8c0000fe,0x005d0004,0,6,5120"
    }
]

The exception being thrown is as follows:

2016-07-11 17:54:50.492 ERROR [kea-dhcp6.dhcp6/3567] DHCP6_INIT_FAIL failed to initialize Kea server: configuration error using file '/home/thomson/isc/kea-lw4o6.conf': option data does not match option definition (space: dhcp6, code: 96): unable to convert the value '0x005a0010' to integer data type (/home/thomson/isc/kea-lw4o6.conf:54:21)

Subtickets

Attachments (1)

kea-lw4o6.conf (3.6 KB) - added by tomek 17 months ago.

Download all attachments as: .zip

Change History (14)

Changed 17 months ago by tomek

comment:1 Changed 17 months ago by fdupont

The error is from OptionDefinition::lexicalCastWithRangeCheck:

    int64_t result = 0;
    try {
        result = boost::lexical_cast<int64_t>(value_str);

    } catch (const boost::bad_lexical_cast&) {
        isc_throw(BadDataTypeCast, "unable to convert the value '"
                  << value_str << "' to integer data type");
    }

According to http://stackoverflow.com/questions/1070497/c-convert-hex-string-to-signed-integer we can keep the lexical_cast by inserting a std::hex in the stream (lexical_cast is stream based, BTW this is why it doesn't handle hexadecimal...) and it is more C++ than strtoll().

Last edited 17 months ago by fdupont (previous) (diff)

comment:2 Changed 16 months ago by tomek

  • Milestone changed from Kea-proposed to Outstanding Tasks

We decided to move this to outstanding, due to many other tickets awaiting out attention in 1.1 milestone. If we somehow manage to get all of them done, this ticket will likely be one of the first to be taken from Outstanding.

comment:3 Changed 15 months ago by tomek

  • Milestone changed from Outstanding Tasks to Kea1.2

comment:4 Changed 14 months ago by tomek

  • Component changed from dhcpdb to configuration

comment:5 Changed 13 months ago by tomek

  • Component changed from configuration to remote-management

comment:6 Changed 11 months ago by tomek

  • Priority changed from medium to low

comment:7 Changed 10 months ago by fdupont

  • Owner set to fdupont
  • Status changed from new to accepted

comment:8 Changed 10 months ago by fdupont

  • Owner changed from fdupont to UnAssigned
  • Status changed from accepted to reviewing

Done. Ready for review. BTW what to put as ChangeLog entry? Should be a doc updated?

comment:9 Changed 9 months ago by tomek

  • Sub-Project changed from DHCP to Mozilla

comment:10 Changed 8 months ago by tomek

  • Milestone changed from Kea1.2 to Kea1.2-final

Code freeze for 1.2-beta. Moving all remaining open tickets to 1.2-final.

comment:11 Changed 7 months ago by tomek

  • Owner changed from UnAssigned to tomek

comment:12 follow-up: Changed 7 months ago by tomek

  • Add Hours to Ticket changed from 0 to 1
  • Owner changed from tomek to fdupont
  • Total Hours changed from 0 to 1

I have reviewed your changes and they are good. However, they are not sufficient. The changes also needed the following:

  • update User's Guide to say how hex should be formatted: that plain hex (abcd) or 0x prefixed (0xabcd) are both accepted
  • update at least one v4 and one v6 example to show how to define hex values.

I did both. Please pull and review. If you're ok with those changes, this code is ready for merge.

The changes compiled ok and unit-tests passed on Ubuntu 16.04 x64.

This change requires a changelog. If you don't have any better proposals, here's a text you can use:

12xx.	[func]		fdupont
	Integer option values can now be specified in either dec od hex
	format.
	(Trac #4540, git tbd)

comment:13 in reply to: ↑ 12 Changed 7 months ago by fdupont

  • Resolution set to complete
  • Status changed from reviewing to closed

Replying to tomek:

I have reviewed your changes and they are good. However, they are not sufficient. The changes also needed the following:

  • update User's Guide to say how hex should be formatted: that plain hex (abcd) or 0x prefixed (0xabcd) are both accepted
  • update at least one v4 and one v6 example to show how to define hex values.

I did both. Please pull and review. If you're ok with those changes, this code is ready for merge.

=> I am happy with the changes but they were incomplete so I added a note in the guide and a Kea6 example.

The changes compiled ok and unit-tests passed on Ubuntu 16.04 x64.

This change requires a changelog. If you don't have any better proposals, here's a text you can use:

12xx.	[func]		fdupont
	Integer option values can now be specified in either dec od hex
	format.
	(Trac #4540, git tbd)

Merged. Closing. BTW I had to edit both multiple-options.json examples to resolve conflicts.

Note: See TracTickets for help on using tickets.