Fix numeric strings being misinterpreted as binary strings

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## [Unreleased]
 
+### Fixed
+
+- `IP::plus()` and `IP::minus()` no longer misinterpret 4-character numeric strings (e.g. `'1000'`) as binary "packed" strings, which caused `IPBlock` iteration to yield addresses outside of the block [#74](https://github.com/rlanvin/php-ip/issues/74)
+
 ## [3.0.0] - 2022-03-02
 
 ### Backward Compatibility Breaking Changes

src/IP.php

@@ -401,7 +401,14 @@ public function plus($value): IP
         }
 
         if (!$value instanceof self) {
-            $value = new static($value);
+            if (is_string($value) && ctype_digit($value)) {
+                // numeric strings must bypass the constructor's auto-detection,
+                // which would treat a string of NB_BYTES characters (e.g. "1000")
+                // as a binary "packed" string
+                $value = new static(static::initGmpFromNumericString($value));
+            } else {
+                $value = new static($value);
+            }
         }
 
         $result = gmp_add($this->ip, $value->ip);
@@ -438,7 +445,14 @@ public function minus($value): IP
         }
 
         if (!$value instanceof self) {
-            $value = new static($value);
+            if (is_string($value) && ctype_digit($value)) {
+                // numeric strings must bypass the constructor's auto-detection,
+                // which would treat a string of NB_BYTES characters (e.g. "1000")
+                // as a binary "packed" string
+                $value = new static(static::initGmpFromNumericString($value));
+            } else {
+                $value = new static($value);
+            }
         }
 
         $result = gmp_sub($this->ip, $value->ip);

src/IPBlock.php

@@ -574,7 +574,7 @@ public function getIterator(): \Generator
         $position = gmp_init(0);
 
         while (gmp_cmp($position, 0) >= 0 && gmp_cmp($position, $this->getNbAddresses()) < 0) {
-            yield $this->first_ip->plus(gmp_strval($position));
+            yield $this->first_ip->plus($position);
             $position = gmp_add($position, 1);
         }
     }

tests/IPTest.php

@@ -86,6 +86,10 @@ public function validOperations()
             ['ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff', -1, null, 'ffff:ffff:ffff:ffff:ffff:ffff:ffff:fffe'],
             ['::', 1, null, '::1'],
             ['::', null, -1, '::1'],
+            // numeric strings of 4 characters must not be mistaken
+            // for binary "packed" strings (see issue #74)
+            ['0.0.0.0',         '1000',           null,             '0.0.3.232'],
+            ['0.0.3.232',       null,             '1000',           '0.0.0.0'],
         ];
     }
 

tests/IPv4BlockTest.php

@@ -45,6 +45,24 @@ public function testIterator()
         $this->assertEquals($expectation, iterator_to_array($subnet->getIterator()));
     }
 
+    /**
+     * Iterating past the 1000th address used to yield addresses outside of
+     * the block, because the numeric position was mistaken for a binary
+     * "packed" string (see issue #74).
+     */
+    public function testIteratorStaysWithinBlock()
+    {
+        $subnet = new IPv4Block('35.35.32.0/22');
+
+        $ips = iterator_to_array($subnet->getIterator());
+
+        $this->assertCount(1024, $ips);
+        $this->assertEquals('35.35.32.0', (string) $ips[0]);
+        $this->assertEquals('35.35.35.231', (string) $ips[999]);
+        $this->assertEquals('35.35.35.232', (string) $ips[1000]);
+        $this->assertEquals('35.35.35.255', (string) $ips[1023]);
+    }
+
     public function testGetPrivateBlocks()
     {
         $private_blocks = IPv4Block::getPrivateBlocks();