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

dunglas · 2022-11-06T20:52:36Z

Q	A
Branch?	6.3
Bug fix?	no
New feature?	yes
Deprecations?	yes
Tickets	n/a
License	MIT
Doc PR	todo

This patch adds support for sending informational responses, including Early Hints responses if supported by the SAPI. It also allows sending other informational status codes such as 102 Processing.

According to Shopify and Cloudflare, using Early Hints, the performance improvement to the Largest Contentful Paint can go from several hundred milliseconds, and up to a second faster.

Usage:

<?php

namespace App\Controller;

use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Routing\Annotation\Route;
use Symfony\Component\WebLink\Link;

class HomepageController extends AbstractController
{
    #[Route("/", name: "homepage")]
    public function index(): Response
    {
        $response = $this->sendEarlyHints([
            (new Link(href: '/style.css'))->withAttribute('as', 'stylesheet'),
            (new Link(href: '/script.js'))->withAttribute('as', 'script'),
        ]);

        // Do something slow...

        return $this->render('homepage/index.html.twig', response: $response);
    }
}

With this patch, HttpFoundation will leverage the headers_send() function provided by FrankenPHP. FrankenPHP is currently the only SAPI supporting Early Hints, but other SAPI such as mod_apache will probably implement this function at some point: php/php-src#7025 (comment)

The low-level API is similar to the one provided by Go: golang/go#42597
The high-level API helper in AbstractController is similar to Node's one: nodejs/node#44180

carsonbot · 2022-11-07T16:56:51Z

Hey!

I think @pascalwoerde has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

chalasr · 2022-11-08T12:20:35Z

As many Symfony devs don't know about the sendHeaders() method and concept, I think it would make sense to expose a more high-level API for this, e.g Response::sendInformationalResponse(STATUS) or even Response::sendEarlyHints(Link[] $links) or similar. WDYT?

~~Also, is it relevant to send preload link headers with both the informational and main response?~~ (Yes, it is)

derrabus · 2022-11-08T12:39:45Z

Is that deprecation necessary? Calling sendHeaders() without any arguments should remain a valid call imho.

dunglas · 2022-11-12T14:20:09Z

@chalasr What would be the benefit of doing that? My proposal is very similar to how it's implemented in Go for instance. IMHO the current API is good enough and there is no need to make it more complex.

@derrabus the plan is to force passing null in Symfony 7 to be able to make this attribute optional again in Symfony 8. Is there another way to add an optional parameter without introducing a breaking change?

derrabus · 2022-11-13T11:08:18Z

@derrabus the plan is to force passing null in Symfony 7 to be able to make this attribute optional again in Symfony 8.

And why don't we make it optional right from the start?

chalasr · 2022-11-13T16:00:05Z

@chalasr What would be the benefit of doing that? My proposal is very similar to how it's implemented in Go for instance. IMHO the current API is good enough and there is no need to make it more complex.

It's not obvious that sendHeaders() is about sending a response, hence I think the proposed API is more complex. I think we'd better have something crystal clear than being consistent with Golang's low-level api IMHO

dunglas · 2022-11-13T16:52:48Z

Informational responses are special: they can only be composed of headers, by the spec:

A 1xx response is terminated by the end of the header section; it cannot contain content or trailers.

https://httpwg.org/specs/rfc9110.html#status.1xx

So basically, sending 1XX responses mean sending headers ahead of time. Adding a new method would just add more confusion IMHO.

chalasr · 2022-11-13T17:12:21Z

So basically basically, sending 1XX responses mean sending headers ahead of time

Yes, that's something I personally know but:

use cases for informational responses other than 103 are rare in the web area, I consider the concept to be quite new for Symfony at least.
nobody calls sendHeaders() currently
exposing this new use case via a status parameter in sendHeaders() does not feel future-proof. A dedicated API (maybe even experimental as this is not widely supported yet?) would provide us with more flexibility.

But maybe it's just me 🤷 hope others opinions will come.

Is there another way to add an optional parameter without introducing a breaking change?

A virtual parameter (@param) should be enough: no added noise for callers, only a deprecation notice for eventual children (triggered by DebugClassLoader)

dunglas · 2022-11-14T10:59:56Z

@chalasr I hope that I've found an acceptable compromise:

the low-level API (in HttpFoundation) stays as-is, and still allows to do advancing things such as sending WebDav's 102 Processing status code
I added a higher-level, object-oriented helper in AbstractController dedicated to Early Hints:

<?php

namespace App\Controller;

