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

[HttpFoundation] Add support for the 103 status code (Early Hints) and other 1XX statuses #48128

Merged
merged 1 commit into from
Mar 13, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions UPGRADE-6.3.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ FrameworkBundle

* Deprecate the `notifier.logger_notification_listener` service, use the `notifier.notification_logger_listener` service instead

HttpFoundation
--------------

* `Response::sendHeaders()` now takes an optional `$statusCode` parameter

HttpKernel
----------

Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ CHANGELOG
* Allow setting `debug.container.dump` to `false` to disable dumping the container to XML
* Add `framework.http_cache.skip_response_headers` option
* Display warmers duration on debug verbosity for `cache:clear` command
* Add `AbstractController::sendEarlyHints()` to send HTTP Early Hints

6.2
---
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Bundle\FrameworkBundle\Controller;

use Psr\Container\ContainerInterface;
use Psr\Link\EvolvableLinkInterface;
use Psr\Link\LinkInterface;
use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
use Symfony\Component\DependencyInjection\ParameterBag\ContainerBagInterface;
Expand Down Expand Up @@ -42,6 +43,7 @@
use Symfony\Component\Serializer\SerializerInterface;
use Symfony\Component\WebLink\EventListener\AddLinkHeaderListener;
use Symfony\Component\WebLink\GenericLinkProvider;
use Symfony\Component\WebLink\HttpHeaderSerializer;
use Symfony\Contracts\Service\Attribute\Required;
use Symfony\Contracts\Service\ServiceSubscriberInterface;
use Twig\Environment;
Expand Down Expand Up @@ -92,6 +94,7 @@ public static function getSubscribedServices(): array
'security.token_storage' => '?'.TokenStorageInterface::class,
'security.csrf.token_manager' => '?'.CsrfTokenManagerInterface::class,
'parameter_bag' => '?'.ContainerBagInterface::class,
'web_link.http_header_serializer' => '?'.HttpHeaderSerializer::class,
];
}

Expand Down Expand Up @@ -402,4 +405,30 @@ protected function addLink(Request $request, LinkInterface $link): void

$request->attributes->set('_links', $linkProvider->withLink($link));
}

/**
* @param LinkInterface[] $links
*/
protected function sendEarlyHints(iterable $links, Response $response = null): Response
{
if (!$this->container->has('web_link.http_header_serializer')) {
throw new \LogicException('You cannot use the "sendEarlyHints" method if the WebLink component is not available. Try running "composer require symfony/web-link".');
}

$response ??= new Response();

$populatedLinks = [];
foreach ($links as $link) {
if ($link instanceof EvolvableLinkInterface && !$link->getRels()) {
$link = $link->withRel('preload');
}

$populatedLinks[] = $link;
}

$response->headers->set('Link', $this->container->get('web_link.http_header_serializer')->serialize($populatedLinks), false);
$response->sendHeaders(103);

return $response;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,18 @@
namespace Symfony\Component\DependencyInjection\Loader\Configurator;

use Symfony\Component\WebLink\EventListener\AddLinkHeaderListener;
use Symfony\Component\WebLink\HttpHeaderSerializer;

return static function (ContainerConfigurator $container) {
$container->services()

->set('web_link.http_header_serializer', HttpHeaderSerializer::class)
->alias(HttpHeaderSerializer::class, 'web_link.http_header_serializer')

->set('web_link.add_link_header_listener', AddLinkHeaderListener::class)
->args([
service('web_link.http_header_serializer'),
])
->tag('kernel.event_subscriber')
;
};
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
use Symfony\Component\Security\Core\User\InMemoryUser;
use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
use Symfony\Component\Serializer\SerializerInterface;
use Symfony\Component\WebLink\HttpHeaderSerializer;
use Symfony\Component\WebLink\Link;
use Twig\Environment;

Expand Down Expand Up @@ -72,6 +73,7 @@ public function testSubscribedServices()
'parameter_bag' => '?Symfony\\Component\\DependencyInjection\\ParameterBag\\ContainerBagInterface',
'security.token_storage' => '?Symfony\\Component\\Security\\Core\\Authentication\\Token\\Storage\\TokenStorageInterface',
'security.csrf.token_manager' => '?Symfony\\Component\\Security\\Csrf\\CsrfTokenManagerInterface',
'web_link.http_header_serializer' => '?Symfony\\Component\\WebLink\\HttpHeaderSerializer',
];

$this->assertEquals($expectedServices, $subscribed, 'Subscribed core services in AbstractController have changed');
Expand Down Expand Up @@ -677,4 +679,20 @@ public function testAddLink()
$this->assertContains($link1, $links);
$this->assertContains($link2, $links);
}

public function testSendEarlyHints()
{
$container = new Container();
$container->set('web_link.http_header_serializer', new HttpHeaderSerializer());

$controller = $this->createController();
$controller->setContainer($container);

$response = $controller->sendEarlyHints([
(new Link(href: '/style.css'))->withAttribute('as', 'stylesheet'),
(new Link(href: '/script.js'))->withAttribute('as', 'script'),
]);

$this->assertSame('</style.css>; rel="preload"; as="stylesheet",</script.js>; rel="preload"; as="script"', $response->headers->get('Link'));
}
}
2 changes: 1 addition & 1 deletion src/Symfony/Bundle/FrameworkBundle/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
"symfony/deprecation-contracts": "^2.5|^3",
"symfony/error-handler": "^6.1",
"symfony/event-dispatcher": "^5.4|^6.0",
"symfony/http-foundation": "^6.2",
"symfony/http-foundation": "^6.3",
"symfony/http-kernel": "^6.3",
"symfony/polyfill-mbstring": "~1.0",
"symfony/filesystem": "^5.4|^6.0",
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Component/HttpFoundation/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ CHANGELOG
* Add `ParameterBag::getEnum()`
* Create migration for session table when pdo handler is used
* Add support for Relay PHP extension for Redis
* The `Response::sendHeaders()` method now takes an optional HTTP status code as parameter, allowing to send informational responses such as Early Hints responses (103 status code)

