This PR was submitted for the 4.4 branch but it was merged into the 3.4 branch instead.
Discussion
----------
[FrameworkBundle] display actual target for error in AssetsInstallCommand
| 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 -->
| Tickets |
| License | MIT
When assets:install fails because the target directory does not exist, it should display the actual directory it wanted to have instead of the configuration directive. In most cases, the target directory is retrieved from the kernel config and thus differs from the argument.
Commits
-------
f177b3d488 [FrameworkBundle] display actual target for error in AssetsInstallCommand
When assets:install fails because the target directory does not exist, it should display the actual directory it wanted to have instead of the configuration directive. In most cases, the target directory is retrieved from the kernel config and thus differs from the argument.
This PR was squashed before being merged into the 3.4 branch.
Discussion
----------
Remove patches for Doctrine bugs and deprecations
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | no
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | n/a
| License | MIT
| Doc PR | n/a
This PR removes patches put in place because of a bug fixed in doctrine/dbal#3994, and because of deprecations addressed in doctrine/orm#7953
Commits
-------
2f305cdc83 Remove patches for Doctrine bugs and deprecations
This PR was merged into the 3.4 branch.
Discussion
----------
Force doctrine/dbal <=2.10.2 when testing
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Until https://github.com/doctrine/dbal/issues/3990 is resolved.
Commits
-------
d1953d61cd Force doctrine/dbal <=2.10.2 when testing
This PR was merged into the 4.4 branch.
Discussion
----------
[Console] don't check tty on stdin, it breaks with "data lost during stream conversion"
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
/cc @ostrolucky FYI
Commits
-------
6d089ac437 [Console] fix "data lost during stream conversion" with QuestionHelper
* 3.4:
[PhpUnitBridge] fix PHP 5.3 compat
[PhpUnitBridge] Mark parent class also covered in CoverageListener
prevent notice for invalid octal numbers on PHP 7.4
This PR was merged into the 3.4 branch.
Discussion
----------
[PhpUnitBridge] Mark parent class also covered in CoverageListener
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
Commits
-------
dcb5653728 [PhpUnitBridge] Mark parent class also covered in CoverageListener
* 3.4:
[Filesystem] Handle paths on different drives
[WebProfiler] Do not add src-elem CSP directives if they do not exist
[Yaml] fix parse error when unindented collections contain a comment
[3.4][Inflector] Improve testSingularize() argument name
[PhpUnitBridge] fix PHP 5.3 compat again
Skip validation when email is an empty object
fix sr_Latn translation
[Validator] fix lazy property usage.
Fix annotation
[PhpUnitBridge] fix compat with PHP 5.3
[DX] Show the ParseException message in YAML file loaders
This PR was merged into the 4.4 branch.
Discussion
----------
[Console] Default hidden question to 1 attempt for non-tty session
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#36565
| License | MIT
| Doc PR |
### Problem 1
`validateAttempts()` method repeats validation forever by default, until exception extending `RuntimeException` isn't thrown. This currently happens disregarding if user is in tty session where they can actually type input, or non-tty session. This presents a problem when user code throws custom exceptions for hidden questions -> loop doesn't stop. As far as I can tell this issue is in all Symfony versions, but it was uncovered only after we stopped marking interactive flag to false automatically ourselves. Actually, all 3 problems were already existing problems, just hidden until now.
### Problem 2
Infinite loop problem is related to hidden questions, but this one isn't. If validation fails, another attempt to read & validate happens. This means user will get two prompts: 2x same question with 2 different error messages. One error message coming from validator, second error message about inability to read input (because this loop repeats until this kind of error happens, so last output will always be this error). As an example, output in practice would look like following
```
What do you want to do:
>
[ERROR] Action must not be empty.
What do you want to do:
>
Aborted.
```
So even if loop stops, output is more than expected.
### Problem 3
This is purely cosmetic issue, but currently user gets `stty: stdin isn't a terminal` printed additionally when question helper tries to ask a hidden question without having tty. I have fixed this in same fashion as was already done for [getShell() method](ee7fc5544e/src/Symfony/Component/Console/Helper/QuestionHelper.php (L500)).
### More details
Well root of the first problem is that `\Symfony\Component\Console\Helper\QuestionHelper::getHiddenResponse` is inconsistent. In some cases it does throw `MissingInputException` (which extends `RuntimeException`), in others doesn't. This is because in others, `shell_exec` is used, which won't return `false` even in non-tty sessions. Initially I attempted to fix this and make them consistent by checking for empty result + `isTty` call, but during my testing I found that at least last, `bash -c` method returns `\n` as output both when passing empty input and when passing newline as input. This means we cannot differentiate with this technique when input is really empty, or at least I can't currently tell how, maybe someone does. I had also idea to use proc_open and check if `STDERR` cotains message about stdin not being a terminal, but I realized these functions might not be available. In future we should modernize this method to use less hacky techniques. Other solutions, eg. Inquirer.js or [hoa/console](https://github.com/hoaproject/Console/blob/master/Source/Readline/Readline.php) have much more elegant solutions. Anyway, since I encountered this issue and additionally this doesn't solve Problem 2, I stopped trying to fix this on this level.
### Alternative solution
Alternative solution to problem 1 and 3 would be to fallback to default in case of hidden questions when tty is missing. But this still doesn't solve problem 2 and I can't think about solution right now which would fix problem 2 separately. We also didn't really reach consensus if reading passwords via stdin is desired. I tried this in `Inquirer.js` and this library *does read password from stdin*
Commits
-------
ee7fc5544e [Console] Default hidden question to 1 attempt for non-tty session
This PR was squashed before being merged into the 3.4 branch.
Discussion
----------
[Filesystem] Handle paths on different drives
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| License | MIT
`makePathRelative` strips and ignores the drive letters given Windows paths on different drives, resulting in a relative path which does not resolve to the desired target.
This PR makes `makePathRelative` notice paths on different drives, and return the full (absolute) target path in case instead.
Commits
-------
00e727ae4e [Filesystem] Handle paths on different drives
This PR was merged into the 3.4 branch.
Discussion
----------
[WebProfiler] Do not add src-elem CSP directives if they do not exist
| Q | A
| ------------- | ---
| Branch? | 3.4, 4.4, 5.0
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#36643
| License | MIT
| Doc PR | n/a
In the latest 3.4.*, 4.4.* and 5.0.* branches the `script-src-elem` and `style-src-elem` directives are added to the Content-Security-Policy header if they don't exist by copying the `default-src`. This causes browsers to ignore the `script-src` and `style-src` directives which likely contain scripts and styles the developer wanted to allow.
As mentioned in the fixed ticket, we shouldn't be adding these directives if they don't exist because the browser will automatically fallback to `script-src` and `style-src` which we have already added `unsafe-inlen` and the `nonce-*` to.
This will need to be merged into 3.4, 4.4 and 5.0, but I was unsure which branch I am meant to base it off to start with. I've put it on 4.4 but can move it to another if required.
Commits
-------
d9c47087c9 [WebProfiler] Do not add src-elem CSP directives if they do not exist
This PR was merged into the 3.4 branch.
Discussion
----------
[DX] Show the ParseException message in all YAML file loaders
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
This PR synchronizes the exception message in the Routing, Validator and Translation YAML file loaders with the DependencyInjection YAML file loader behavior. Adding the ParseException message is a big DX gain because it highlights the problem directly instead of having to scroll down 7 previous exceptions.
I'm targetting 3.4 because DX can be considered as a bug fix AFAIK.
Commits
-------
fc6cf3d3c6 [DX] Show the ParseException message in YAML file loaders
This PR was squashed before being merged into the 3.4 branch.
Discussion
----------
[Yaml] fix parse error when unindented collections contain a comment
| Q | A
| ------------- | ---
| Branch? | 5.0
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#36558
| License | MIT
### Problem
The method `Parser::getNextEmbedBlock` did not determine the yaml-block correctly when there was a comment before the first unindented collection item. This was caused by the fact that the check for unindented collection items was done for the _first line of the block only_. So in case this first line is a comment, this check will result in _false_, while in fact the parser is in an unindented collection.
### Solution
In the solution I implemented the parser will check for comment lines as well. As long as the loop encounters a comment line, it will check (in the next iteration) whether the line is an unindented collection item. So this check will be done until all comments before the first uncommented item are parsed.
Commits
-------
58bb2c52ac [Yaml] fix parse error when unindented collections contain a comment
This PR was merged into the 4.4 branch.
Discussion
----------
Execute docker dependent tests with github actions
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | Fixes#36427
| License | MIT
| Doc PR | -
* redis, memcached, rabbitmq and vulcain dependent tests moved to the github action
* run on PHP 7.1 and 7.4 only
* use the `integration` group for all tests that depend on docker services
* do not exclude the `integration` group on Travis, but make sure tests that depend on docker services are skipped properly
[<img width="1222" alt="image" src="https://user-images.githubusercontent.com/190447/80806323-48339100-8bb2-11ea-95cd-5ce773c74ce6.png">](https://github.com/jakzal/symfony/runs/636461875?check_suite_focus=true)
Commits
-------
d710c1b654 Execute docker dependent tests with github actions
This PR was merged into the 3.4 branch.
Discussion
----------
[Validator] Skip validation when email is an empty object
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | <!-- prefix each issue number with "Fix #", if any -->
| License | MIT
| Doc PR | <!-- required for new features -->
When the value passed to the email validator is an empty object the validator is still called and will mark the value as invalid. The object should be skipped in this case, as it is also done in the `UrlValidator`
bfdbb244fe/src/Symfony/Component/Validator/Constraints/UrlValidator.php (L59-L62)
<!--
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 master.
-->
Commits
-------
de5d68ef2a Skip validation when email is an empty object
This PR was submitted for the master branch but it was merged into the 4.4 branch instead.
Discussion
----------
Update exception.html.php
| Q | A
| ------------- | ---
| Branch? | master for features / 3.4, 4.4 or 5.0 for bug fixes <!-- see below -->
| Bug fix? |no
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | NIL <!-- prefix each issue number with "Fix #", if any -->
| License | MIT
| Doc PR | symfony/symfony-docs#... <!-- required for new features -->
<!--
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 master.
-->
Removed the second use of `count` and replaced with the result of the first call to `count()`.
In other words: the use of `$last = count($exceptionAsArray) - 1;` is rewritten as `$last = $exceptionAsArrayCount - 1;` to use the result of the last count.
Commits
-------
1c9162d2ad Update exception.html.php
This PR was merged into the 4.4 branch.
Discussion
----------
[Inflector] Fix testPluralize() arguments names
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
This is minor but the testPluralize() method arguments names are inversed ($plural contains the singular, $singular contains the plural, etc.). I lost some time on this because I thought the test was wrong 😅 I guess it's better to have a test that is easier to understand.
Commits
-------
fb42f98315 [Inflector] Fix testPluralize() arguments names
This PR was merged into the 3.4 branch.
Discussion
----------
[3.4][Inflector] Improve testSingularize() argument name
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | https://github.com/symfony/symfony/pull/36639#discussion_r418192900
| License | MIT
| Doc PR | -
Follow up of https://github.com/symfony/symfony/pull/36639 on 3.4 to keep both tests methods synchronized.
Commits
-------
75405247be [3.4][Inflector] Improve testSingularize() argument name
This PR was merged into the 4.4 branch.
Discussion
----------
[Translation] Fix for translation:update command updating ICU messages
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#36459
| License | MIT
If `translation:update` command executed with option `--domain=messages` – it ignore `messages-intl-icu` file and just create new `messages`
Method `TranslationUpdateCommand::filterCatalogue()` on `MessageCatalogue::all()` method to get all messages for domain
But `MessageCatalogue::all()` method disredard `intl-icu` domains and simply merge all.
[Translation] added $strict parameter for MessageCatalogueInterface::all() to be able to get only defined domain messages
[FrameworkBundle] modified translation:update command to respect intl-icu domain
Commits
-------
567cee5f02 [Translation] Fix for translation:update command updating ICU messages
This PR was squashed before being merged into the 3.4 branch (closes#36627).
Discussion
----------
[Validator] fix lazy property usage.
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#36343
| License | MIT
| Doc PR |
This attempts to fix a large regression introduced in #36343, which broke recursing values returned from `getter` Constraints, because they are now wrapped in in a `LazyProperty`. The `LazyProperty` needs to be evaluated because some checks are done on the type of `$value`, i.e `is_array` etc... in `validateGenericNode`.
I'm concerned that the original PR didn't really add sufficient test coverage for the introduction of `LazyProperty`, and I'm not 100% sure that I've caught all the cases where the `instanceof` check are needed in this PR.
For the tests, I added the `@dataProvider getConstraintMethods` to every test that hit the problem area of code.
~~The only issue is that my fixed has broken the test introduced in #36343, `testGroupedMethodConstraintValidateInSequence`.~~
~~I think I need @HeahDude to help me work through this. Maybe there is a more simple solution, one that doesn't require doing `instanceof LazyPropery` checks in multiple places, because this feels very brittle.~~
EDIT: fixed that test.
Commits
-------
281861e788 [Validator] fix lazy property usage.
This PR was merged into the 3.4 branch.
Discussion
----------
Fix Form annotation
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? |no
| License | MIT
Symfony form component provide his own exception `OutOfBoundsException` which is implementing `ExceptionInterface`. Form are returning this custom `OutOfBoundsException`, but the phpDoc say it's an original `\OutOfBoundsException`, which is not extending the `ExceptionInterface`.
This can provide issue with static analysis tools.
Commits
-------
67b744929f Fix annotation
This PR was merged into the 4.4 branch.
Discussion
----------
[Form] provide a useful message when extension types don't match
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | Fix#36610
| License | MIT
| Doc PR |
Commits
-------
88d836643a provide a useful message when extension types don't match
Nicolas Grekas
Matthias Larisch
Grégoire Paris
Hugo Alliaume
Gocha Ossinkine
Matthias Derer
Fabien Potencier
Grégoire Pineau
Christian Flothmann
Christian Rishøj
Nathan Dench
Wouter Diesveld
Jakub Zalas
Olatunbosun Egberinde
Thomas Calvet
Jeroen Thora
Marko Kaznovac
Ben Davies
