Skip to content

Commit fa843ce

Browse files
nicolas-grekasnicolas-grekas
committed
bug #53733 [HttpFoundation] Prevent duplicated headers when using Early 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
2 parents f53f279 + ca5db79 commit fa843ce

File tree

4 files changed

+79
-14
lines changed

4 files changed

+79
-14
lines changed

‎.github/workflows/integration-tests.yml

+10
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,16 @@ jobs:
110110
KAFKA_ADVERTISED_HOST_NAME: 127.0.0.1
111111
KAFKA_ZOOKEEPER_CONNECT: 'zookeeper:2181'
112112
KAFKA_ADVERTISED_PORT: 9092
113+
frankenphp:
114+
image: dunglas/frankenphp:1.1.0
115+
ports:
116+
- 80:80
117+
volumes:
118+
- ${{ github.workspace }}:/symfony
119+
env:
120+
SERVER_NAME: 'http://localhost'
121+
CADDY_SERVER_EXTRA_DIRECTIVES: |
122+
root * /symfony/src/Symfony/Component/HttpFoundation/Tests/Fixtures/response-functional/
113123
114124
steps:
115125
- name: Checkout

‎src/Symfony/Component/HttpFoundation/Response.php

+11-13
Original file line numberDiff line numberDiff line change
@@ -355,23 +355,21 @@ public function sendHeaders(/* int $statusCode = null */): static
355355
$replace = false;
356356

357357
// As recommended by RFC 8297, PHP automatically copies headers from previous 103 responses, we need to deal with that if headers changed
358-
if (103 === $statusCode) {
359-
$previousValues = $this->sentHeaders[$name] ?? null;
360-
if ($previousValues === $values) {
361-
// Header already sent in a previous response, it will be automatically copied in this response by PHP
362-
continue;
363-
}
358+
$previousValues = $this->sentHeaders[$name] ?? null;
359+
if ($previousValues === $values) {
360+
// Header already sent in a previous response, it will be automatically copied in this response by PHP
361+
continue;
362+
}
364363

365-
$replace = 0 === strcasecmp($name, 'Content-Type');
364+
$replace = 0 === strcasecmp($name, 'Content-Type');
366365

367-
if (null !== $previousValues && array_diff($previousValues, $values)) {
368-
header_remove($name);
369-
$previousValues = null;
370-
}
371-
372-
$newValues = null === $previousValues ? $values : array_diff($values, $previousValues);
366+
if (null !== $previousValues && array_diff($previousValues, $values)) {
367+
header_remove($name);
368+
$previousValues = null;
373369
}
374370

371+
$newValues = null === $previousValues ? $values : array_diff($values, $previousValues);
372+
375373
foreach ($newValues as $value) {
376374
header($name.': '.$value, $replace, $this->statusCode);
377375
}

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

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
3+
// Requires FrankenPHP
4+
5+
use Symfony\Component\HttpFoundation\Response;
6+
7+
$parent = __DIR__;
8+
while (!@file_exists($parent.'/vendor/autoload.php')) {
9+
if (!@file_exists($parent)) {
10+
// open_basedir restriction in effect
11+
break;
12+
}
13+
if ($parent === dirname($parent)) {
14+
echo "vendor/autoload.php not found\n";
15+
exit(1);
16+
}
17+
18+
$parent = dirname($parent);
19+
}
20+
21+
require $parent.'/vendor/autoload.php';
22+
23+
$r = new Response();
24+
$r->headers->set('Link', '</css/style.css>; rel="preload"; as="style"');
25+
$r->sendHeaders(103);
26+
27+
$r->headers->set('Link', '</js/app.js>; rel="preload"; as="script"', false);
28+
$r->sendHeaders(103);
29+
30+
$r->setContent('Hello, Early Hints');
31+
$r->send();

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

+27-1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
namespace Symfony\Component\HttpFoundation\Tests;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\Process\ExecutableFinder;
16+
use Symfony\Component\Process\Process;
1517

1618
class ResponseFunctionalTest extends TestCase
1719
{
@@ -51,7 +53,31 @@ public function testCookie($fixture)
5153
public static function provideCookie()
5254
{
5355
foreach (glob(__DIR__.'/Fixtures/response-functional/*.php') as $file) {
54-
yield [pathinfo($file, \PATHINFO_FILENAME)];
56+
if (str_contains($file, 'cookie')) {
57+
yield [pathinfo($file, \PATHINFO_FILENAME)];
58+
}
5559
}
5660
}
61+
62+
/**
63+
* @group integration
64+
*/
65+
public function testInformationalResponse()
66+
{
67+
if (!(new ExecutableFinder())->find('curl')) {
68+
$this->markTestSkipped('curl is not installed');
69+
}
70+
71+
if (!($fp = @fsockopen('localhost', 80, $errorCode, $errorMessage, 2))) {
72+
$this->markTestSkipped('FrankenPHP is not running');
73+
}
74+
fclose($fp);
75+
76+
$p = new Process(['curl', '-v', 'http://localhost/early_hints.php']);
77+
$p->run();
78+
$output = $p->getErrorOutput();
79+
80+
$this->assertSame(3, preg_match_all('#Link: </css/style\.css>; rel="preload"; as="style"#', $output));
81+
$this->assertSame(2, preg_match_all('#Link: </js/app\.js>; rel="preload"; as="script"#', $output));
82+
}
5783
}

0 commit comments

Comments
 (0)