symfony / symfony Public

This reverts commit 0558ea3.

* 4.4:
  Fix colors for 4.4
  Stand with Ukraine
  [FrameworkBundle] Ensure container is reset between tests
  Remove blocking test for adding support for placeholders in EmumNode in 6.1

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

Discussion
----------

Stand with Ukraine

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | n/a <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT
| Doc PR        | n/a

On 4.4
![image](https://user-images.githubusercontent.com/47313/157268381-2cf9466b-a7b8-4879-af6a-e60833e32d12.png)

On 5.4
![image](https://user-images.githubusercontent.com/47313/157268499-6dbd7be8-1584-4a50-8717-872554739136.png)

Commits
-------

0558ea3 Fix colors for 4.4
37ca066 Stand with Ukraine

…Kocal)

This PR was merged into the 5.4 branch.

Discussion
----------

[Runtime] Fix passing $debug parameter to `ErrorHandler`

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | yes/no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

As discussed with `@nicolas`-grekas on Slack, we think we should pass `$debug` parameter instead of `true`, since the 2nd argument of [`ErrorHandler`](https://github.com/symfony/symfony/blob/5.4/src/Symfony/Component/ErrorHandler/ErrorHandler.php#L184) is called `$debug`.

Commits
-------

a39f4f3 [Runtime] Fix passing $debug parameter to `ErrorHandler`

…inds: ...)] failing (LANGERGabrielle)

This PR was merged into the 5.4 branch.

Discussion
----------

[FrameworkBundle] Fix container:lint and #[Autoconfigure(binds: ...)] failing

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        |

# Description

Running `container:lint` when a service uses `#[Autoconfigure(binds: ['$myDependency' => {...})]` raises an error :

```
[ERROR] A binding is configured for an argument named "$myDependency" under
         "_instanceof" in file  "/{...}/MyService.php", but no
         corresponding argument has been found. It may be unused and should be
         removed, or it may have a typo.
```

# How to reproduce

Given a service
```php
#[Autoconfigure(bind: ['$myDependency' => '`@app`.my_dependency'])]
class MyService
{
    public function __construct(MyDependency $myDependency)
    {
    }
}
```

``@app`.my_dependency` is correctly injected into `MyService`. But when running `container:lint` **with the container dumped** (`App_KernelDevDebugContainer.xml` is present and up to date), it raises an error.

This only happens with `#[Autoconfigure]`. A similar configuration but using `services.yaml` works as expected :
```yaml
__instanceof:
    App\MyService:
        binds:
            $myDependency: '`@app`.my_dependency'
```

# Explanation of the issue

