bug #53733 [HttpFoundation] Prevent duplicated headers when using Ear…

…ly Hints (dunglas)

This PR was squashed before being merged into the 6.4 branch.

Discussion
----------

[HttpFoundation] Prevent duplicated headers when using Early Hints

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Issues        | n/a
| License       | MIT

Currently, headers are being duplicated in the final response if a 103 response has previously been sent. This patch fixes the issue.

Before:

```
< HTTP/2 103
< alt-svc: h3=":443"; ma=2592000
< cache-control: no-cache, private
< date: Fri, 02 Feb 2024 15:21:00 GMT
< link: </style.css>; rel=preload; as=style
< server: Caddy
< x-powered-by: PHP/8.3.1
< HTTP/2 200
< cache-control: no-cache, private
< cache-control: no-cache, private
< content-type: text/html; charset=UTF-8
< date: Fri, 02 Feb 2024 15:21:00 GMT
< date: Fri, 02 Feb 2024 15:21:00 GMT
< link: </style.css>; rel=preload; as=style
< link: </style.css>; rel=preload; as=style
```

Commits
-------

ca5db79 [HttpFoundation] Prevent duplicated headers when using Early Hints

.github/workflows/integration-tests.yml

@@ -110,6 +110,16 @@ jobs:
           KAFKA_ADVERTISED_HOST_NAME: 127.0.0.1
           KAFKA_ZOOKEEPER_CONNECT: 'zookeeper:2181'
           KAFKA_ADVERTISED_PORT: 9092
+      frankenphp:
+        image: dunglas/frankenphp:1.1.0
+        ports:
+          - 80:80
+        volumes:
+          - ${{ github.workspace }}:/symfony
+        env:
+          SERVER_NAME: 'http://localhost'
+          CADDY_SERVER_EXTRA_DIRECTIVES: |
+            root * /symfony/src/Symfony/Component/HttpFoundation/Tests/Fixtures/response-functional/
 
     steps:
       - name: Checkout

src/Symfony/Component/HttpFoundation/Response.php

@@ -355,23 +355,21 @@ public function sendHeaders(/* int $statusCode = null */): static
             $replace = false;
 
             // As recommended by RFC 8297, PHP automatically copies headers from previous 103 responses, we need to deal with that if headers changed
-            if (103 === $statusCode) {
-                $previousValues = $this->sentHeaders[$name] ?? null;
-                if ($previousValues === $values) {
-                    // Header already sent in a previous response, it will be automatically copied in this response by PHP
-                    continue;
-                }
+            $previousValues = $this->sentHeaders[$name] ?? null;
+            if ($previousValues === $values) {
+                // Header already sent in a previous response, it will be automatically copied in this response by PHP
+                continue;
+            }
 
-                $replace = 0 === strcasecmp($name, 'Content-Type');
+            $replace = 0 === strcasecmp($name, 'Content-Type');
 
-                if (null !== $previousValues && array_diff($previousValues, $values)) {
-                    header_remove($name);
-                    $previousValues = null;
-                }
-
-                $newValues = null === $previousValues ? $values : array_diff($values, $previousValues);
+            if (null !== $previousValues && array_diff($previousValues, $values)) {
+                header_remove($name);
+                $previousValues = null;
             }
 
+            $newValues = null === $previousValues ? $values : array_diff($values, $previousValues);
+
             foreach ($newValues as $value) {
                 header($name.': '.$value, $replace, $this->statusCode);
             }

src/Symfony/Component/HttpFoundation/Tests/Fixtures/response-functional/early_hints.php

@@ -0,0 +1,31 @@
+<?php
+
+// Requires FrankenPHP
+
+use Symfony\Component\HttpFoundation\Response;
+
+$parent = __DIR__;
+while (!@file_exists($parent.'/vendor/autoload.php')) {
+    if (!@file_exists($parent)) {
+        // open_basedir restriction in effect
+        break;
+    }
+    if ($parent === dirname($parent)) {
+        echo "vendor/autoload.php not found\n";
+        exit(1);
+    }
+
+    $parent = dirname($parent);
+}
+
+require $parent.'/vendor/autoload.php';
+
+$r = new Response();
+$r->headers->set('Link', '</css/style.css>; rel="preload"; as="style"');
+$r->sendHeaders(103);
+
+$r->headers->set('Link', '</js/app.js>; rel="preload"; as="script"', false);
+$r->sendHeaders(103);
+
+$r->setContent('Hello, Early Hints');
+$r->send();

src/Symfony/Component/HttpFoundation/Tests/ResponseFunctionalTest.php

@@ -12,6 +12,8 @@
 namespace Symfony\Component\HttpFoundation\Tests;
 
 use PHPUnit\Framework\TestCase;
+use Symfony\Component\Process\ExecutableFinder;
+use Symfony\Component\Process\Process;
 
 class ResponseFunctionalTest extends TestCase
 {
@@ -51,7 +53,31 @@ public function testCookie($fixture)
     public static function provideCookie()
     {
         foreach (glob(__DIR__.'/Fixtures/response-functional/*.php') as $file) {
-            yield [pathinfo($file, \PATHINFO_FILENAME)];
+            if (str_contains($file, 'cookie')) {
+                yield [pathinfo($file, \PATHINFO_FILENAME)];
+            }
         }
     }
+
+    /**
+     * @group integration
+     */
+    public function testInformationalResponse()
+    {
+        if (!(new ExecutableFinder())->find('curl')) {
+            $this->markTestSkipped('curl is not installed');
+        }
+
+        if (!($fp = @fsockopen('localhost', 80, $errorCode, $errorMessage, 2))) {
+            $this->markTestSkipped('FrankenPHP is not running');
+        }
+        fclose($fp);
+
+        $p = new Process(['curl', '-v', 'http://localhost/early_hints.php']);
+        $p->run();
+        $output = $p->getErrorOutput();
+
+        $this->assertSame(3, preg_match_all('#Link: </css/style\.css>; rel="preload"; as="style"#', $output));
+        $this->assertSame(2, preg_match_all('#Link: </js/app\.js>; rel="preload"; as="script"#', $output));
+    }
 }