Skip to content

Proposing Flex-specific error messages in the controller shortcuts #25133

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 24, 2017
Merged

Proposing Flex-specific error messages in the controller shortcuts #25133

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions src/Symfony/Bundle/FrameworkBundle/Controller/ControllerTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ protected function file($file, string $fileName = null, string $disposition = Re
protected function addFlash(string $type, string $message)
{
if (!$this->container->has('session')) {
throw new \LogicException('You can not use the addFlash method if sessions are disabled.');
throw new \LogicException('You can not use the addFlash method if sessions are disabled. Enable them in config/packages/framework.yaml.');
}

$this->container->get('session')->getFlashBag()->add($type, $message);
Expand All @@ -169,7 +169,7 @@ protected function addFlash(string $type, string $message)
protected function isGranted($attributes, $subject = null): bool
{
if (!$this->container->has('security.authorization_checker')) {
throw new \LogicException('The SecurityBundle is not registered in your application.');
throw new \LogicException('The SecurityBundle is not registered in your application. Try running "composer require security"');
}

return $this->container->get('security.authorization_checker')->isGranted($attributes, $subject);
Expand Down Expand Up @@ -206,7 +206,7 @@ protected function renderView(string $view, array $parameters = array()): string
}

if (!$this->container->has('twig')) {
throw new \LogicException('You can not use the "renderView" method if the Templating Component or the Twig Bundle are not available.');
throw new \LogicException('You can not use the "renderView" method if the Templating Component or the Twig Bundle are not available. Try running "composer require twig"');
}

return $this->container->get('twig')->render($view, $parameters);
Expand All @@ -224,7 +224,7 @@ protected function render(string $view, array $parameters = array(), Response $r
} elseif ($this->container->has('twig')) {
$content = $this->container->get('twig')->render($view, $parameters);
} else {
throw new \LogicException('You can not use the "render" method if the Templating Component or the Twig Bundle are not available.');
throw new \LogicException('You can not use the "render" method if the Templating Component or the Twig Bundle are not available. Try running "composer require twig"');
}

if (null === $response) {
Expand Down Expand Up @@ -256,7 +256,7 @@ protected function stream(string $view, array $parameters = array(), StreamedRes
$twig->display($view, $parameters);
};
} else {
throw new \LogicException('You can not use the "stream" method if the Templating Component or the Twig Bundle are not available.');
throw new \LogicException('You can not use the "stream" method if the Templating Component or the Twig Bundle are not available. Try running "composer require twig"');
}

if (null === $response) {
Expand Down Expand Up @@ -326,7 +326,7 @@ protected function createFormBuilder($data = null, array $options = array()): Fo
protected function getDoctrine(): ManagerRegistry
{
if (!$this->container->has('doctrine')) {
throw new \LogicException('The DoctrineBundle is not registered in your application.');
throw new \LogicException('The DoctrineBundle is not registered in your application. Try running "composer require doctrine"');
}

return $this->container->get('doctrine');
Expand All @@ -346,7 +346,7 @@ protected function getDoctrine(): ManagerRegistry
protected function getUser()
{
if (!$this->container->has('security.token_storage')) {
throw new \LogicException('The SecurityBundle is not registered in your application.');
throw new \LogicException('The SecurityBundle is not registered in your application. Try running "composer require security"');
}

if (null === $token = $this->container->get('security.token_storage')->getToken()) {
Expand All @@ -372,7 +372,7 @@ protected function getUser()
protected function isCsrfTokenValid(string $id, string $token): bool
{
if (!$this->container->has('security.csrf.token_manager')) {
throw new \LogicException('CSRF protection is not enabled in your application.');
throw new \LogicException('CSRF protection is not enabled in your application. Enable it with the "csrf_protection" key in "config/packages/framework.yaml"');
Copy link
Member

Choose a reason for hiding this comment

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

Flex should be auto-enabling it when requiring the security-csrf package, no ? If yes, the right advice is to require it too

Copy link
Member Author

Choose a reason for hiding this comment

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

That seems logical to me, bit I actually don't think it's enabled by default. It looks like it's always disabled unless you explicitly enable it: https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php#L107-L116

Copy link
Member

Choose a reason for hiding this comment

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

😢 why making FrameworkBundle auto-enable all components except one ?

Copy link
Member

Choose a reason for hiding this comment

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

Looks like something we need to fix.

Copy link
Member

Choose a reason for hiding this comment

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

Anybody willing to submit a PR on that one? Would be cool to have it for 3.4/4.0 before final.

Copy link
Contributor

@sroze sroze Nov 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@fapbot here you go: #25151

Copy link
Contributor

Choose a reason for hiding this comment

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

Enable them in config/packages/framework.yaml. vs. Enable it with the "csrf_protection" key in "config/packages/framework.yaml"

always be specific/generic?

}

return $this->container->get('security.csrf.token_manager')->isTokenValid(new CsrfToken($id, $token));
Expand Down