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.

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

fix: URI::setSegment() accepts the last +2 segment without Exception #7251

Merged
merged 2 commits into from
Feb 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 7 additions & 3 deletions system/HTTP/URI.php
Original file line number Diff line number Diff line change
Expand Up @@ -562,9 +562,9 @@ public function getSegment(int $number, string $default = ''): string
*/
public function setSegment(int $number, $value)
{
// 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) + 1) {
if ($this->silent) {
Expand All @@ -574,6 +574,10 @@ public function setSegment(int $number, $value)
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--;

$this->segments[$number] = $value;
$this->refreshPath();

Expand Down
19 changes: 12 additions & 7 deletions tests/system/HTTP/URITest.php
Original file line number Diff line number Diff line change
Expand Up @@ -808,32 +808,37 @@ public function testSetSegment()
$this->assertSame('foo/banana/baz', $uri->getPath());
}

public function testSetSegmentFallback()
public function testSetSegmentNewOne()
{
$base = 'http://example.com';
$uri = new URI($base);

$uri = new URI($base);
// Can set the next segment.
$uri->setSegment(1, 'first');
$uri->setSegment(3, 'third');
// Can set the next segment.
$uri->setSegment(2, 'third');

$this->assertSame('first/third', $uri->getPath());

// Can replace the existing segment.
$uri->setSegment(2, 'second');

$this->assertSame('first/second', $uri->getPath());

// Can set the next segment.
$uri->setSegment(3, 'third');

$this->assertSame('first/second/third', $uri->getPath());

$uri->setSegment(5, 'fifth');
// Can set the next segment.
$uri->setSegment(4, 'fourth');

$this->assertSame('first/second/third/fifth', $uri->getPath());
$this->assertSame('first/second/third/fourth', $uri->getPath());

// sixth or seventh was not set
// Cannot set the next next segment.
$this->expectException(HTTPException::class);

$uri->setSegment(8, 'eighth');
$uri->setSegment(6, 'six');
}

public function testSetBadSegment()
Expand Down
9 changes: 9 additions & 0 deletions user_guide_src/source/changelogs/v4.4.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@ BREAKING
Behavior Changes
================

URI::setSegment() and Non-Existent Segment
------------------------------------------

An exception is now thrown when you set the last ``+2`` segment.
In previous versions, an exception was thrown only if the last segment ``+3``
or more was specified. See :ref:`upgrade-440-uri-setsegment`.

The next segment (``+1``) of the current last segment can be set as before.

Interface Changes
=================

Expand Down
13 changes: 13 additions & 0 deletions user_guide_src/source/installation/upgrade_440.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,19 @@ Please refer to the upgrade instructions corresponding to your installation meth
Breaking Changes
****************

.. _upgrade-440-uri-setsegment:

URI::setSegment() Change
========================

Dut to a bug, in previous versions an exception was not thrown if the last segment
``+2`` was specified. This bug has been fixed.

If your code depends on this bug, fix the segment number.

.. literalinclude:: upgrade_440/002.php
:lines: 2-

Mandatory File Changes
**********************

Expand Down
12 changes: 12 additions & 0 deletions user_guide_src/source/installation/upgrade_440/002.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

// URI: http://example.com/one/two

// Before:
$uri->setSegment(4, 'three');
// The URI will be http://example.com/one/two/three

// After:
$uri->setSegment(4, 'three'); // Will throw Exception
$uri->setSegment(3, 'three');
// The URI will be http://example.com/one/two/three