fix: URI::getSegment() returns non-existent segment

system/HTTP/URI.php

@@ -537,18 +537,22 @@ public function getSegments(): array
      * @param string $default Default value
      *
      * @return string The value of the segment. If no segment is found,
-     *                throws InvalidArgumentError
+     *                throws HTTPException
      */
     public function getSegment(int $number, string $default = ''): string
     {
-        // The segment should treat the array as 1-based for the user
-        // but we still have to deal with a zero-based array.
-        $number--;
+        if ($number < 1) {
+            throw HTTPException::forURISegmentOutOfRange($number);
+        }
 
         if ($number > count($this->segments) && ! $this->silent) {
             throw HTTPException::forURISegmentOutOfRange($number);
         }
 
+        // The segment should treat the array as 1-based for the user
+        // but we still have to deal with a zero-based array.
+        $number--;
+
         return $this->segments[$number] ?? $default;
     }
 

tests/system/HTTP/URITest.php

@@ -62,37 +62,58 @@ public function testSegmentsIsPopulatedRightForMultipleSegments()
         $this->assertSame('path', $uri->getSegment(1));
         $this->assertSame('to', $uri->getSegment(2));
         $this->assertSame('script', $uri->getSegment(3));
-        $this->assertSame('', $uri->getSegment(4));
 
         $this->assertSame(3, $uri->getTotalSegments());
     }
 
     public function testSegmentOutOfRange()
     {
         $this->expectException(HTTPException::class);
+
         $uri = new URI('http://hostname/path/to/script');
-        $uri->getSegment(5);
+        $uri->getSegment(4);
+    }
+
+    public function testSegmentOutOfRangeZero()
+    {
+        $this->expectException(HTTPException::class);
+
+        $uri = new URI('http://hostname/path/to/script');
+        $uri->getSegment(0);
+    }
+
+    public function testSegmentOutOfRangeNegative()
+    {
+        $this->expectException(HTTPException::class);
+
+        $uri = new URI('http://hostname/path/to/script');
+        $uri->getSegment(-1);
     }
 
     public function testSegmentOutOfRangeWithSilent()
     {
         $url = 'http://abc.com/a123/b/c';
         $uri = new URI($url);
+
+        $this->assertSame('', $uri->setSilent()->getSegment(4));
         $this->assertSame('', $uri->setSilent()->getSegment(22));
     }
 
     public function testSegmentOutOfRangeWithDefaultValue()
     {
         $this->expectException(HTTPException::class);
+
         $url = 'http://abc.com/a123/b/c';
         $uri = new URI($url);
+
         $uri->getSegment(22, 'something');
     }
 
     public function testSegmentOutOfRangeWithSilentAndDefaultValue()
     {
         $url = 'http://abc.com/a123/b/c';
         $uri = new URI($url);
+
         $this->assertSame('something', $uri->setSilent()->getSegment(22, 'something'));
     }
 

tests/system/Pager/PagerTest.php

@@ -168,6 +168,8 @@ public function testStoreWithQueries()
 
     public function testStoreWithSegments()
     {
+        $this->createPager('/3?page=3&foo=bar');
+
         $_GET['page'] = 3;
         $_GET['foo']  = 'bar';
 

user_guide_src/source/changelogs/v4.4.0.rst

@@ -20,6 +20,13 @@ BREAKING
 Behavior Changes
 ================
 
+URI::getSegment() and Non-Existent Segment
+------------------------------------------
+
+An exception is now thrown whenever a non-existent segment number is passed.
+In previous versions, an exception was not thrown if the last segment ``+1`` was
+specified.
+
 Interface Changes
 =================
 

user_guide_src/source/installation/upgrade_440.rst

@@ -15,6 +15,18 @@ Please refer to the upgrade instructions corresponding to your installation meth
 Breaking Changes
 ****************
 
+URI::getSegment() Change
+========================
+
+Dut to a bug, in previous versions an exception was not thrown if the last segment
+``+1`` was specified. This bug has been fixed. If the non-existent segment is
+specified, an exception is always thrown.
+
+If your code depends on this bug, add ``->setSilent()`` before the call.
+
+.. literalinclude:: upgrade_440/001.php
+   :lines: 2-
+
 Mandatory File Changes
 **********************
 

user_guide_src/source/installation/upgrade_440/001.php

@@ -0,0 +1,5 @@
+<?php
+
+$uri->getSegment(2);
+// ↓
+$uri->setSilent()->getSegment(2);

user_guide_src/source/libraries/uri/024.php

@@ -2,13 +2,17 @@
 
 // URI = http://example.com/users/15/profile
 
+echo $uri->getSegment(1, 'foo');
+// will print 'users'
+
+echo $uri->getSegment(3, 'bar');
 // will print 'profile'
-echo $uri->getSegment(3, 'foo');
-// will print 'bar'
-echo $uri->getSegment(4, 'bar');
+
+echo $uri->getSegment(4, 'baz');
 // will throw an exception
-echo $uri->getSegment(5, 'baz');
+
+echo $uri->setSilent()->getSegment(4, 'baz');
 // will print 'baz'
-echo $uri->setSilent()->getSegment(5, 'baz');
+
+echo $uri->setSilent()->getSegment(4);
 // will print '' (empty string)
-echo $uri->setSilent()->getSegment(5);