8000 [FrameworkBundle] Deprecate absolute template paths by jakzal · Pull Request #18034 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[FrameworkBundle] Deprecate absolute template paths #18034

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
Mar 15, 2016
Merged

[FrameworkBundle] Deprecate absolute template paths #18034

Show file tree
Hide file tree
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
[FrameworkBundle] Deprecate absolute template paths
  • Loading branch information
@jakzal
jakzal committed Mar 8, 2016
commit 85a9d67e93621e320423f7d046faf6ea9ab1b12a
6 changes: 6 additions & 0 deletions UPGRADE-3.1.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ Form
in `ResizeFormListener::preSubmit` method has been deprecated and will be
removed in Symfony 4.0.

FrameworkBundle
---------------

* As it was never an officially supported feature, the support for absolute
template paths has been deprecated and will be removed in Symfony 4.0.

HttpKernel
----------

Expand Down
5 changes: 5 additions & 0 deletions UPGRADE-4.0.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ Form
* Support for data objects that implements both `Traversable` and
`ArrayAccess` in `ResizeFormListener::preSubmit` method has been removed

FrameworkBundle
---------------

* Support for absolute template paths has been removed from the template name parser.

HttpKernel
----------

Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ CHANGELOG
-----

* Added `Controller::json` to simplify creating JSON responses when using the Serializer component
* Deprecated absolute template paths support in the template name parser

3.0.0
-----
Expand Down
10 changes: 8 additions & 2 deletions src/Symfony/Bundle/FrameworkBundle/Templating/TemplateNameParser.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public function parse($name)
throw new \RuntimeException(sprintf('Template name "%s" contains invalid characters.', $name));
}

if (!preg_match('/^(?:([^:]*):([^:]*):)?(.+)\.([^\.]+)\.([^\.]+)$/', $name, $matches) || $this->isAbsolutePath($name) || 0 === strpos($name, '@')) {
if ($this->isAbsolutePath($name) || !preg_match('/^(?:([^:]*):([^:]*):)?(.+)\.([^\.]+)\.([^\.]+)$/', $name, $matches) || 0 === strpos($name, '@')) {
return parent::parse($name);
}

Expand All @@ -74,6 +74,12 @@ public function parse($name)

private function isAbsolutePath($file)
{
return (bool) preg_match('#^(?:/|[a-zA-Z]:)#', $file);
$isAbsolute = (bool) preg_match('#^(?:/|[a-zA-Z]:)#', $file);

if ($isAbsolute) {
@trigger_error('Absolute template path support is deprecated since Symfony 3.1 and will be removed in 4.0.', E_USER_DEPRECATED);
}

return $isAbsolute;
}
}
42 changes: 37 additions & 5 deletions src/Symfony/Bundle/FrameworkBundle/Tests/Templating/TemplateNameParserTest.php
View file
Open in desktop
< 8000 div class="js-file-content Details-content--hidden position-relative" data-hydro-view="{"event_type":"pull_request.select_diff_range","payload":{"actor_id":null,"pull_request_id":61850609,"repository_id":458058,"diff_type":"UNIFIED","whitespace_ignored":false,"originating_url":"https://github.com/symfony/symfony/pull/18034/commits/85a9d67e93621e320423f7d046faf6ea9ab1b12a","user_id":null}}" data-hydro-view-hmac="bcf25f786c24a73cecb6f20d0fb44047b7410afe86a73d3c6ef88482b46f8bf2">
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,6 @@ public function parseProvider()
array('FooBundle:Post:foo.bar.index.html.php', 'FooBundle:Post:foo.bar.index.html.php', '@FooBundle/Resources/views/Post/foo.bar.index.html.php', new TemplateReference('FooBundle', 'Post', 'foo.bar.index', 'html', 'php')),
array('@FooBundle/Resources/views/layout.html.twig', '@FooBundle/Resources/views/layout.html.twig', '@FooBundle/Resources/views/layout.html.twig', new BaseTemplateReference('@FooBundle/Resources/views/layout.html.twig', 'twig')),
array('@FooBundle/Foo/layout.html.twig', '@FooBundle/Foo/layout.html.twig', '@FooBundle/Foo/layout.html.twig', new BaseTemplateReference('@FooBundle/Foo/layout.html.twig', 'twig')),
array('/path/to/section/index.html.php', '/path/to/section/index.html.php', '/path/to/section/index.html.php', new BaseTemplateReference('/path/to/section/index.html.php', 'php')),
array('C:\\path\\to\\section\\name.html.php', 'C:path/to/section/name.html.php', 'C:path/to/section/name.html.php', new BaseTemplateReference('C:path/to/section/name.html.php', 'php')),
array('C:\\path\\to\\section\\name:foo.html.php', 'C:path/to/section/name:foo.html.php', 'C:path/to/section/name:foo.html.php', new BaseTemplateReference('C:path/to/section/name:foo.html.php', 'php')),
array('\\path\\to\\section\\name.html.php', '/path/to/section/name.html.php', '/path/to/section/name.html.php', new BaseTemplateReference('/path/to/section/name.html.php', 'php')),
array('/path/to/section/name.php', '/path/to/section/name.php', '/path/to/section/name.php', new BaseTemplateReference('/path/to/section/name.php', 'php')),
array('name.twig', 'name.twig', 'name.twig', new BaseTemplateReference('name.twig', 'twig')),
array('name', 'name', 'name', new BaseTemplateReference('name')),
array('default/index.html.php', '::default/index.html.php', 'views/default/index.html.php', new TemplateReference(null, null, 'default/index', 'html', 'php')),
Expand All @@ -86,4 +81,41 @@ public function testParseValidNameWithNotFoundBundle()
{
$this->parser->parse('BarBundle:Post:index.html.php');
}

