Open hparadiz opened 6 years ago
@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?
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.
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 😄
Recently encountered this bug when running a site localhost on my mac. The IP was
::1
butip2long
was giving me a null value so the generated query was invalid.https://github.com/JarvusInnovations/emergence-skeleton/blob/676863437d2c64b1c1effe88ecdc1398244bab54/php-classes/Session.class.php#L60-L65
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