use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Routing\Annotation\Route;
use Symfony\Component\WebLink\Link;

class HomepageController extends AbstractController
{
    #[Route("/", name: "homepage")]
    public function index(Request $request): Response
    {
        $response = $this->sendEarlyHints([
            (new Link(href: '/style.css'))->withAttribute('as', 'stylesheet'),
            (new Link(href: '/script.js'))->withAttribute('as', 'script'),
        ]);

        // Do something slow...

        return $this->render('homepage/index.html.twig', response: $response);
    }
}

So we have the best of both worlds. WDYT?

chalasr · 2022-11-14T11:46:26Z

Looks great! Thanks

src/Symfony/Component/HttpFoundation/StreamedResponse.php

src/Symfony/Component/HttpFoundation/Response.php

UPGRADE-6.3.md

src/Symfony/Component/HttpFoundation/CHANGELOG.md

src/Symfony/Component/HttpFoundation/Response.php

Co-authored-by: Christophe Coevoet <stof@notk.org>

Co-authored-by: Robin Chalas <chalasr@users.noreply.github.com>

chalasr

Although AbstractController is not first-class citizen anymore, it's good for discovery and we could consider moving this method to the component later if needed.

src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php

derrabus · 2022-12-09T12:56:57Z

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

This sends the headers immediately to the client, right? How can I test a controller that makes use of that feature?

dunglas · 2022-12-09T17:34:33Z

Indeed. You can test using FrakenPHP and curl --no-buffer.

chalasr · 2022-12-09T17:46:04Z

It would be nice to make sure that functional testing utilities are ready for this, and to patch them otherwise. Maybe even setup FrankenPHP in one of our GHA workflows?

…er.php Co-authored-by: Alexander M. Turek <me@derrabus.de>

nicolas-grekas · 2022-12-16T14:05:21Z

src/Symfony/Component/HttpFoundation/StreamedResponse.php

     * @return $this
     */
