When $_SERVER['REMOTE_ADDR'] is IPv6 Session::getFromRequest() triggers MySQL error.

[hparadiz]

Recently encountered this bug when running a site localhost on my mac. The IP was ::1 but ip2long was giving me a null value so the generated query was invalid.

emergence-skeleton/php-classes/Session.class.php

Lines 60 to 65 in 6768634

	public static function getFromRequest($create = true)
	{
	$sessionData = array(
	'LastIP' => !empty($_SERVER['REMOTE_ADDR']) ? ip2long($_SERVER['REMOTE_ADDR']) : null
	,'LastRequest' => time()
	);

Considering the architectural challenges with Session data. What do you think about creating a second field for ipv6 addresses?

binary (16) as per this article is suggested: https://medium.com/@richardoosterhof/optimize-php-and-mysql-for-ipv6-daab664962e2

[themightychris]

@hparadiz great report, thanks! What error/log printer are you using in that screenshot BTW?

It sounds like the inet_pton/inet_ntop functions suggested in that article supports both IPv4 and IPv6 addresses. So my preferred solution if we can't find any reason why this wouldn't work would be to just convert the Session class to using those methods for all IPs, and include a DB migration alongside that update that changes the session IP column to a longer field and converts all the existing values. Is there any reason that would be too risky?

[hparadiz]

It's filp/whoops

Here's an implementation example for SQL errors:
https://github.com/hparadiz/divergence/blob/master/classes/Divergence/IO/Database/MySQL.php#L400-L414

I like the idea of keeping it as a single binary field.

This is honestly not that breaking of a change even if you truncate the sessions table on an existing site (which I do anyway for sessions older than 30 days) but I can see why you would want a migration script ready on your end.

[hparadiz]

I did it on Technex.us
https://github.com/hparadiz/technexus/blob/master/classes/technexus/Models/Session.php

Need to follow up on the SQL class and make sure it generated a create query for the binary field correctly.

Be prepared for interesting text values for the field 😄