That's something I was trying to understand as well.
In theory it should not be empty, but apparently it is...
And I would call that a bug, yes.
And it affects SMF 2.1 as well, but not 2.0 so yes it's a bug.
It may be this:
// Some final checking.
if (preg_match('~^((([1]?\d)?\d|2[0-4]\d|25[0-5])\.){3}(([1]?\d)?\d|2[0-4]\d|25[0-5])$~', $this->_ban_ip) === 0 || !isValidIPv6($this->_ban_ip))
$this->_ban_ip = '';
In the past it was just:
// Some final checking.
if (preg_match('~^((([1]?\d)?\d|2[0-4]\d|25[0-5])\.){3}(([1]?\d)?\d|2[0-4]\d|25[0-5])$~', $this->_ban_ip) === 0)
$this->_ban_ip = '';
And it make sense that now is overwriting all the IPs because the regexp in isValidIPv6 doesn't match an IPv4 address.
So the code should be:
// Some final checking.
if (preg_match('~^((([1]?\d)?\d|2[0-4]\d|25[0-5])\.){3}(([1]?\d)?\d|2[0-4]\d|25[0-5])$~', $this->_ban_ip) === 0 && !isValidIPv6($this->_ban_ip))
$this->_ban_ip = '';
I was thinking also to change the ban_ip method to:
public function ban_ip()
{
return !empty($this->_ban_ip) ? $this->_ban_ip : $this->client_ip();
}
Does it make sense?