-    public function sendHeaders(): static
+    public function sendHeaders(/* ?int $statusCode = null */): static
    {
        if ($this->headersSent) {
            return $this;
        }

        $this->headersSent = true;


won't this prevent 103 from working with streamed responses?

nicolas-grekas · 2022-12-16T14:05:21Z

src/Symfony/Component/HttpFoundation/Response.php

+    /**
+     * Tracks headers already sent in informational responses
+     */
+    private array $sentHeaders;


would exposing a getter improve testability of this?

nicolas-grekas · 2022-12-16T14:05:21Z

src/Symfony/Component/HttpFoundation/Response.php

        // headers
        foreach ($this->headers->allPreserveCaseWithoutCookies() as $name => $values) {
+            $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


it will be automatically copied in this response by PHP

which part does that? frakenphp?
what if we send the headers again?
it would make sense to me, to keep things simple on our side. Up to the SAPI to do more if it wants to.
at least for 103 we must repeat the same headers in the >= 200 response (and I know no other 1xx that expect headers.)

Yes FrankenPHP does that automatically (it delegates to Go which does it automatically too). I don't think it's necessary to introduce more complexity yet because FrankenPHP is currently the only SAPI supporting this feature, and if new SAPIs add it they will likely work the same way (if that's not the case, we'll still be able to patch at that time).

As I see it, this behavior from FrankenPHP is introducing complexity on our side. I'd better have it not keep early headers in memory and rely on the app to resend its headers in the real response.
I feel like the current behavior is also not future-proof and is coupled to 103. But what if another 1xx status code decides to not require repeating some headers?

Go and FrankenPHP do it only for 103, so it's future proof. We should probably do it only for 103 here too.

It definitely introduces some complexity on Symfony-side, but it makes it more easy for all users using PHP directly. Also, it's a bit faster.

We should probably do it only for 103 here too

Definitely! I would prefer removing this special case for 103. If it's still doable, please consider my modest request :)
The complexity this requires on our side says why, no need for more arguments.

nicolas-grekas · 2022-12-16T14:05:21Z

src/Symfony/Component/HttpFoundation/Response.php

+                $previousValues = null;
+            }
+
+            $newValues = null === $previousValues ? $values : array_diff($values, $previousValues);


see my previous comment: why not keep things simple and sent all headers again?

With FrankenPHP is you don't do that headers will be duplicated.

nicolas-grekas · 2022-12-16T14:05:21Z

src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php

+    }
+
+    /**
+     * @internal


can be removed, the method is private

carsonbot added Status: Needs Review Deprecation Feature HttpFoundation labels Nov 6, 2022

carsonbot added this to the 6.2 milestone Nov 6, 2022

This comment was marked as outdated.

Sign in to view

dunglas force-pushed the feat/103-status-code branch 2 times, most recently from aa3684a to 85bb5ca Compare Nov 6, 2022

welcoMattic modified the milestones: 6.2, 6.3 Nov 7, 2022

dunglas added 2 commits Nov 14, 2022

[HttpFoundation] Add support for the 103 status code (Early Hints)

a287042

feat: integration with FrameworkBundle

dd18dc7

dunglas force-pushed the feat/103-status-code branch from 85bb5ca to dd18dc7 Compare Nov 14, 2022

cs

f73669e

chalasr reviewed Nov 14, 2022

View changes

stof reviewed Nov 14, 2022

View changes

dunglas and others added 3 commits Nov 14, 2022

Update src/Symfony/Component/HttpFoundation/CHANGELOG.md

8a20b16

Co-authored-by: Christophe Coevoet <stof@notk.org>

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

2eca954

Co-authored-by: Robin Chalas <chalasr@users.noreply.github.com>

review

2b3026f

dunglas force-pushed the feat/103-status-code branch from 4d5e71d to 2b3026f Compare Nov 17, 2022

dunglas added 4 commits Nov 17, 2022

handle headers modification

6d511f5

cleanup

c3980dd

remove wrong phpdoc

f75c725

more cleanup

59c397f

chalasr approved these changes Dec 3, 2022

View changes

src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php Show resolved Hide resolved

carsonbot added Status: Reviewed and removed Status: Needs Review labels Dec 3, 2022

derrabus reviewed Dec 9, 2022

View changes

src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php Outdated Show resolved Hide resolved

src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php Outdated Show resolved Hide resolved

dunglas and others added 2 commits Dec 9, 2022

Update src/Symfony/Bundle/FrameworkBundle/Controller/AbstractControll…

4dd78a1

…er.php Co-authored-by: Alexander M. Turek <me@derrabus.de>

Update src/Symfony/Bundle/FrameworkBundle/Controller/AbstractControll…

701f2c1

…er.php Co-authored-by: Alexander M. Turek <me@derrabus.de>

nicolas-grekas reviewed Dec 16, 2022

View changes

nicolas-grekas added Status: Needs Work and removed Status: Reviewed labels Dec 22, 2022

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

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

dunglas commented Nov 6, 2022 •

edited

This comment was marked as outdated.

carsonbot commented Nov 7, 2022

chalasr commented Nov 8, 2022 •

edited

derrabus commented Nov 8, 2022

dunglas commented Nov 12, 2022

derrabus commented Nov 13, 2022

chalasr commented Nov 13, 2022

dunglas commented Nov 13, 2022 •

edited

chalasr commented Nov 13, 2022 •

edited

dunglas commented Nov 14, 2022

chalasr commented Nov 14, 2022

chalasr left a comment

derrabus commented Dec 9, 2022

dunglas commented Dec 9, 2022

chalasr commented Dec 9, 2022

nicolas-grekas Dec 16, 2022

nicolas-grekas Dec 16, 2022

nicolas-grekas Dec 16, 2022

dunglas Dec 16, 2022

nicolas-grekas Dec 18, 2022 •

edited

dunglas Dec 18, 2022

nicolas-grekas Dec 19, 2022

nicolas-grekas Dec 16, 2022

dunglas Dec 16, 2022

nicolas-grekas Dec 16, 2022

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

Are you sure you want to change the base?

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

Conversation

dunglas commented Nov 6, 2022 • edited

This comment was marked as outdated.

carsonbot commented Nov 7, 2022

chalasr commented Nov 8, 2022 • edited

derrabus commented Nov 8, 2022

dunglas commented Nov 12, 2022

derrabus commented Nov 13, 2022

chalasr commented Nov 13, 2022

dunglas commented Nov 13, 2022 • edited

chalasr commented Nov 13, 2022 • edited

dunglas commented Nov 14, 2022

chalasr commented Nov 14, 2022

chalasr left a comment

derrabus commented Dec 9, 2022

dunglas commented Dec 9, 2022

chalasr commented Dec 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolas-grekas Dec 18, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dunglas commented Nov 6, 2022 •

edited

chalasr commented Nov 8, 2022 •

edited

dunglas commented Nov 13, 2022 •

edited

chalasr commented Nov 13, 2022 •

edited

nicolas-grekas Dec 18, 2022 •

edited