[HttpKernel] Adding new `#[MapRequestHeader]` attribute and resolver #51379

StevenRenaux · 2023-08-14T10:04:24Z

Q	A
Branch?	6.4
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	#51342
License	MIT
Doc PR	#20541

Hi,

Related to this #51342 I tried to work on this PR.
To use this new attribute, you just need to add it in your method of your controller.

namespace App\Controller;

class TestingController extends AbstractController
{
    #[Route('/', name: 'home', methods: 'GET')]
    public function home(
        #[MapRequestHeader] array $accept,
        #[MapRequestHeader('accept')] string $foo,
        #[MapRequestHeader('user-agent')] ?string $bar,
        #[MapRequestHeader(name: 'accept-encoding')] AcceptHeader $encoding,
    )
    {
            dump($accept);
            //        array:7 [▼
            //          0 => "text/html"
            //          1 => "application/xhtml+xml"
            //          2 => "image/avif"
            //          3 => "image/webp"
            //          4 => "image/apng"
            //          5 => "application/xml"
            //          6 => "*/*"
            //        ]
            
            dump($foo);
            //  "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8"
            
            dump($bar);
            //  "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/131.0.0.0 Safari/537.36"
            
            dump($encoding);
            //        Symfony\Component\HttpFoundation\AcceptHeader {#4727 ▼
            //            -items: array:4 [▼
            //                "gzip" => 
            //                    Symfony\Component\HttpFoundation\AcceptHeaderItem {#2437 ▼
            //                        -value: "gzip"
            //                        -quality: 1.0
            //                        -index: 0
            //                        -attributes: []
            //                }
            //                "deflate" => 
            //                    Symfony\Component\HttpFoundation\AcceptHeaderItem {#4882 ▼
            //                        -value: "deflate"
            //                        -quality: 1.0
            //                        -index: 1
            //                        -attributes: []
            //                }
            //                "br" => 
            //                    Symfony\Component\HttpFoundation\AcceptHeaderItem {#4549 ▼
            //                        -value: "br"
            //                        -quality: 1.0
            //                        -index: 2
            //                        -attributes: []
            //                }
            //                "zstd" => 
            //                    Symfony\Component\HttpFoundation\AcceptHeaderItem {#4773 ▼
            //                        -value: "zstd"
            //                        -quality: 1.0
            //                        -index: 3
            //                        -attributes: []
            //                }
            //            ]
            //              -sorted: false
            //        }
    }

For some parameters as User-Agent we need to add the name for the resolver, because the conversion of userAgent is not interpreted by the resolver and return an exception as Missing header parameter "userAgent".

And for some others parameters as cookie I don't know what to do, because at the moment it only return string for all of them and is it really useful 🤷

carsonbot · 2023-08-14T10:04:29Z

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has a contribution guide which I suggest you to read.

In short:

Always add tests
Keep backward compatibility (see https://symfony.com/bc).
Bug fixes must be submitted against the lowest maintained branch where they apply (see https://symfony.com/releases)
Features and deprecations must be submitted against the 6.4 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

AurelienPillevesse · 2023-08-14T13:38:06Z

Maybe I'm wrong but seems that something related to mapQueryString and mapRequestPayload is in this file and I see nothing from yours :

symfony/src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php

Line 84 in 16a4681

    
           public function onKernelControllerArguments(ControllerArgumentsEvent $event): void

Why did not add in the same file?

src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestHeaderValueResolver.php

robert-sykes

Would this not be best to also allow it to map to an object, like how the MapRequestPayload currently works, as I can pass an Object as a reference the serializer will deserialize to this object? That way you can have a Header class that the headers deserialize into.

You can merge it with your current concept and have a bit of best of both? Map individual Headers to an string, or an object or even just an array if required?

Something potentially like this?

    private function mapRequestHeader(Request $request, string $type, MapRequestHeader $attribute): ?object
    {
        $headers = [];

        $requestHeaders = $request->headers->all();

        array_walk($requestHeaders, function ($value, $key) use (&$headers) {
            $key = $this->snakeCaseToCamelCase($key);
            $headers[$key] = $value[0];
        });

        $headers = json_encode($headers);

        $format = 'json';

        try {
            return $this->serializer->deserialize($headers, $type, $format, []);
        } catch (UnsupportedFormatException $e) {
            throw new HttpException(Response::HTTP_UNSUPPORTED_MEDIA_TYPE, sprintf('Unsupported format: "%s".', $format), $e);
        } catch (NotEncodableValueException $e) {
            throw new HttpException(Response::HTTP_BAD_REQUEST, sprintf('Request payload contains invalid "%s" data.', $format), $e);
        }
    }

and you could have a class like this:

class HeaderRequest
{
    public function __construct(
        public readonly string $userAgent,
        public readonly string $accept,
        public readonly string $contentType,
        public readonly string $acceptEncoding,
    ) {
    }
}

An advantage is you can use the Validator too and enforce any header types if needed or create a HeaderRequest class per request as needed.

You can add this as a new method to the existing RequestPayloadValueResolver as already mentioned by @AurelienPillevesse so it will tie nicely into the existing process.

This is a rough example but just food for thought.

Very useful PR this and something we need, so looking forward to seeing how it progresses :)

src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php

ruudk · 2023-08-21T17:10:04Z

Too bad the PR was changed after @robert-sykes suggestion. I think these are 2 distinct ideas. The original PR description mentions a single attribute per header, same as how MapQueryParameter works. There are valid cases for that, where you only want and need a single header scalar.

The updated code now requires you to map a header to a header object.

Would be great to also keep supporting the original one for just a single header.

See this comment where a similar situation happened: #49134 (comment)

robert-sykes · 2023-08-21T18:53:16Z

Too bad the PR was changed after @robert-sykes suggestion. I think these are 2 distinct ideas. The original PR description mentions a single attribute per header, same as how MapQueryParameter works. There are valid cases for that, where you only want and need a single header scalar.

The updated code now requires you to map a header to a header object.

Would be great to also keep supporting the original one for just a single header.

See this comment where a similar situation happened: #49134 (comment)

I'm all for this, to be fair. I did imply this in my original comment where I said You can merge it with your current concept and have a bit of best of both? Map individual Headers to an string, or an object or even just an array if required? That said - I didn't push it any further in my commentary, apologies.

@StevenRenaux You could check the type and if it's a string map to that similar to your original commit, otherwise if it's object map to those fields instead. We get best of both worlds this way. What do you think?

StevenRenaux · 2023-08-21T19:48:10Z

Hey, I will try to do it and I didn't understand well for sure but we are there to share and go further 😉

robert-sykes · 2023-08-21T21:33:36Z

Hey, I will try to do it and I didn't understand well for sure but we are there to share and go further 😉

No problem at all. If you need a hand let me know, more than happy to pair on this with you. :)

StevenRenaux · 2023-08-22T15:22:50Z

To get back to the original Issue I suggest this

namespace App\Controller;

class TestController extends AbstractController
{
    #[Route(path: '/', name: 'test')]
    public function test(
        #[MapRequestHeader] HeaderRequestDto $headerRequestDto,
        #[MapRequestHeader(name: 'user-agent')] string $agent,
        #[MapRequestHeader] array $accept
    ): Response
    {
        dd(
            $headerRequestDto,
//      App\Entity\HeaderRequestDto {
//        accept: "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8"
//        agent: "Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:109.0) Gecko/20100101 Firefox/116.0"
//        acceptEncoding: "gzip, deflate, br"
//        host: "localhost:8000"
//      }
           
            $agent,
//      "Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:109.0) Gecko/20100101 Firefox/116.0"

            $accept
//      array:6 [
//          0 => "text/html"
//          1 => "application/xhtml+xml"
//          2 => "application/xml;q=0.9"
//          3 => "image/avif"
//          4 => "image/webp"
//          5 => "*/*;q=0.8"
//      ]
        );
    }
}

And a Dto could look like this :

namespace App\Entity;

use Symfony\Component\Serializer\Annotation\SerializedName;
use Symfony\Component\Validator\Constraints as Assert;

class HeaderRequestDto
{
    public function __construct(
        public readonly string $accept,
        #[Assert\EqualTo('test', groups: ['test'])]
        #[SerializedName('user-agent')] public readonly string $agent = 'agent',
        #[SerializedName('accept-encoding')] public readonly string $acceptEncoding,
        #[Assert\EqualTo('localhost', groups: ['valid'])]
        public readonly string $host
    ) {
    }
}

We only use kebab-case (as returned by HeaderBag) now to get some property like 'user-agent'.
And if we want an array rather than a string for the property as accept it will return an array after an explode with a coma delimiter.

alexandre-daubois · 2024-08-20T08:40:21Z

Hi @StevenRenaux! What's the current state of this PR? 🙂

StevenRenaux · 2024-08-20T08:45:03Z

Hi @StevenRenaux! What's the current state of this PR? 🙂

@alexandre-daubois Waiting for review, since the last requested update.

src/Symfony/Component/HttpKernel/CHANGELOG.md

src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestHeaderValueResolver.php

StevenRenaux · 2025-01-07T13:27:17Z

Back to the basics of PR, using only the MapRequestHeader attribute. Mapping to a DTO does not seem like a relevant or common use case or if it is really necessary could be done in a future PR.

The allowed types are therefore:

string
array
AcceptHeader

When the header parameter cannot be determined by the variable name, we can specify the name argument in the attribute.

public function test(
    #[MapRequestHeader('user-agent')] ?string $bar,
): Response
{
  dump($bar);
  //  "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/131.0.0.0 Safari/537.36"
}

If we are looking for a header of type Accept-* with an array type, we will use the dedicated methods to retrieve this data.

https://symfony.com/doc/current/components/http_foundation.html#accessing-accept-headers-data

public function test(
    #[MapRequestHeader] array $accept,
): Response
{
  dump($accept);
  //        array:7 [▼
  //          0 => "text/html"
  //          1 => "application/xhtml+xml"
  //          2 => "image/avif"
  //          3 => "image/webp"
  //          4 => "image/apng"
  //          5 => "application/xml"
  //          6 => "*/*"
  //        ]
}

And with AcceptHeader type

public function test(
    #[MapRequestHeader(name: 'accept-encoding')] AcceptHeader $encoding,
): Response
{
  dump($encoding);
  //        Symfony\Component\HttpFoundation\AcceptHeader {#4727 ▼
  //            -items: array:4 [▼
  //                "gzip" =>
  //                    Symfony\Component\HttpFoundation\AcceptHeaderItem {#2437 ▼
  //                        -value: "gzip"
  //                        -quality: 1.0
  //                        -index: 0
  //                        -attributes: []
  //                }
  //                "deflate" =>
  //                    Symfony\Component\HttpFoundation\AcceptHeaderItem {#4882 ▼
  //                        -value: "deflate"
  //                        -quality: 1.0
  //                        -index: 1
  //                        -attributes: []
  //                }
  //                "br" =>
  //                    Symfony\Component\HttpFoundation\AcceptHeaderItem {#4549 ▼
  //                        -value: "br"
  //                        -quality: 1.0
  //                        -index: 2
  //                        -attributes: []
  //                }
  //                "zstd" =>
  //                    Symfony\Component\HttpFoundation\AcceptHeaderItem {#4773 ▼
  //                        -value: "zstd"
  //                        -quality: 1.0
  //                        -index: 3
  //                        -attributes: []
  //                }
  //            ]
  //              -sorted: false
  //        }
}

MatTheCat · 2025-01-10T09:52:02Z

It bugs me the array type has two semantics whether it applies to an Accept-* header or not. Plus

in the first case there already is the AcceptHeader type.
in the second case only the first value is returned in an array, but
- there can be many
- the first value can actually be many values combined by the SAPI

ruudk · 2025-05-26T11:25:54Z

@StevenRenaux What's needed to get this to a mergeable state? I would love this to be included in the new Symfony version.

StevenRenaux · 2025-05-26T11:36:39Z

@StevenRenaux What's needed to get this to a mergeable state? I would love this to be included in the new Symfony version.

@ruudk Waiting reviews from core-contributors, but it's seems to late for merging into the next 7.3 version.
Feel free also to review, approve...
I think at the moment it's fine to get a simple implementation as it is because it's the kind of stuff you can't statifies every problematics.

ruudk · 2025-05-26T11:37:52Z

Could you rebase it, so that the tests go green?

ruudk · 2025-05-26T11:49:55Z

@StevenRenaux Thanks! 🙏 It needs a few tweaks in https://github.com/symfony/symfony/actions/runs/15253101552/job/42894283925?pr=51379

mathroc · 2025-05-26T20:10:19Z

src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestHeaderValueResolver.php

+                    'accept-charset' => $request->getCharsets(),
+                    'accept-language' => $request->getLanguages(),
+                    'accept-encoding' => $request->getEncodings(),
+                    default => [$request->headers->get($name)],


Suggested change

default => [$request->headers->get($name)],

default => $request->headers->all($name),

mathroc · 2025-05-26T20:14:04Z

src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestHeaderValueResolver.php

+        }
+
+        if (null === $value && !$argument->isNullable()) {
+            throw new HttpException($attribute->validationFailedStatusCode, \sprintf('Missing header "%s".', $name));


I think if type is array we could return an empty array

carsonbot added Status: Needs Review Feature labels Aug 14, 2023

carsonbot added this to the 6.4 milestone Aug 14, 2023

StevenRenaux force-pushed the map-request-header-attribute branch from 527c461 to e85136d Compare August 14, 2023 11:06

WebMamba reviewed Aug 16, 2023

View reviewed changes

src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestHeaderValueResolver.php Show resolved Hide resolved

src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestHeaderValueResolver.php Outdated Show resolved Hide resolved

robert-sykes reviewed Aug 16, 2023

View reviewed changes

StevenRenaux force-pushed the map-request-header-attribute branch from 261f6e6 to 72cb1ee Compare August 17, 2023 14:32

robert-sykes suggested changes Aug 17, 2023

View reviewed changes

src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php Outdated Show resolved Hide resolved

carsonbot added Status: Needs Work and removed Status: Needs Review labels Aug 17, 2023

StevenRenaux force-pushed the map-request-header-attribute branch 2 times, most recently from afbdd35 to 90f1015 Compare August 17, 2023 14:50

robert-sykes reviewed Aug 17, 2023

View reviewed changes

src/Symfony/Component/HttpKernel/Controller/ArgumentResolver/RequestPayloadValueResolver.php Outdated Show resolved Hide resolved

robert-sykes approved these changes Aug 18, 2023

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Work labels Aug 18, 2023

StevenRenaux force-pushed the map-request-header-attribute branch from ff6b41f to 0bda0bf Compare August 22, 2023 15:02

StevenRenaux force-pushed the map-request-header-attribute branch from 0bda0bf to cced8c1 Compare August 22, 2023 15:30

nicolas-grekas added the HttpKernel label Aug 23, 2023

carsonbot changed the title ~~Adding a new Attribute MapRequestHeader class and resolver~~ [HttpKernel] Adding a new Attribute MapRequestHeader class and resolver Aug 23, 2023

StevenRenaux requested a review from robert-sykes September 7, 2023 09:49

carsonbot added Status: Needs Review and removed Status: Reviewed labels Sep 7, 2023

xabbuh added this to the 7.2 milestone May 15, 2024

robert-sykes approved these changes Aug 20, 2024

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Aug 20, 2024

alexandre-daubois reviewed Aug 20, 2024

View reviewed changes

src/Symfony/Component/HttpKernel/CHANGELOG.md Show resolved Hide resolved

mathroc reviewed Aug 20, 2024

View reviewed changes

fabpot modified the milestones: 7.2, 7.3 Nov 20, 2024

StevenRenaux changed the title ~~[HttpKernel] Adding new #[MapRequestHeader] and #[MapRequestHeaders] attributes and resolver~~ [HttpKernel] Adding new #[MapRequestHeader] attribute and resolver Jan 7, 2025

StevenRenaux force-pushed the map-request-header-attribute branch 3 times, most recently from 7494a85 to 0f38cde Compare January 7, 2025 13:11

StevenRenaux mentioned this pull request Jan 7, 2025

Add MapRequestHeader documentation symfony/symfony-docs#20541

Open

StevenRenaux force-pushed the map-request-header-attribute branch 2 times, most recently from fbde828 to 89f420f Compare January 10, 2025 08:26

StevenRenaux force-pushed the map-request-header-attribute branch from 89f420f to 2142045 Compare May 26, 2025 11:44

Adding a new Attribute MapRequestHeader class and resolver

7a123ea

StevenRenaux force-pushed the map-request-header-attribute branch from 2142045 to 7a123ea Compare May 26, 2025 15:12

fabpot modified the milestones: 7.3, 7.4 May 26, 2025

mathroc reviewed May 26, 2025

View reviewed changes

	default => [$request->headers->get($name)],
	default => $request->headers->all($name),

Uh oh!

[HttpKernel] Adding new #[MapRequestHeader] attribute and resolver #51379

Are you sure you want to change the base?

[HttpKernel] Adding new #[MapRequestHeader] attribute and resolver #51379

Conversation

StevenRenaux commented Aug 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

carsonbot commented Aug 14, 2023

Uh oh!

AurelienPillevesse commented Aug 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

robert-sykes left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ruudk commented Aug 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robert-sykes commented Aug 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

StevenRenaux commented Aug 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robert-sykes commented Aug 21, 2023

Uh oh!

StevenRenaux commented Aug 22, 2023 • edited by OskarStark Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexandre-daubois commented Aug 20, 2024

Uh oh!

StevenRenaux commented Aug 20, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

StevenRenaux commented Jan 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MatTheCat commented Jan 10, 2025

Uh oh!

ruudk commented May 26, 2025

Uh oh!

StevenRenaux commented May 26, 2025

Uh oh!

ruudk commented May 26, 2025

Uh oh!

ruudk commented May 26, 2025

Uh oh!

mathroc May 26, 2025

Choose a reason for hiding this comment

Uh oh!

mathroc May 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

[HttpKernel] Adding new `#[MapRequestHeader]` attribute and resolver #51379

[HttpKernel] Adding new `#[MapRequestHeader]` attribute and resolver #51379

StevenRenaux commented Aug 14, 2023 •

edited

Loading

AurelienPillevesse commented Aug 14, 2023 •

edited

Loading

robert-sykes left a comment •

edited

Loading

ruudk commented Aug 21, 2023 •

edited

Loading

robert-sykes commented Aug 21, 2023 •

edited

Loading

StevenRenaux commented Aug 21, 2023 •

edited

Loading

StevenRenaux commented Aug 22, 2023 •

edited by OskarStark

Loading

StevenRenaux commented Jan 7, 2025 •

edited

Loading