This PR was merged into the 4.4 branch.
Discussion
----------
[HttpKernel] Configure `session.cookie_secure` earlier
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#40221
| License | MIT
| Doc PR | N/A
This PR does what @stof had suggested in #40221, allow me to quote him directly:
> 1. avoid setting auto as a value for the ini setting in the NativeSessionStorage initialization
> 2. ensuring that SessionListener resolves the auto value by the time the SessionListener runs, and not by the time the getSession() method is called in the Request session factory callback
Commits
-------
e82918cd60 [HttpKernel] Configure `session.cookie_secure` earlier
This PR was squashed before being merged into the 4.4 branch.
Discussion
----------
Make sure the Psalm review CI job is working
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
This PR is just a test to make sure psalm works as expected.
EDIT: It also fixes issues..
Commits
-------
d5a05f1b30 Make sure the Psalm review CI job is working
This PR was squashed before being merged into the 4.4 branch.
Discussion
----------
Adding a Github action to run Psalm
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR | https://github.com/symfony/symfony-docs/pull/15024
I've seen sometimes that we've forgotten to add `\` before `Throwable` or that we refer to a class that does not exist. One could argue that the code is not properly tested, but somehow these PRs still get merged. (And quickly fixed in a follow up PR).
I suggest to add psalm to check every PR for some errors that can be found with a static analyser. This is to help/automate the PR review process. All psalm errors found should be reviewed and discussed. The maintainers can decide to ignore some warnings if they want to. (Ie false positives)
This PR is about “Psalm PR review”. It does not try to fix “Psalm compatibility”. Psalm compatibility is a separate issue that should be discussed separate from the "Psalm PR review".
I currently plan to follow up with the more controversial topic of "Should we make Symfony more compatible with Psalm or not".
Commits
-------
c5ed24d8cb Adding a Github action to run Psalm
This PR was merged into the 4.4 branch.
Discussion
----------
[TwigBridge] Install symfony/intl to run tests on Travis
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | bi
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
The hard dependency on `symfony/intl` was removed from the Form component in 5.3-dev (#40298). I suggest to add the explicit dev dependency on TwigBridge on 4.4 already.
Commits
-------
b2970456bf [TwigBridge] Install symfony/intl to run tests on Travis
This PR was submitted for the 5.2 branch but it was merged into the 4.4 branch instead.
Discussion
----------
[Translation] Make `name` attribute optional in xliff2
Do not set a fake `name` attribute on `unit` element from xliff2 to allow using `source` attribute and avoid missing translation error
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | yes/no
| New feature? | no
| Deprecations? | no
| Tickets | Fix#37055
| License | MIT
| Doc PR | symfony/symfony-docs#... <!-- required for new features -->
When `xlf` translations are loaded, if a name exists on `unit` element, the segment's source is ignored:
```foreach ($xml->xpath('//xliff:unit') as $unit) {
foreach ($unit->segment as $segment) {
$attributes = $unit->attributes();
$source = $attributes['name'] ?? $segment->source;
```
At the same time, when dumping translations, the segment's source is copied into the unit's name attribute, unless it's longer than 80 characters. In that case, `substr(md5($source), -7)` is set into the name attribute.
This results in a missing translation error, because the source is ignored and the name is a random string.
Suggested solution: only set the name attribute if the string is less than 80 characters.
Commits
-------
97058559cc [Translation] Make `name` attribute optional in xliff2
This PR was merged into the 5.2 branch.
Discussion
----------
[Security] #[CurrentUser] arguments should resolve to null for "anon."
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
The UserValueResolver should only resolve `UserInterface` (or subtype) typed arguments:
bc9e946a56/src/Symfony/Component/Security/Http/Controller/UserValueResolver.php (L54-L55)
When using the `#CurrentUser` attribute with an AnonymousToken in the storage, the resolved argument value is `anon.`. This PR fixes it.
/cc @jvasseur
Commits
-------
8d3078dd35 [Security] #[CurrentUser] argument should resolve to null when it is anonymous
This PR was merged into the 5.2 branch.
Discussion
----------
[Config] Switched to non-null defaults in exception constructors
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | N/A
| License | MIT
| Doc PR | N/A
Follow-up to #40271 on the 5.2 branch.
Commits
-------
2e865ac057 Switched to non-null defaults in exception constructors
This PR was submitted for the 5.x branch but it was merged into the 5.2 branch instead.
Discussion
----------
[FrameworkBundle] Allow x-forwarded-prefix trusted header in config
| Q | A
| ------------- | ---
| Branch? | 5.2 (as requested by @nicolas-grekas)
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| License | MIT
Support for `X_FORWARDED_PREFIX` has been added in PR https://github.com/symfony/symfony/pull/37734.
However, it is impossible to use it because the configuration doesn't allow the `x-forwarded-prefix` value in `framework.yaml`.
Commits
-------
95fdd90491 Allow x-forwarded-prefix trusted header.
* 4.4:
Move github templates at the org level
[Cache] Fix Redis TLS scheme `rediss` for Redis connection
In calls to mb_ functions, silently transform arg into string
This PR was merged into the 4.4 branch.
Discussion
----------
[Console] Handle calls to mb_ functions with non string arguments
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#40200
| License | MIT
| Doc PR | no
In PHP8.1, a number of functions who were accepting null arguments will only accept
string ones.
(see https://wiki.php.net/rfc/deprecate_null_to_scalar_internal_arg)
In the polyfill, mb_* functions are already declared with a strict type checking of "string".
Therefore, it is necessary to get rid of the use of non string arguments when calling mb_* functions,
so that it won't break when either using the polyfill,or future php8 versions.
In every call where the argument may not be a string, this commit enforces the string type of the argument (with transtyping)
--- For reviewers
* I generally don't like transtyping, but found it was the more "secure" way (on a non-BC point of view) here.
Specially in Console/Helper/Table.php, where $cell can be an object (there are 2 "$cell instanceof ... tests)
However, where the argument can already be either null or string (and not anything else), there may a beter approach ?
* It's the first time I send a PR on symfony, so don't hesitate pointing me to thinks I've forgotten to done.
Commits
-------
ac45be2580 In calls to mb_ functions, silently transform arg into string
This PR was submitted for the 5.x branch but it was merged into the 4.4 branch instead.
Discussion
----------
[Cache] Fix Redis TLS scheme `rediss` for Redis connection
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR | https://github.com/symfony/symfony-docs/pull/14728
Like https://github.com/symfony/symfony/pull/35503 on Symfony Messenger, this will enable TLS support for Redis adapter.
The implementation just prefix the host with `tls://` as described here: https://github.com/phpredis/phpredis#connect-open
I don't know how to test it because I guess I need a TLS Redis in `src/Symfony/Component/Cache/Tests/Adapter/RedisAdapterTest.php`.
Commits
-------
3288897e0f [Cache] Fix Redis TLS scheme `rediss` for Redis connection
In PHP8, a number of functions who were accepting null arguments will only accept
string ones.
In the polyfill, mb_* functions are declared with a trict type checking of "string".
Therefore, we deprecate the use of non string arguments, so that it won't break when either using the polyfill,
or future php8 versions.
* 4.4:
Switched to non-null defaults in exception constructors
[Routing] fix conflict with param named class in attribute
[Cache] fix setting items' metadata on commit()
This PR was merged into the 4.4 branch.
Discussion
----------
Switched to non-null defaults in exception constructors
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | N/A
| License | MIT
| Doc PR | N/A
PHP 8.1 will trigger a deprecation warning if we pass `null` as `$message` or `$code` to the constructor of `\Exception`. However, many of our own exception accept `null` for those parameters and even use them as default.
This is unfortunate because code like the following snippet would trigger that deprecation although the code itself is perfectly fine:
```php
throw new NotFoundHttpException();
```
With this PR, I'd like to change our defaults to `''` and `0` while still allowing to pass `null` for BC. In a follow-up PR for the 5.x branch, I'd like to deprecate passing `null`, matching the future behavior of PHP.
This PR also adjust various PHPDoc blocks with inaccurate types.
Commits
-------
f8e10094a4 Switched to non-null defaults in exception constructors
This PR was squashed before being merged into the 4.4 branch.
Discussion
----------
[Routing] fix conflict with param named class in attribute
| Q | A
| ------------- | ---
| Branch | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#40225
| License | MIT
| Doc PR | -
Fix conflict with AnnotationFileLoader and class PHP8 Attribute with param named "class"
Commits
-------
27bba684d8 [Routing] fix conflict with param named class in attribute
* 4.4:
[Form] Ignoring invalid forms from delete_empty behavior in CollectionType
Add Symfony Armenian Translations
cs fix
Add different header notations to tests
This PR was squashed before being merged into the 4.4 branch.
Discussion
----------
[Form] Ignoring invalid forms from delete_empty behavior in CollectionType
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#37660
| License | MIT
| Doc PR | -
As expained on linked issue, empty forms cannot be removed if they are invalid upon submitting (e.g. transformation failures). Thus, form errors can be displayed properly to the end user.
Commits
-------
e4911554e3 [Form] Ignoring invalid forms from delete_empty behavior in CollectionType
This PR was merged into the 4.4 branch.
Discussion
----------
Add Symfony Armenian Translations
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | Fix#40204
| License | MIT
| Doc PR |
Added missing armenian translations for Form components
Commits
-------
d1c3e21eff Add Symfony Armenian Translations
This PR was merged into the 4.4 branch.
Discussion
----------
[BrowserKit] Add different header notations to tests
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | Fix#29736
| License | MIT
| Doc PR | -
In the past there was a issue #29736 that the header wasn't processed correctly. To reproduce and prevent this behaviour i extended the tests for this. At the end everything is fine, also in 5.x branch. See bf03d6889d.
Commits
-------
251ce8f924 Add different header notations to tests
This PR was merged into the 4.4 branch.
Discussion
----------
Stop using deprecated ArrayCache from Doctrine
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Follows https://github.com/doctrine/cache/pull/355
Commits
-------
15f021f825 Stop using deprecated ArrayCache from Doctrine