Add split Method to Subnet for Dividing Network Ranges into Smaller Subnets

[alissn]

Hi,

This pull request introduces a new split method to the IPLib\Range\Subnet class, enabling the division of a network range into smaller subnets. This feature supports both IPv4 and IPv6, making it versatile for various networking scenarios.

Usage

$subnet = \IPLib\Range\Subnet::parseString('192.168.112.203/24');
$smallerSubnets = $subnet->split(25);

Result

[
    0 => IPLib\Range\Subnet {
        #fromAddress: IPLib\Address\IPv4 {
            #address: "192.168.112.0"
            #bytes: null
            #rangeType: null
            #comparableString: null
        }
        #toAddress: IPLib\Address\IPv4 {
            #address: "192.168.112.127"
            #bytes: null
            #rangeType: null
            #comparableString: null
        }
        #networkPrefix: 25
        #rangeType: null
    },
    1 => IPLib\Range\Subnet {
        #fromAddress: IPLib\Address\IPv4 {
            #address: "192.168.112.128"
            #bytes: null
            #rangeType: null
            #comparableString: null
        }
        #toAddress: IPLib\Address\IPv4 {
            #address: "192.168.112.255"
            #bytes: null
            #rangeType: null
            #comparableString: null
        }
        #networkPrefix: 25
        #rangeType: null
    }
]

Key Benefits

• IPv4 and IPv6 Support: Easily split both IPv4 and IPv6 ranges.
• Improved Efficiency: Simplifies the process of working with smaller subnets from a larger range.

Let me know if additional details or adjustments are needed!

[mlocati]

Why do we need to increase the minimum supported PHP version?

[alissn]

Hi @mlocati,

Thank you for reviewing my pull request.

To handle large IP ranges efficiently, I used a generator class. This suggestion is from the IDE, recommending that the PHP version be updated to at least 5.5.

[mlocati]

Yel, generators require PHP 5.5. But I don't think I'd increase the minimum PHP version just for this reason

[alissn]

@mlocati,

I have updated the code to return an array instead of a generator instance.

[mlocati]

I'd

• remove the RangeSplitInterface interace and let any range class accept the split method (for "Range\Single" it will simply return an array with a single address)
• add tests

[coveralls]

coverage: 98.552% (-0.07%) from 98.622%
when pulling d1f5a94 on alissn:AddSplitMethod
into 67ce14c on mlocati:main.

[alissn]

Hi @mlocati,

Do you have any feedback on this pull request?

[mlocati]

Do you have any feedback on this pull request?

@alissn The automated actions already gave you a feedback 😉 - see https://github.com/mlocati/ip-lib/pull/93/checks

[alissn]

@alissn The automated actions already gave you a feedback 😉 - see https://github.com/mlocati/ip-lib/pull/93/checks

@mlocati
I have fixed the test.

However, PHP 5.3, 5.4, and 5.5 are no longer relevant for me and many others. 🙂
For more details, you can check the official PHP EOL page: https://www.php.net/eol.php.

[mlocati]

@alissn I've updated the code of this pull request.
But your code only works if the range is a subnet, right? What about the other 2 types of ranges?

[mlocati]

Also: please add some tests for the Single and Pattern classes (I don't think we need new test methods: adding cases to the two existing providers should be enough)

[alissn]

@mlocati
I have completed all the reviews.

However, when passing a pattern-based range to the split method, it cannot return an array of \IPLib\Range\Pattern instances. Instead, I had to return \IPLib\Range\Subnet objects.

[alissn]

        $systemBitness = PHP_INT_SIZE * 8;
        $minPrefixByBitness = $maxPrefix - $systemBitness + 2;
        if ($minPrefixByBitness > $networkPrefix) {
            throw new OverflowException("The value of \$networkPrefix leads to too large ranges for the current machine bitness (you can use a value of at least {$minPrefixByBitness})");
        }

I removed this section because the $networkPrefix passed to the function cannot be smaller than the current range.

Additionally, an error is already handled in the following section:

if ($networkPrefix < $myNetworkPrefix) {
    throw new OutOfBoundsException("The value of the \$networkPrefix parameter can't be smaller than the network prefix of the range ({$myNetworkPrefix})");
}

[mlocati]

You shouldn't remove the chunk of code that throws an OverflowException: it handles the case when pow(2, ...) returns a non-integer value

[alissn]

Reverted the change, but I couldn't add a test for it. If you can, please add a test case to ensure the exception is thrown.

[mlocati]

please add a test case to ensure the exception is thrown.

It's already there:

ip-lib/test/tests/Ranges/RangesSplitTest.php

Line 588 in 410ce39

if ($minNetworkPrefixFor32BitSystems !== null && PHP_INT_SIZE === 4) {

[alissn]

Based on the coverage report here this code path isn't covered by tests.

[mlocati]

We send to Coveralls only one coverage, and it's for a case where we use a 64- bit PHP.
Maybe we can change that case to use a 32-bit PHP

[mlocati]

Thanks!

[mlocati]

See also #96 (which required #94 and #95)