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: prioritize headers set by the Response class #9235

Merged
merged 5 commits into from
Dec 27, 2024
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
7 changes: 5 additions & 2 deletions system/HTTP/ResponseTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -403,16 +403,19 @@ public function sendHeaders()
if ($value instanceof Header) {
header(
$name . ': ' . $value->getValueLine(),
false,
true,
$this->getStatusCode()
);
} else {
$replace = true;

foreach ($value as $header) {
header(
$name . ': ' . $header->getValueLine(),
false,
$replace,
$this->getStatusCode()
);
$replace = false;
}
}
}
Expand Down
50 changes: 50 additions & 0 deletions tests/system/HTTP/ResponseSendTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -179,4 +179,54 @@ public function testDoNotSendUnSecureCookie(): void
// send it
$response->send();
}

/**
* Make sure that the headers set by the header() function
* are overridden by the headers defined in the Response class.
*/
#[PreserveGlobalState(false)]
#[RunInSeparateProcess]
#[WithoutErrorHandler]
public function testHeaderOverride(): void
{
$response = new Response(new App());
$response->pretend(false);

$body = 'Hello';
$response->setBody($body);

// single header
$response->setHeader('Vary', 'Accept-Encoding');
$this->assertSame('Accept-Encoding', $response->header('Vary')->getValue());

// multiple headers
$response->setHeader('Access-Control-Expose-Headers', 'X-Custom-Header');
$response->addHeader('Access-Control-Expose-Headers', 'Content-Length');
$header = $response->header('Access-Control-Expose-Headers');
$this->assertIsArray($header);
$this->assertSame('X-Custom-Header', $header[0]->getValue());
$this->assertSame('Content-Length', $header[1]->getValue());

// send it
ob_start();
header('Vary: User-Agent');
header('Access-Control-Expose-Headers: Content-Encoding');
header('Allow: GET, POST');
$response->send();
if (ob_get_level() > 0) {
ob_end_clean();
}

// single header
$this->assertHeaderEmitted('Vary: Accept-Encoding');
$this->assertHeaderNotEmitted('Vary: User-Agent');

// multiple headers
$this->assertHeaderEmitted('Access-Control-Expose-Headers: X-Custom-Header');
$this->assertHeaderEmitted('Access-Control-Expose-Headers: Content-Length');
$this->assertHeaderNotEmitted('Access-Control-Expose-Headers: Content-Encoding');

// not overridden by the response class
$this->assertHeaderEmitted('Allow: GET, POST');
}
}
46 changes: 46 additions & 0 deletions user_guide_src/source/changelogs/v4.6.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,48 @@ See :ref:`Upgrading Guide <upgrade-460-sid-change>` for details.

.. _v460-interface-changes:

Headers
-------

The headers set by the ``Response`` class replace those that can be set by the PHP
``header()`` function.

In previous versions, headers set by the ``Response`` class were added to existing
ones - giving no options to change them. That could lead to unexpected behavior when
the same headers were set with mutually exclusive directives.
michalsn marked this conversation as resolved.
Show resolved Hide resolved

For example, session will automatically set headers with the ``header()`` function:

.. code-block:: none

Expires: Thu, 19 Nov 1981 08:52:00 GMT
Cache-Control: no-store, no-cache, must-revalidate
Pragma: no-cache

So if we set **Expires** header one more time we will end up with a duplicated header:

.. code-block:: php

$response->removeHeader('Expires'); // has no effect
return $response->setHeader('Expires', 'Sun, 17 Nov 2024 14:17:37 GMT');

Response headers:

.. code-block:: none

Expires: Thu, 19 Nov 1981 08:52:00 GMT
// ...
Expires: Sun, 17 Nov 2024 14:17:37 GMT

Now, we don't know which one will be picked by the browser or which header is the correct one.
With changes in this version our previous header will be overridden:

.. code-block:: none

Cache-Control: no-store, no-cache, must-revalidate
Pragma: no-cache
Expires: Sun, 17 Nov 2024 14:17:37 GMT

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

Expand Down Expand Up @@ -298,6 +340,10 @@ Deprecations
Bugs Fixed
**********

- **Response:**
- Headers set using the ``Response`` class are now prioritized and replace headers
that can be set manually using the PHP ``header()`` function.

See the repo's
`CHANGELOG.md <https://github.com/codeigniter4/CodeIgniter4/blob/develop/CHANGELOG.md>`_
for a complete list of bugs fixed.
4 changes: 4 additions & 0 deletions user_guide_src/source/outgoing/response.rst
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ which can be either a string or an array of values that will be combined correct
Using these functions instead of using the native PHP functions allows you to ensure that no headers are sent
prematurely, causing errors, and makes testing possible.

.. important:: Since v4.6.0, if you set a header using PHP's native ``header()``
function and then use the ``Response`` class to set the same header, the
previous one will be overwritten.

.. note:: This method just sets headers to the response instance. So, if you create
and return another response instance (e.g., if you call :php:func:`redirect()`),
the headers set here will not be sent automatically.
Expand Down
Loading