6.2
---
Expand Down
54 changes: 50 additions & 4 deletions src/Symfony/Component/HttpFoundation/Response.php
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,11 @@ class Response
511 => 'Network Authentication Required', // RFC6585
];

/**
* Tracks headers already sent in informational responses.
*/
private array $sentHeaders;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would exposing a getter improve testability of this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest keeping that entirely internal. WDYT?


/**
* @param int $status The HTTP status code (200 "OK" by default)
*
Expand Down Expand Up @@ -326,30 +331,71 @@ public function prepare(Request $request): static
/**
* Sends HTTP headers.
*
* @param null|positive-int $statusCode The status code to use, override the statusCode property if set and not null
*
* @return $this
*/
public function sendHeaders(): static
public function sendHeaders(/* int $statusCode = null */): static
{
// headers have already been sent by the developer
if (headers_sent()) {
dunglas marked this conversation as resolved.
Show resolved Hide resolved
return $this;
}

$statusCode = \func_num_args() > 0 ? func_get_arg(0) : null;
$informationalResponse = $statusCode >= 100 && $statusCode < 200;
if ($informationalResponse && !\function_exists('headers_send')) {
// skip informational responses if not supported by the SAPI
return $this;
}

// headers
foreach ($this->headers->allPreserveCaseWithoutCookies() as $name => $values) {
$replace = 0 === strcasecmp($name, 'Content-Type');
foreach ($values as $value) {
$newValues = $values;
$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;
}

$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);
}

foreach ($newValues as $value) {
header($name.': '.$value, $replace, $this->statusCode);
}

if ($informationalResponse) {
$this->sentHeaders[$name] = $values;
}
}

// cookies
foreach ($this->headers->getCookies() as $cookie) {
header('Set-Cookie: '.$cookie, false, $this->statusCode);
}

if ($informationalResponse) {
headers_send($statusCode);

return $this;
}
Comment on lines 384 to +393
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally would not send the Cookies in the Informational Response.

As the session and so on could be changing and that handling already complex in the AbstractSessionListener to support all kind of Runtime (Swoole, Roadrunner, ... running in the CLI context) and so would not do any set-cookie in the early hint responses.

Suggested change
// cookies
foreach ($this->headers->getCookies() as $cookie) {
header('Set-Cookie: '.$cookie, false, $this->statusCode);
}
if ($informationalResponse) {
headers_send($statusCode);
return $this;
}
if ($informationalResponse) {
headers_send($statusCode);
return $this;
}
// cookies
foreach ($this->headers->getCookies() as $cookie) {
header('Set-Cookie: '.$cookie, false, $this->statusCode);
}


$statusCode ??= $this->statusCode;

// status
header(sprintf('HTTP/%s %s %s', $this->version, $this->statusCode, $this->statusText), true, $this->statusCode);
header(sprintf('HTTP/%s %s %s', $this->version, $statusCode, $this->statusText), true, $statusCode);

return $this;
}
Expand Down
11 changes: 8 additions & 3 deletions src/Symfony/Component/HttpFoundation/StreamedResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,17 +59,22 @@ public function setCallback(callable $callback): static
/**
* This method only sends the headers once.
*
* @param null|positive-int $statusCode The status code to use, override the statusCode property if set and not null
*
dunglas marked this conversation as resolved.
Show resolved Hide resolved
* @return $this
*/
public function sendHeaders(): static
public function sendHeaders(/* int $statusCode = null */): static
{
if ($this->headersSent) {
return $this;
}

$this->headersSent = true;
$statusCode = \func_num_args() > 0 ? func_get_arg(0) : null;
if ($statusCode < 100 || $statusCode >= 200) {
$this->headersSent = true;
}

return parent::sendHeaders();
return parent::sendHeaders($statusCode);
}

/**
Expand Down
11 changes: 11 additions & 0 deletions src/Symfony/Component/HttpFoundation/Tests/ResponseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,17 @@ public function testSendHeaders()
$this->assertSame($response, $headers);
}

public function testSendInformationalResponse()
{
$response = new Response();
$response->sendHeaders(103);

// Informational responses must not override the main status code
$this->assertSame(200, $response->getStatusCode());

$response->sendHeaders();
}

public function testSend()
{
$response = new Response();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,4 +124,15 @@ public function testSetNotModified()
$string = ob_get_clean();
$this->assertEmpty($string);
}

public function testSendInformationalResponse()
{
$response = new StreamedResponse();
$response->sendHeaders(103);

// Informational responses must not override the main status code
$this->assertSame(200, $response->getStatusCode());

$response->sendHeaders();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,9 @@ class_exists(HttpHeaderSerializer::class);
*/
class AddLinkHeaderListener implements EventSubscriberInterface
{
private HttpHeaderSerializer $serializer;

public function __construct()
{
$this->serializer = new HttpHeaderSerializer();
public function __construct(
private readonly HttpHeaderSerializer $serializer = new HttpHeaderSerializer(),
) {
}

public function onKernelResponse(ResponseEvent $event): void
Expand Down