Skip to content

Fix IP validation in digital signature file download (CIDR support)#305

Open
cableman wants to merge 2 commits intoOS2Forms:developfrom
itk-dev:feature/digital_signature_ips
Open

Fix IP validation in digital signature file download (CIDR support)#305
cableman wants to merge 2 commits intoOS2Forms:developfrom
itk-dev:feature/digital_signature_ips

Conversation

@cableman
Copy link
Contributor

@cableman cableman commented Feb 17, 2026

This allow us to filter on CIDR, which is useful in a dynamic setup where the IP may change

@cableman cableman requested a review from jekuaitk February 17, 2026 15:30
@cableman cableman force-pushed the feature/digital_signature_ips branch 2 times, most recently from 7631097 to 83921fd Compare February 17, 2026 15:41
@cableman cableman marked this pull request as ready for review February 17, 2026 19:47
@cableman cableman force-pushed the feature/digital_signature_ips branch from 83921fd to de2282a Compare February 17, 2026 19:50
Comment on lines 116 to 125
$ip = ip2long($ip);
$subnet = ip2long($subnet);

if ($ip === FALSE || $subnet === FALSE || $bits < 0 || $bits > 32) {
return FALSE;
}

$mask = -1 << (32 - $bits);

return ($ip & $mask) === ($subnet & $mask);
Copy link
Contributor

Choose a reason for hiding this comment

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

It is not obvious what and why this is happening. A comment or two would do wonders

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, a simpler way will be to use the IpUtils class from symfony. We switch over to that...

@cableman cableman requested a review from jekuaitk February 18, 2026 08:14
$mask = -1 << (32 - $bits);

return ($ip & $mask) === ($subnet & $mask);
return IpUtils::checkIp($ip, $cidr);
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider just using the IpUtils directly in the above code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments