This PR was submitted for the 5.x branch but it was merged into the 5.2 branch instead.
Discussion
----------
[ErrorHandler] Fix error caused by `include` + open_basedir
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
https://github.com/symfony/symfony/pull/37492 introduced the option to specify your own error template using `HtmlErrorRenderer::setTemplate('...');`
However, the implementation using `file_exists(...)` in `include` can cause an error when not setting a custom template.
> php.WARNING: Warning: file_exists(): open_basedir restriction in effect. File(assets/css/error.css) is not within the allowed path(s): (...) {"exception":"[object] (ErrorException(code: 0): Warning: file_exists(): open_basedir restriction in effect. File(assets/css/error.css) is not within the allowed path(s): (...) at ...\\vendor\\symfony\\error-handler\\ErrorRenderer\\HtmlErrorRenderer.php:355)"} []
As you can see the error is caused by checking `file_exists(...)` using relative paths in environments with a restrictive `open_basedir` policy.
The proposed solution always uses absolute paths to include errors templates (and other files).
Commits
-------
9ad7832acd [ErrorHandler] Fix error caused by `include` + open_basedir
This PR was squashed before being merged into the 5.2 branch.
Discussion
----------
[FrameworkBundle] Make the TestBrowserToken interchangeable with other tokens
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
For some reason most tokens implement getFirewallName (and the deprecated getProviderKey), but not all. This adds it to TestBrowserToken because it caused issues in my tests when upgrading to Symfony 5's `Client::loginUser()`.
I am also happy to add this to AnonymousToken but I am more worried about BC impacts there, so I left it alone for now. All other token classes implement it, so I am not sure why this isn't in TokenInterface to begin with.
Commits
-------
a4958ae7ad [FrameworkBundle] Make the TestBrowserToken interchangeable with other tokens
This PR was squashed before being merged into the 4.4 branch.
Discussion
----------
[Console] ProgressBar clears too many lines on update
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? |no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? |no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| License | MIT
The ProgressBar incorrectly calculates line lengths when updating, including non-displayable characters such as ANSI colors. This causes it to clear too many lines if the terminal width is greater than the displayed line length but less than the line length including non-displayed characters. An example of this bug in action is https://github.com/acquia/cli/issues/467
Commits
-------
2aa3df0c74 [Console] ProgressBar clears too many lines on update
This PR was squashed before being merged into the 4.4 branch.
Discussion
----------
[FrameworkBundle] Exclude unreadable files when executing About command
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix https://github.com/symfony/symfony/issues/39916
| License | MIT
| Doc PR | ~
Fix as explained in: https://github.com/symfony/symfony/issues/39916.
Tests failed looks like not releated to my changes.
Commits
-------
25f503a813 [FrameworkBundle] Exclude unreadable files when executing About command
This PR was merged into the 5.3-dev branch.
Discussion
----------
[DependencyInjection] Add `#[TaggedItem]` attribute for defining the index and priority of classes found in tagged iterators/locators
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Next to #39804, this PR adds a new `#[TaggedItem]` attribute that ppl can use to define the index of their service classes when they're used in tagged collections (iterators/locators.
This replaces the `public static getDefaultName()` and `getDefaultPriority()` methods that ppl could use for this purpose:
```php
#[TaggedItem(index: 'api.logger', priority: 123)]
class MyApiLogger implements LoggerInterface
{
}
```
This will ship the corresponding service at index `api.logger`, priority=123 when building locators/iterators.
Commits
-------
252f2ca1fb [DependencyInjection] Add `#[TaggedItem]` attribute for defining the index and priority of classes found in tagged iterators/locators
This PR was merged into the 4.4 branch.
Discussion
----------
Be explicit about anchor background color in profiler toolbar
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| License | MIT
Hello,
we have a project where we work with background colors on links. When we define this in our stylesheet, the Web Profiler picks these styles up due to the cascade. We added two CSS statements to the Web Profiler stylesheet to be explicit about the transparent background.
Commits
-------
f0cac47ae7 Be explicit about transparent background color of links in toolbar
This PR was submitted for the 5.x branch but it was squashed and merged into the 4.4 branch instead.
Discussion
----------
[Bridge\Twig] Add 'form-control-range' for range input type
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Add 'form-control-range' for range input type for Bootstrap theme, [see the doc](https://getbootstrap.com/docs/4.6/components/forms/#range-inputs)
Commits
-------
e73db2b1f1 [Bridge\Twig] Add 'form-control-range' for range input type
This PR was merged into the 5.2 branch.
Discussion
----------
make async-ses required
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#40480
| License | MIT
| Doc PR | symfony/symfony-docs#... <!-- required for new features -->
AWS S3 signature is not supported anymore, which mean the old "raw HTTP" implementation does not work anymore, and only the implementation that use "async-aws" works.
This PR move "async-aws" to the "requires" section in order to fix that.
Commits
-------
7904d0896b Make async-ses required
This PR was merged into the 4.4 branch.
Discussion
----------
[Cache] fix test case name
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Should make #40468 green.
Commits
-------
e2dffea49d [Translation] fix test case name
This PR was merged into the 4.4 branch.
Discussion
----------
[4.4] Fix wrong namespace in tests
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Commits
-------
b3759c2882 [Cache] Fix wrong namespace in test
This PR was merged into the 5.2 branch.
Discussion
----------
[Routing] Remove unnecessary references to User class in test fixtures
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Commits
-------
76ee9baf8e [Routing] Remove unnecessary references to User class in test fixtures
* 5.2:
[Mailer] fix lowest allowed dependencies
Refresh original user in SwitchUserListener.
check if templating engine supports view
[Mime] Escape commas in address names
* 4.4:
[Mailer] fix lowest allowed dependencies
Refresh original user in SwitchUserListener.
check if templating engine supports view
[Mime] Escape commas in address names
This PR was merged into the 5.3-dev branch.
Discussion
----------
[Validator] Add Validation::createIsValidCallable() that returns a boolean instead of exception
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | Fix#36820
| License | MIT
| Doc PR | tbd
This adds a `Validator::createValidCallable()` (I'm very open for other name suggestions) that returns a boolean instead of exceptions. This allows usingit in places where booleans are expected, for instance in the referenced OptionsResolver case:
```php
$resolver->setAllowedValues('name', Validation::createValidCallable(
new Assert\Length(['min' => 10 ])
));
```
Commits
-------
e731f5fda9 [Validator] Add createValidCallable() that returns a boolean
This PR was merged into the 4.4 branch.
Discussion
----------
[Mime] Escape commas in address names
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#39416
| License | MIT
| Doc PR | --
<!--
Replace this notice by a short README for your feature/bugfix. This will help people
understand your PR and can be used as a start for the documentation.
Additionally (see https://symfony.com/releases):
- Always add tests and ensure they pass.
- Never break backward compatibility (see https://symfony.com/bc).
- Bug fixes must be submitted against the lowest maintained branch where they apply
(lowest branches are regularly merged to upper ones so they get the fixes too.)
- Features and deprecations must be submitted against branch 5.x.
-->
Before:
```php
$address = new Address('fabien@symfony.com', 'Fabien, Potencier');
$address->toString(); // Fabien, Potencier <fabien@symfony.com> -> Interpreted like two emails
```
After:
```php
$address = new Address('fabien@symfony.com', 'Fabien, Potencier');
$address->toString(); // "Fabien, Potencier" <fabien@symfony.com>
```
Commits
-------
39e9158999 [Mime] Escape commas in address names
This PR was merged into the 5.3-dev branch.
Discussion
----------
[Console] improve exception message when required argument is added after an optional one
| Q | A
| ------------- | ---
| Branch? | 5.x <!-- see below -->
| Bug fix? | no
| New feature? | wouldn't say so, rather DX improvement <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | Fix#40302 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License | MIT
| Doc PR | n/a
Hello, this is my first contribution to Symfony Framework. It's time to pull my weight.
About the issue:
I did improve an error message to include passed argument's name and the latest optional argument's name.
An author also mentioned "But which command?", however this is shown twice as we can see on the screenshot above so I skipped that.
Commits
-------
9bddbbd7ed#40302 improve exception message when required argument is added after an optional one
This PR was merged into the 5.3-dev branch.
Discussion
----------
[HttpFoundation] Use InputBag for POST requests too
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | Partially revert #37265
| License | MIT
| Doc PR | -
#37265 was created as a fix for #37100. However, when #37327 was merged, the original bug was also fixed with a different solution (allowing null values on `InputBag::set`) and parts of #37265 are not needed anymore. By using only `InputBag` as the `$request` bag we can tighten the typehint again and make static analysis a bit more useful.
Commits
-------
381a0a19f7 use InputBag for POST requests too, added missing scalar type hints
This PR was merged into the 4.4 branch.
Discussion
----------
Check if templating engine supports given view
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Currently the `ControllerTrait::render` and `ControllerTrait::renderView` methods assume that when the `templating` service is available it can always render the given view via that service.
However, that might not be the case. For example if `framework.templating` is not configured to support the `twig` engine for example but only some other custom engine, e.g.
```yaml
framework:
templating:
engines:
- foobar
```
it will fail when the given view references a Twig template for instance and the configured engine cannot render that.
The `Symfony\Component\Templating\EngineInterface` which `templating` implements defines a `support` method:
2536e178b1/src/Symfony/Component/Templating/EngineInterface.php (L56-L63)
which should be used here, in order to make sure that `templating` actually does support rendering the given view.
Commits
-------
0f9434c189 check if templating engine supports view
This PR was squashed before being merged into the 5.3-dev branch.
Discussion
----------
[FrameworkBundle] Add KernelTestCase::getContainer()
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? | yes
| Tickets |
| License | MIT
| Doc PR | https://github.com/symfony/symfony-docs/pull/14731
There are at least 3 ways to get the container in a test class:
```php
class FooTest extends WebTestCase
{
public function testGetContainerA()
{
$kernel = self::bootKernel();
$container = $kernel->getContainer();
}
public function testGetContainerB()
{
self::bootKernel();
$container = self::$container;
}
public function testGetContainerC()
{
$client = self::createClient();
$container = $client->getContainer();
}
}
```
I suggest to add a fourth =)
Basically, in tests you should always use the `test.service_container`. It is hard to remove A and C, but I can deprecate C and add a helper function.
```php
class FooTest extends WebTestCase
{
public function testGetContainerTheOnlyWayYouShouldUse()
{
$container = $this->getContainer();
}
}
```
This new way will also boot your kernel if it is not already booted.
Commits
-------
f4c97240ff [FrameworkBundle] Add KernelTestCase::getContainer()
This PR was squashed before being merged into the 5.3-dev branch.
Discussion
----------
[FrameworkBundle] AnnotationsCacheWarmer should support doctrine/annotations:^1.13
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#40353
| License | MIT
| Doc PR |
The `AnnotationsCacheWarmer` is creating a cached a `new CachedReader` in the `doWarmUp()`. But this will fail if doctrine/cache is not installed. In #40338 I added the `kernel.cache_warmer` tag on `annotations.cache_warmer` service even though the doctrine/cache is not installed.
This PR will make sure `AnnotationsCacheWarmer` is using the `PsrCacheReader` when available.
This bug was not found in the tests because doctrine/cache is always installed.
----------
Big golden star to @jrushlow because you test dev-master.
Commits
-------
fb1cc72b18 [FrameworkBundle] AnnotationsCacheWarmer should support doctrine/annotations:^1.13
This PR was merged into the 4.4 branch.
Discussion
----------
[Security] Refresh original user in SwitchUserListener
Fixes#39991
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#39991
| License | MIT
| Doc PR | NA
Fix SwitchUserListener to update original token with refreshed user. This prevents a non-refreshed user from causing problems elsewhere, such as in Voters.
Commits
-------
42453454c9 Refresh original user in SwitchUserListener.
This PR was squashed before being merged into the 5.3-dev branch.
Discussion
----------
[WebProfilerBundle] Disable CSP if dumper was used
| Q | A
| ------------- | ---
| Branch? | 5.x for features
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | Fix#29084
| License | MIT
| Doc PR | -
Disables a configured Content Security Policy if the dumper was used to avoid breaking the toolbar. This is a less invasive alternative to #29155 which fixes#29084 (there is a project with a test case linked there).
Commits
-------
5dc2637263 [WebProfilerBundle] Disable CSP if dumper was used
This PR was squashed before being merged into the 5.3-dev branch.
Discussion
----------
[twig-bridge] Allow NotificationEmail to be marked as public
| Q | A
| ------------- | ---
| Branch? | 5.x for features
| Bug fix? |no
| New feature? | yes
| Deprecations? |no
| Tickets | Fix#37879
| License | MIT
| Doc PR | -
Closes https://github.com/symfony/symfony/issues/37879
`NotificationEmail` can be created public from the `asPublicEmail` method in any context but with the Notifier component. In this case it is created by it as not public and therefore displays the importance in the subject of the email.
For end users, aka not admin, the importance in the subject's email is not necessary.
This PR will allow if needed to set a `NotificationEmail` as public even after being created, wherever it was created.
I am not sure how to handle the version in the changelog.
For the tests I don't know what's the policy so I just added a new case in each test case.
Commits
-------
8f753d27f2 [twig-bridge] Allow NotificationEmail to be marked as public
