Skip to content

Navigation Menu

Sign in
Appearance settings
Sign in
Sign up
Appearance settings

Commit 19a9f0c

Browse filesBrowse the repository at this point in the historyBrowse files
nicolas-grekasnicolas-grekas
committed
bug symfony#65637 [Console][FrameworkBundle] Fix profiling a command stopped at ConsoleEvents::COMMAND (Spomky)
This PR was merged into the 6.4 branch. Discussion ---------- [Console][FrameworkBundle] Fix profiling a command stopped at ConsoleEvents::COMMAND | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT A listener may stop a command at `ConsoleEvents::COMMAND`, by calling `$event->disableCommand()` or by throwing. The command then never runs, yet `ConsoleEvents::TERMINATE` is still dispatched and `ConsoleProfilerListener` still collects the profile. That profile is read off the `TraceableCommand`, which records `input`, `output`, `arguments` and `options` from inside `run()`, the method that was never called. All four are typed properties with no default, so `--profile` replaces the reason the command was stopped with: ``` Error: Typed property Symfony\Component\Console\Command\TraceableCommand::$input must not be accessed before initialization ``` thrown out of `CliRequest::getUri()`, then the same for `$arguments` out of `CommandDataCollector::collect()`. The command reports a failure that has nothing to do with why it was stopped, and no profile is saved for the very run one wanted to look at. `ConsoleProfilerListener` already records the exit code and the interrupting signal on the `TraceableCommand` before collecting. It now also records the input, the output, the arguments and the options when the command never got to record them itself. The terminate event carries the input after it was bound to the command definition, so the profile of a stopped command lists its arguments and options the same way as the profile of a command that ran. Nothing a command that does run records is affected, since `run()` assigns all four itself and the guard then skips them. One more thing on the same path: `CliRequest::getResponse()` returned an anonymous `Response` overriding `getStatusCode()`, which is ``@final``, so every profiled command emitted a self deprecation. An exit code is not an HTTP status and does not pass the validation in `setStatusCode()`, where `0` and `113` are both rejected, so the property is written directly instead. Three tests come with it, on a path that had no coverage until now: a command stopped by a listener that disables it, a command stopped by a listener that throws, and a command that runs. Found while writing a console listener that denies access at `ConsoleEvents::COMMAND`. Commits ------- fe42885 [Console][FrameworkBundle] Fix profiling a command stopped at ConsoleEvents::COMMAND
2 parents ce8b429 + fe42885 commit 19a9f0c
Copy full SHA for 19a9f0c

5 files changed

+151-7Lines changed: 151 additions & 7 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎src/Symfony/Bundle/FrameworkBundle/EventListener/ConsoleProfilerListener.php‎

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/EventListener/ConsoleProfilerListener.php
+11-2Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,17 @@ public function profile(ConsoleTerminateEvent $event): void
122122
}
123123
}
124124

125-
$request->command->exitCode = $event->getExitCode();
126-
$request->command->interruptedBySignal = $event->getInterruptingSignal();
125+
$command = $request->command;
126+
$command->exitCode = $event->getExitCode();
127+
$command->interruptedBySignal = $event->getInterruptingSignal();
128+
129+
if (!isset($command->input)) {
130+
// the command was stopped before it could run and record what it was given
131+
$command->input = $input = $event->getInput();
132+
$command->output = $event->getOutput();
133+
$command->arguments = $input->getArguments();
134+
$command->options = $input->getOptions();
135+
}
127136

128137
$profile = $this->profiler->collect($request, $request->getResponse(), $error);
129138
$this->profiles[$request] = $profile;
Collapse file

‎src/Symfony/Bundle/FrameworkBundle/Tests/Functional/ConsoleProfilerTest.php‎