1. [`RegisterAutoconfigureAttributesPass` parses the `#[Autoconfigure]` and registers its binding](https://github.com/symfony/symfony/blob/08fa74a16c84895575e305b2a7ee3a03e371f79b/src/Symfony/Component/DependencyInjection/Compiler/RegisterAutoconfigureAttributesPass.php#L71).
2. [`ResolveBindingsPass` reads the binding and uses it to configure the service `arguments`](https://github.com/symfony/symfony/blob/1255cfffc260500adbfb3268b8f6c644e8bc43dd/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php#L223).
3. The container is dumped, with `App_KernelDevDebugContainer.xml` containing
    ```xml
    <service id="App\MyService" class="App\MyService" autowire="true" autoconfigure="true">
      <argument type="service" id="app.my_dependency"/>
    </service>
    ```
4. [`ContainerLintCommand` then creates a container from the dump](https://github.com/symfony/symfony/blob/08fa74a16c84895575e305b2a7ee3a03e371f79b/src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php#L104).
5. [The container is compiled](https://github.com/symfony/symfony/blob/08fa74a16c84895575e305b2a7ee3a03e371f79b/src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php#L65). Since `Kernel::initializeBundles()` is not called, only the [base passes](https://github.com/symfony/symfony/blob/60ce5a3dfbd90fad60cd39fcb3d7bf7888a48659/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php#L42) are used.
6. [`RegisterAutoconfigureAttributesPass` parses the `#[Autoconfigure]` and registers its binding](https://github.com/symfony/symfony/blob/08fa74a16c84895575e305b2a7ee3a03e371f79b/src/Symfony/Component/DependencyInjection/Compiler/RegisterAutoconfigureAttributesPass.php#L71), **again**.
7. [`ResolveBindingsPass` sees that the service already has an `argument` (from the xml dump from the first `ResolveBindingsPass`)](https://github.com/symfony/symfony/blob/1255cfffc260500adbfb3268b8f6c644e8bc43dd/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php#L181). This mean that the binding is **not** used this time, [and it is never removed from `$unusedBindings`](https://github.com/symfony/symfony/blob/1255cfffc260500adbfb3268b8f6c644e8bc43dd/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php#L126).
8. [The error is then thrown](https://github.com/symfony/symfony/blob/1255cfffc260500adbfb3268b8f6c644e8bc43dd/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php#L76).

# Explanation of the fix
This fix removes the passes that already processed the dumped container.
A more future proof fix would be 209516b, but it would require changes to `DependencyInjection`.

Both fixes have been tested on a mid size Symfony application.

Commits
-------

309998b [FrameworkBundle] Fix compiler passes processing a container twice when it's loaded from the debug dump

… in EmumNode in 6.1 (ecourtial)

This PR was merged into the 4.4 branch.

Discussion
----------

Remove blocking test for adding support for placeholders in EmumNode in 6.1

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

Remove blocking test for the PR #45624

Commits
-------

a0bce33 Remove blocking test for adding support for placeholders in EmumNode in 6.1

…(nicolas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[FrameworkBundle] Ensure container is reset between tests

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #45668
| License       | MIT
| Doc PR        | -

Calling boot() resets the container only is a request was handled before.
Since this is not always the case here, we need to reset the container explicitly.

Commits
-------

608b1dd [FrameworkBundle] Ensure container is reset between tests

…guments (kbond)

This PR was merged into the 5.4 branch.

Discussion
----------

[HttpKernel] fix using Target attribute with controller arguments

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #45021
| License       | MIT
| Doc PR        | n/a

This was supposed to fix #45021 but after adding what I suspected was a failing test, it works as expected. I guess I'm not testing the correct thing?

Here's a quick reproducer that shows the problem in a "real app":

```php
class HomepageController extends AbstractController
{
    public function __construct(#[Target('request.logger')] private LoggerInterface $logger)
    {
    }

    #[Route('/', name: 'app_homepage')]
    public function index(#[Target('request.logger')] LoggerInterface $logger): Response
    {
        dd(
            $this->logger->getName(), // "request" (expected)
            $logger->getName()        // "app" (not-expected)
        );

        // ...
    }
}
```

Commits
-------

448cc3a [HttpKernel] fix using Target with controller args

…onListener (nicolas-grekas)

This PR was merged into the 5.4 branch.

Discussion
----------

[HttpKernel] Fix advertizing deprecations for *TestSessionListener

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Looks like we missed that during the review of #41390

Commits
-------

3e209c3 [HttpKernel] Fix advertizing deprecations for *TestSessionListener

…in test (xabbuh)

This PR was merged into the 5.4 branch.

Discussion
----------

reflect Cache component version dependent default value in test

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        |

Commits
-------

c4d5b04 reflect Cache component version dependent default value in test

* 4.4:
  Bump Symfony version to 4.4.40
  Update VERSION for 4.4.39
  Update CONTRIBUTORS for 4.4.39
  Update CHANGELOG for 4.4.39

released v5.4.6

released v4.4.39

* 4.4:
  [HttpFoundation] Fix PHP 8.1 deprecation in isNotModified

…in 6.1

…en it's loaded from the debug dump

…lETaylor)

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

Discussion
----------

[redis-messenger] remove undefined array key warnings

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #45270  <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT

check that we actually have some information back from redis before assuming its an array with 2 keys to avoid undefined array key warnings as per #45270

----

so the code in question is this:

```php
public function get(): ?array
    {
        if ($this->autoSetup) {
            $this->setup();
        }
        $now = microtime();
        $now = substr($now, 11).substr($now, 2, 3);

        $queuedMessageCount = $this->rawCommand('ZCOUNT', 0, $now);

        while ($queuedMessageCount--) {
            if (![$queuedMessage, $expiry] = $this->rawCommand('ZPOPMIN', 1)) {
                break;
            }

            if (\strlen($expiry) === \strlen($now) ? $expiry > $now : \strlen($expiry) < \strlen($now)) {
                // if a future-placed message is popped because of a race condition with
                // another running consumer, the message is readded to the queue

                if (!$this->rawCommand('ZADD', 'NX', $expiry, $queuedMessage)) {
                    throw new TransportException('Could not add a message to the redis stream.');
                }

                break;
            }

            $decodedQueuedMessage = json_decode($queuedMessage, true);
            $this->add(\array_key_exists('body', $decodedQueuedMessage) ? $decodedQueuedMessage['body'] : $queuedMessage, $decodedQueuedMessage['headers'] ?? [], 0);
        }
```

I think what's happening, as I often can have up to 1000 workers (yup, for real, making 1000 http requests to remote sites - see mySites.guru) I think the logic might be flawed.

Because, the following line gets the number of messages using ZCOUNT

```php
$queuedMessageCount = $this->rawCommand('ZCOUNT', 0, $now);
```

It will then enter a loop counting down that number of messages checking them

```php
while ($queuedMessageCount--) {
```

At the same time, 999 other workers can be doing the same thing

it gets to a point where one of the workers doesnt get a message back from `ZPOPMIN` and thus we get undefined array keys...

You can see the result of ZPOPMIN is no longer an array once empty using the interactive test on the official redis page.

<img width="616" alt="Screen Shot 2022-03-02 at 20 21 00" src="https://user-images.githubusercontent.com/400092/156443762-564bc496-19dd-4e60-9b0d-94589d7dcd4d.png">

Commits
-------

758539a [redis-messenger] remove undefined array key warnings

This PR was merged into the 5.4 branch.

Discussion
----------

[Cache] do not pass DBAL connections to PDO adapters

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #45407
| License       | MIT
| Doc PR        |

Commits
-------

1027465 do not pass DBAL connections to PDO adapters

…otModified` (HypeMC)

This PR was merged into the 4.4 branch.

Discussion
----------

[HttpFoundation] Fix PHP 8.1 deprecation in `Response::isNotModified`

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

If an `If-None-Match` header is provided in the request and there's no `ETag` header in the response, the following deprecation notice occurs on PHP 8.1 in `Response::isNotModified`:

```
Deprecated: strncmp(): Passing null to parameter #1 ($string1) of type string is deprecated
```

Commits
-------

b909acf [HttpFoundation] Fix PHP 8.1 deprecation in isNotModified