fix: prioritize headers set by the Response class (#9235)

* fix: prioritize headers set by the Response class

* cs fix

* add a note to the user guide

* docs: add an example to the header changes

* Update user_guide_src/source/changelogs/v4.6.0.rst

Co-authored-by: John Paul E. Balandan, CPA <[email protected]>

---------

Co-authored-by: John Paul E. Balandan, CPA <[email protected]>

Author: michalsn

system/HTTP/ResponseTrait.php

@@ -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;
                 }
             }
         }

tests/system/HTTP/ResponseSendTest.php

@@ -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');
+    }
 }

user_guide_src/source/changelogs/v4.6.0.rst

@@ -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.
+
+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
 =================
 
@@ -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.

user_guide_src/source/outgoing/response.rst

@@ -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.