Possible Bug in DHCP Server - Devs

[meesterk]

Not sure if this is the best place to post this issue, but as it is related to the DHCP Server, it seems to make sense.

pfSense Version:    2.3.1-RELEASE(amd64)

I set the system up to serve out DHCP addresses in the subnet of 10.2.0.0/24. This all worked fine and dandy, but for various reasons I am running a separate BIND server, hence I am not using the builtin DNS Forwarder or Resolver. I did however setup the system to register leased DHCP addresses on behalf of the clients by the pfSense router. This I believe worked for adding a forward map to the corresponding domain zone on the BIND server (I say believe, because I can't for sure recall if it worked for forwards before I modified the services.inc file to add the ddns-domainname and ddns-rev-domainname), either way, forwards worked as expected. Where I ran into trouble was with the reverse maps…these were not working as I expected. Now, for times sake (namely my time), I will skip a few things and go straight to the issue at hand, there appears to be a bug with the 'services.inc' script (this is the script that dynamically generates the dhcpd.conf file upon boot). What seems to be the issue is that even though the subnect 10.2.0.0/24 is a perfectly valid subnet....the code in the services.inc file does not handle the '0' in the third octet well. So, what happens is that the system generates a 'zone' entry in the dhcpd.conf file that looks like this:

zone 02.10.in-addr.arpa {
        primary 10.2.1.200;
}

When I saw this I thought this was very strange...and low it is! After quite a bit of digging, I believe I found the offending code in the services.inc file (line 682 - 713):
##############################################################################
$revsubnet = array_reverse(explode('.',$subnet));

/* Take care of full classes first /
switch ($ifcfgsn) {
case 8:
$start_octet = 3;
break;
case 16:
$start_octet = 2;
break;
case 24:
$start_octet = 1;
break;
default:
$start_octet = 0;
/ Add subnet bitmask to first octet */
$revsubnet[0] .= '-' . $ifcfgsn;
break;

}

$ptr_domain = '';
for ($octet = 0; $octet <= 3; $octet++) {
if ($octet < $start_octet) {
continue;
}
$ptr_domain .= (empty($ptr_domain) ? '' : '.');
$ptr_domain .= $revsubnet[$octet];
}
$ptr_domain .= ".in-addr.arpa";
$newzone['ptr-domain'] = $ptr_domain;
unset($ptr_domain, $revsubnet, $start_octet);
##############################################################################

The specific offending line is (line 708):
$ptr_domain .= (empty($ptr_domain) ? '' : '.');

This makes perfect sense once you take the time to write a small test routine like I did with the above incorporated (includes a couple of functions from util.inc).

The issue here is that the PHP 'empty()' function returns true for a value of '0', hence instead of adding a '.' which is what I am guessing is the desired effect, it concatenates a blank. Then, the next line adds the value of the octet. So, basically this is how you end up with an incorrect 'zone' entry in the dhcpd.conf file.

So, to the devs, I have three requests:

1. Verify if this is a bug or intentional (I'm thinking bug)
2. If it is a bug, please correct it (i.e.: don't use 'empty()')
3. Please add an option in the WebConfigurator to add 'ddns-rev-domainname' to the dhcpd.conf (I would like reverse maps with no hassle  8))