Copy file name to clipboard
+108Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bundle\FrameworkBundle\Tests\Functional;
13+
14+
use Symfony\Bundle\FrameworkBundle\Console\Application;
15+
use Symfony\Component\Console\Command\Command;
16+
use Symfony\Component\Console\ConsoleEvents;
17+
use Symfony\Component\Console\Event\ConsoleCommandEvent;
18+
use Symfony\Component\Console\Input\ArrayInput;
19+
use Symfony\Component\Console\Input\InputArgument;
20+
use Symfony\Component\Console\Output\NullOutput;
21+
use Symfony\Component\HttpKernel\Profiler\Profile;
22+
23+
class ConsoleProfilerTest extends AbstractWebTestCase
24+
{
25+
protected function setUp(): void
26+
{
27+
static::bootKernel(['test_case' => 'ConsoleProfiler', 'debug' => true]);
28+
static::getContainer()->get('public_profiler')->purge();
29+
}
30+
31+
public function testProfileCommandDisabledByListener()
32+
{
33+
$application = $this->createApplication();
34+
$this->stopCommandWith(static fn (ConsoleCommandEvent $event) => $event->disableCommand());
35+
36+
$exitCode = $application->run(new ArrayInput(['command' => 'app:profiled', 'name' => 'Fabien', '--profile' => true]), new NullOutput());
37+
38+
$this->assertSame(ConsoleCommandEvent::RETURN_CODE_DISABLED, $exitCode);
39+
40+
$profile = $this->loadProfile();
41+
42+
$this->assertStringContainsString('app:profiled', $profile->getUrl());
43+
$this->assertSame(ConsoleCommandEvent::RETURN_CODE_DISABLED, $profile->getStatusCode());
44+
$this->assertSame('Fabien', $profile->getCollector('command')->getArguments()['name']->getValue());
45+
}
46+
47+
public function testProfileCommandStoppedByThrowingListener()
48+
{
49+
$application = $this->createApplication();
50+
$this->stopCommandWith(static fn () => throw new \RuntimeException('Access denied.'));
51+
52+
try {
53+
$application->run(new ArrayInput(['command' => 'app:profiled', 'name' => 'Fabien', '--profile' => true]), new NullOutput());
54+
$this->fail('The listener should have stopped the command.');
55+
} catch (\RuntimeException $e) {
56+
$this->assertSame('Access denied.', $e->getMessage());
57+
}
58+
59+
$profile = $this->loadProfile();
60+
61+
$this->assertSame(1, $profile->getStatusCode());
62+
$this->assertSame('Fabien', $profile->getCollector('command')->getArguments()['name']->getValue());
63+
$this->assertTrue($profile->getCollector('exception')->hasException());
64+
}
65+
66+
public function testProfileCommandThatRuns()
67+
{
68+
$application = $this->createApplication();
69+
70+
$exitCode = $application->run(new ArrayInput(['command' => 'app:profiled', 'name' => 'Fabien', '--profile' => true]), new NullOutput());
71+
72+
$this->assertSame(Command::SUCCESS, $exitCode);
73+
74+
$profile = $this->loadProfile();
75+
76+
$this->assertSame(Command::SUCCESS, $profile->getStatusCode());
77+
$this->assertSame('Fabien', $profile->getCollector('command')->getArguments()['name']->getValue());
78+
}
79+
80+
private function createApplication(): Application
81+
{
82+
$command = new Command('app:profiled');
83+
$command->addArgument('name', InputArgument::REQUIRED);
84+
$command->setCode(static fn (): int => Command::SUCCESS);
85+
86+
$application = new Application(static::$kernel);
87+
$application->setAutoExit(false);
88+
$application->setCatchExceptions(false);
89+
$application->add($command);
90+
91+
return $application;
92+
}
93+
94+
private function stopCommandWith(callable $listener): void
95+
{
96+
static::getContainer()->get('event_dispatcher')->addListener(ConsoleEvents::COMMAND, $listener);
97+
}
98+
99+
private function loadProfile(): Profile
100+
{
101+
$profiler = static::getContainer()->get('public_profiler');
102+
$tokens = $profiler->find('', '', 2, '', '', '');
103+
104+
$this->assertCount(1, $tokens);
105+
106+
return $profiler->loadProfile($tokens[0]['token']);
107+
}
108+
}
Collapse file

‎src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/ConsoleProfiler/bundles.php‎

Copy file name to clipboard
+16Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
use Symfony\Bundle\FrameworkBundle\FrameworkBundle;
13+
14+
return [
15+
new FrameworkBundle(),
16+
];
Collapse file

‎src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/ConsoleProfiler/config.yml‎

Copy file name to clipboard
+13Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
imports:
2+
- { resource: ../config/default.yml }
3+
4+
framework:
5+
http_method_override: false
6+
profiler:
7+
enabled: true
8+
collect: true
9+
10+
services:
11+
public_profiler:
12+
alias: profiler
13+
public: true
Collapse file

‎src/Symfony/Component/Console/Debug/CliRequest.php‎

Copy file name to clipboardExpand all lines: src/Symfony/Component/Console/Debug/CliRequest.php
+3-5Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,12 @@ public function getMethod(): string
4949
public function getResponse(): Response
5050
{
5151
return new class($this->command->exitCode) extends Response {
52-
public function __construct(private readonly int $exitCode)
52+
public function __construct(int $exitCode)
5353
{
5454
parent::__construct();
55-
}
5655

57-
public function getStatusCode(): int
58-
{
59-
return $this->exitCode;
56+
// getStatusCode() is final and setStatusCode() rejects an exit code
57+
$this->statusCode = $exitCode;
6058
}
6159
};
6260
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.