Skip to content
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.

Sign up for GitHub

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

🎨 Use filter_var to check IPv4 and IPv6 addresses #10

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

🎨 Use filter_var to check IPv4 and IPv6 addresses #10

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
41b4a0a
:art: Use filter_var to check IPv4 and IPv6 addresses
rudnerbjoern Dec 5, 2024
605298a
Update src/vSphereIPv4AddressCollector.class.inc.php
rudnerbjoern Dec 6, 2024
20700d8
Update src/vSphereIPv4AddressCollector.class.inc.php
rudnerbjoern Dec 6, 2024
5d17db7
Update src/vSphereIPv4AddressCollector.class.inc.php
rudnerbjoern Dec 6, 2024
034aa14
remove local vscode settings from git
rudnerbjoern Dec 6, 2024
4cd0e75
Remove redundant checks for empty strings
rudnerbjoern Dec 6, 2024
dc1c67f
Update src/vSphereVirtualMachineCollector.class.inc.php
rudnerbjoern Dec 6, 2024
f19faa1
Update src/vSphereVirtualMachineCollector.class.inc.php
rudnerbjoern Dec 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
.vscode/settings.json
90 changes: 42 additions & 48 deletions src/vSphereIPv4AddressCollector.class.inc.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,23 +68,21 @@ public function GetIPv4Addresses()
$aVMs = vSphereVirtualMachineCollector::GetVMs();
foreach ($aVMs as $oVM) {
$sIP = $oVM['managementip'] ?? '';
if ($sIP != '') {
if (strpos($sIP, ':') == false) {
Utils::Log(LOG_DEBUG, 'IPv4 Address: '.$sIP);
if (in_array('short_name', $oVM)) {
$sShortName = explode('.', $oVM['short_name'])[0]; // Remove chars after '.', if any
} else {
$sShortName = '';
}
$this->aIPv4Addresses[] = array(
'id' => $sIP,
'ip' => $sIP,
'org_id' => $sDefaultOrg,
'ipconfig_id' => $sDefaultOrg,
'short_name' => $sShortName,
'status' => $this->oCollectionPlan->GetTeemIpOption('default_ip_status'),
);
if (filter_var($sIP, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also rewrite the following:

				$sIP = $oVM['managementip'] ?? '';
				if (filter_var($sIP, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) {

To something like:

				$sIP = filter_var($oVM['managementip'] ?? '', FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)
				if ($sIP !== false) {

Or just if ($sIP) {, but I think Combodo would prefer the one above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we discussed that. But to me it "hurts" to see a bool written into a variable pretending to be a string. (I know now that this is valid PHP, but it just feels not right for me to do this)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please reopen conversation to let Combodo decide on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry

Utils::Log(LOG_DEBUG, 'IPv4 Address: ' . $sIP);
if (in_array('short_name', $oVM)) {
$sShortName = explode('.', $oVM['short_name'])[0]; // Remove chars after '.', if any
} else {
$sShortName = '';
}
$this->aIPv4Addresses[] = array(
'id' => $sIP,
'ip' => $sIP,
'org_id' => $sDefaultOrg,
'ipconfig_id' => $sDefaultOrg,
'short_name' => $sShortName,
'status' => $this->oCollectionPlan->GetTeemIpOption('default_ip_status'),
);
}
}
}
Expand All @@ -93,18 +91,16 @@ public function GetIPv4Addresses()
$aServers = vSphereServerCollector::CollectServerInfos();
foreach ($aServers as $oServer) {
$sIP = $oServer['managementip_id'] ?? '';
if ($sIP != '') {
if (strpos($sIP, ':') == false) {
Utils::Log(LOG_DEBUG, 'IPv4 Address: '.$sIP);
$this->aIPv4Addresses[] = array(
'id' => $sIP,
'ip' => $sIP,
'org_id' => $sDefaultOrg,
'ipconfig_id' => $sDefaultOrg,
'short_name' => '',
'status' => $sDefaulIpStatus,
);
}
if (filter_var($sIP, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) {
Utils::Log(LOG_DEBUG, 'IPv4 Address: ' . $sIP);
$this->aIPv4Addresses[] = array(
'id' => $sIP,
'ip' => $sIP,
'org_id' => $sDefaultOrg,
'ipconfig_id' => $sDefaultOrg,
'short_name' => '',
'status' => $sDefaulIpStatus,
);
}
}
}
Expand All @@ -113,25 +109,23 @@ public function GetIPv4Addresses()
$aLnkInterfaceIPAddressses = vSpherelnkIPInterfaceToIPAddressCollector::GetLnks();
foreach ($aLnkInterfaceIPAddressses as $oLnkInterfaceIPAddresss) {
$sIP = $oLnkInterfaceIPAddresss['ipaddress_id'] ?? '';
if ($sIP != '') {
if (strpos($sIP, ':') == false) {
// Check if address is already listed as it may be that vSphere reported it as management IP too
// Don't register duplicates otherwise
$sKey = false;
if (!empty($this->aIPv4Addresses)) {
$sKey = array_search($sIP, array_column($this->aIPv4Addresses, 'ip'));
}
if ($sKey === false) {
Utils::Log(LOG_DEBUG, 'IPv4 Address: '.$sIP);
$this->aIPv4Addresses[] = array(
'id' => $sIP,
'ip' => $sIP,
'org_id' => $sDefaultOrg,
'ipconfig_id' => $sDefaultOrg,
'short_name' => '',
'status' => $sDefaulIpStatus,
);
}
if (filter_var($sIP, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) {
// Check if address is already listed as it may be that vSphere reported it as management IP too
// Don't register duplicates otherwise
$sKey = false;
if (!empty($this->aIPv4Addresses)) {
$sKey = array_search($sIP, array_column($this->aIPv4Addresses, 'ip'));
}
if ($sKey === false) {
Utils::Log(LOG_DEBUG, 'IPv4 Address: ' . $sIP);
$this->aIPv4Addresses[] = array(
'id' => $sIP,
'ip' => $sIP,
'org_id' => $sDefaultOrg,
'ipconfig_id' => $sDefaultOrg,
'short_name' => '',
'status' => $sDefaulIpStatus,
);
}
}
}
Expand Down Expand Up @@ -178,4 +172,4 @@ public function Fetch()

return false;
}
}
}
90 changes: 42 additions & 48 deletions src/vSphereIPv6AddressCollector.class.inc.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,23 +68,21 @@ public function GetIPv6Addresses()
$aVMs = vSphereVirtualMachineCollector::CollectVMInfos();
foreach ($aVMs as $oVM) {
$sIP = $oVM['managementip_id'] ?? '';
if ($sIP != '') {
if (strpos($sIP, ':') !== false) {
Utils::Log(LOG_DEBUG, 'IPv6 Address: '.$sIP);
if (in_array('short_name', $oVM)) {
$sShortName = explode('.', $oVM['short_name'])[0]; // Remove chars after '.', if any
} else {
$sShortName = '';
}
$this->aIPv6Addresses[] = array(
'id' => $sIP,
'ip' => $sIP,
'org_id' => $sDefaultOrg,
'ipconfig_id' => $sDefaultOrg,
'short_name' => $sShortName,
'status' => $sDefaulIpStatus,
);
if (filter_var($sIP, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) {
Utils::Log(LOG_DEBUG, 'IPv6 Address: ' . $sIP);
if (in_array('short_name', $oVM)) {
$sShortName = explode('.', $oVM['short_name'])[0]; // Remove chars after '.', if any
} else {
$sShortName = '';
}
$this->aIPv6Addresses[] = array(
'id' => $sIP,
'ip' => $sIP,
'org_id' => $sDefaultOrg,
'ipconfig_id' => $sDefaultOrg,
'short_name' => $sShortName,
'status' => $sDefaulIpStatus,
);
}
}
}
Expand All @@ -93,18 +91,16 @@ public function GetIPv6Addresses()
$aServers = vSphereServerCollector::CollectServerInfos();
foreach ($aServers as $oServer) {
$sIP = $oServer['managementip_id'] ?? '';
if ($sIP != '') {
if (strpos($sIP, ':') !== false) {
Utils::Log(LOG_DEBUG, 'IPv4 Address: '.$sIP);
$this->aIPv6Addresses[] = array(
'id' => $sIP,
'ip' => $sIP,
'org_id' => $sDefaultOrg,
'ipconfig_id' => $sDefaultOrg,
'short_name' => '',
'status' => $sDefaulIpStatus,
);
}
if (filter_var($sIP, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) {
Utils::Log(LOG_DEBUG, 'IPv6 Address: ' . $sIP);
$this->aIPv6Addresses[] = array(
'id' => $sIP,
'ip' => $sIP,
'org_id' => $sDefaultOrg,
'ipconfig_id' => $sDefaultOrg,
'short_name' => '',
'status' => $sDefaulIpStatus,
);
}
}
}
Expand All @@ -113,25 +109,23 @@ public function GetIPv6Addresses()
$aLnkInterfaceIPAddressses = vSpherelnkIPInterfaceToIPAddressCollector::GetLnks();
foreach ($aLnkInterfaceIPAddressses as $oLnkInterfaceIPAddresss) {
$sIP = $oLnkInterfaceIPAddresss['ipaddress_id'] ?? '';
if ($sIP != '') {
if (strpos($sIP, ':') !== false) {
// Check if address is already listed as it may be that vSphere reported it as management IP too
// Don't register duplicates otherwise
$sKey = false;
if (!empty($this->aIPv6Addresses)) {
$sKey = array_search($sIP, array_column($this->aIPv6Addresses, 'ip'));
}
if ($sKey === false) {
Utils::Log(LOG_DEBUG, 'IPv6 Address: '.$sIP);
$this->aIPv6Addresses[] = array(
'id' => $sIP,
'ip' => $sIP,
'org_id' => $sDefaultOrg,
'ipconfig_id' => $sDefaultOrg,
'short_name' => '',
'status' => $sDefaulIpStatus,
);
}
if (filter_var($sIP, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) {
// Check if address is already listed as it may be that vSphere reported it as management IP too
// Don't register duplicates otherwise
$sKey = false;
if (!empty($this->aIPv6Addresses)) {
$sKey = array_search($sIP, array_column($this->aIPv6Addresses, 'ip'));
}
if ($sKey === false) {
Utils::Log(LOG_DEBUG, 'IPv6 Address: ' . $sIP);
$this->aIPv6Addresses[] = array(
'id' => $sIP,
'ip' => $sIP,
'org_id' => $sDefaultOrg,
'ipconfig_id' => $sDefaultOrg,
'short_name' => '',
'status' => $sDefaulIpStatus,
);
}
}
}
Expand Down Expand Up @@ -176,4 +170,4 @@ public function Fetch()

return false;
}
}
}
4 changes: 1 addition & 3 deletions src/vSphereServerCollector.class.inc.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,7 @@ protected static function DoCollectServer($aHyperV)
$sIP = '';
if ($bCollectIps == 'yes') {
// Check if name has IPv4 or "IPv6" format
$sNum = '(\d|[1-9]\d|1\d\d|2[0-4]\d|25[0-5])';
$sPattern = "($sNum\.$sNum\.$sNum\.$sNum)";
if (preg_match($sPattern, $sName) || (($bCollectIPv6Addresses == 'yes') && (strpos($sName, ":") !== false))) {
if (filter_var($sName, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4) || (($bCollectIPv6Addresses == 'yes') && filter_var($sName, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6))) {
$sIP = $sName;
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/vSphereVirtualMachineCollector.class.inc.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ static protected function DoCollectVMInfo($aFarms, $oVirtualMachine, $aVLANs, $i
$sGuestIP = '';
} else {
if (!$bCollectIPv6Addresses) {
$sGuestIP = (strpos($sGuestIP, ':') !== false) ? '' : $sGuestIP;
$sGuestIP = filter_var($sGuestIP, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) ? '' : $sGuestIP;
rudnerbjoern marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand All @@ -317,7 +317,7 @@ static protected function DoCollectVMInfo($aFarms, $oVirtualMachine, $aVLANs, $i
} else {
// ManagementIP cannot be an IPV6 address, if no IPV4 was found above, let's clear the field
// Note: some OpenVM clients report IP addresses with a trailing space, so let's trim the field
$aData['managementip'] = (strpos($sGuestIP, ':') !== false) ? '' : trim($sGuestIP);
$aData['managementip'] = filter_var(trim($sGuestIP), FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) ? '' : trim($sGuestIP);
rudnerbjoern marked this conversation as resolved.
Show resolved Hide resolved
}

return $aData;
Expand All @@ -335,7 +335,7 @@ static protected function DoCollectVMIPs($aMACToNetwork, $oVirtualMachine)
if ($oNICInfo->ipConfig && $oNICInfo->ipConfig->ipAddress) {
foreach ($oNICInfo->ipConfig->ipAddress as $oIPInfo) {
Utils::Log(LOG_DEBUG, "Reading VM's IP and MAC address");
if (strpos($oIPInfo->ipAddress ?? '', ':') !== false) {
if (filter_var($oIPInfo->ipAddress ?? '', FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) {
// It's an IPv6 address
if ($oCollectionPlan->IsTeemIpInstalled() && $bCollectIPv6Addresses) {
$aNWInterfaces[] = array(
Expand All @@ -349,7 +349,7 @@ static protected function DoCollectVMIPs($aMACToNetwork, $oVirtualMachine)
}
} else {
// If we have a guest IP set to IPv6, replace it with the first IPv4 we find
if (strpos($oVirtualMachine->guest->ipAddress ?? '', ":") !== false) {
if (filter_var($oVirtualMachine->guest->ipAddress ?? '', FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) {
$oVirtualMachine->guest->ipAddress = $oIPInfo->ipAddress;
}

Expand Down