/**
* @group legacy
* @dataProvider provideAbsolutePaths
*/
public function testAbsolutePathsAreDeprecated($name, $logicalName, $path, $ref)
{
$deprecations = array();
set_error_handler(function ($type, $msg) use (&$deprecations) {
if (E_USER_DEPRECATED !== $type) {
throw new \LogicException(sprintf('Unexpected error: "%s".', $msg));
}
Copy link
Member

Choose a reason for hiding this comment

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

just for safety, maybe we should do something in the "else" case? like forwarding to the previous error handler?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. To be honest I copied this one from other test. Perhaps we should fix in in all ocurences? I won't be able to look into this before evening today.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nicolas-grekas is throwing an exception like I did just now acceptable?


$deprecations[] = $msg;
});

$template = $this->parser->parse($name);

restore_error_handler();

$this->assertSame($ref->getLogicalName(), $template->getLogicalName());
$this->assertSame($logicalName, $template->getLogicalName());
$this->assertSame($path, $template->getPath());
$this->assertCount(1, $deprecations);
$this->assertContains('Absolute template path support is deprecated since Symfony 3.1 and will be removed in 4.0.', $deprecations[0]);
}

public function provideAbsolutePaths()
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we could add here a file:///... path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this a supported use case?

Currently, and even before recent changes to the TemplateNameParser, file:///path/to/section/index.html.php path would be converted to file:path/to/section/index.html.php

Copy link
Member

Choose a reason for hiding this comment

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

OK then. Nothing more to do :) Thanks.

{
return array(
array('/path/to/section/index.html.php', '/path/to/section/index.html.php', '/path/to/section/index.html.php', new BaseTemplateReference('/path/to/section/index.html.php', 'php')),
array('C:\\path\\to\\section\\name.html.php', 'C:path/to/section/name.html.php', 'C:path/to/section/name.html.php', new BaseTemplateReference('C:path/to/section/name.html.php', 'php')),
array('C:\\path\\to\\section\\name:foo.html.php', 'C:path/to/section/name:foo.html.php', 'C:path/to/section/name:foo.html.php', new BaseTemplateReference('C:path/to/section/name:foo.html.php', 'php')),
array('\\path\\to\\section\\name.html.php', '/path/to/section/name.html.php', '/path/to/section/name.html.php', new BaseTemplateReference('/path/to/section/name.html.php', 'php')),
array('/path/to/section/name.php', '/path/to/section/name.php', '/path/to/section/name.php', new BaseTemplateReference('/path/to/section/name.php', 'php')),
);
}
}
0