-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/internet extension #8
Conversation
src/Extensions/InternetExtension.php
Outdated
|
||
public function ipv4(): string | ||
{ | ||
return long2ip($this->randomizer->getInt(0, 1) === 0 ? $this->randomizer->getInt(-2147483648, -2) : $this->randomizer->getInt(16777216, 2147483647)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The negative numbers ensures compatibility with 32 bits systems, but I think we should check all the positive numbers to ensure compatibility with 64 bits systems. Cause ip2long
(invert of long2ip
) always return positive numbers so a lot of 64 bits systems are storing IPs as int from 0
to 4294967295
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems you are right, the documentation is poor on that side, I don't know if I should be avoiding local ips, like in this case, what do you think ?
src/Extensions/InternetExtension.php
Outdated
$res = []; | ||
|
||
for ($i = 0; $i < 8; $i++) { | ||
$res[] = dechex($this->randomizer->getInt(0, 65535)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you prefer this way to ensure compatibility, or to use inet_ntop
with a randomizer, which is the prefered way of using IPv6 when PHP is compiled with IPv6 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did have a look at this but i'm not familiar enough with this function and documentation didn't help me on that, but this isn't a reason, will need to have a look
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are some questions about IPs
@martinsoenen should be fixed 😉 |
Doesn't contain user agent