This PR was merged into the 5.x branch.
Discussion
----------
[HttpClient] simplify retry mechanism around RetryStrategyInterface
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Replaces #38466
I feel like the mechanism behind RetryableHttpClient is too bloated for no pragmatic reasons.
I propose to merge RetryDeciderInterface and RetryBackoffInterface into one single RetryStrategyInterface.
Having two separate interfaces supports no real-world use case. The only implementations we provide are trivial, and they can be reused by extending the provided GenericRetryStrategy implementation if one really doesn't want to duplicate that.
The methods are also simplified by accepting an AsyncContext as argument. This makes the signatures shorter and allows accessing the "info" of the request (allowing to decide based on the IP of the host, etc).
/cc @jderusse
Commits
-------
544c3e8678 [HttpClient] simplify retry mechanism around RetryStrategyInterface
This PR was merged into the 5.1 branch.
Discussion
----------
[TwigBridge] Fix preload hint and remove "unlinked class class@anonymous" warning
| Q | A
| ------------- | ---
| Branch? | 5.1
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#38197
| License | MIT
| Doc PR | n/a
This should fix the preload warning mentioned in https://github.com/symfony/symfony/issues/38197 by applying the fix as suggested by @nicolas-grekas:
```
PHP Warning: Can't preload unlinked class class@anonymous: Unknown trait Symfony\\Contracts\\Translation\\TranslatorTrait in /var/www/html/vendor/symfony/twig-bridge/Extension/TranslationExtension.php on line 50
```
Commits
-------
0f167f6ed4 fix warning for preloading TranslatorTrait class
This PR was merged into the 5.x branch.
Discussion
----------
[Security] Making login link signature_properties option required
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | yes (for a 5.2 feature)
| New feature? | no
| Deprecations? | no
| Tickets | none
| License | MIT
| Doc PR | not needed
Hi!
My intention was always to force the user to set this option. Before this PR, you can simply leave this option off completely without a validation error. Thanks to Wouter for finding it.
Also: made some punctuation & capitalization consistent on info messages.
Cheers!
Commits
-------
f7bb954979 Making login link signature_properties option required
This PR was merged into the 5.x branch.
Discussion
----------
[Serializer] Enabled mapping configuration via attributes
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | N/A
| License | MIT
| Doc PR | TODO
Let's use attributes for the serializer mapping configuration!
```php
class MyEntity
{
#[Groups(['list', 'detail])]
#[SerializedName('some-property')]
public $someProperty;
#[Ignore]
public string $secret;
}
```
Commits
-------
cfb9986203 [Serializer] Enabled mapping configuration via attributes.
This PR was merged into the 5.x branch.
Discussion
----------
[Form] fix ViolationMapper was always generating a localized label for each FormType
| Q | A
| ------------- | ---
| Branch? | 5.x (fix new behavior from 5.2)
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | /
| License | MIT
| Doc PR | /
Follow-up of #38435, with branch changed
**Explanation of the issue**
In Symfony 5.2, the `{{ label }}` placeholder can be used in constraint messages (-> introduced in commit 0d9f44235c)
However, the way it was coded is introducing a small side effect: now, every time there is validation error, `ViolationMapper` will ask the Form Type its `label`, and if not `false`, it will try to translate it.
**Why it is important/why it causes a BC break**
Since by default `AbstractType` does not have any `label`, it also introduces a minor BC break.
I will explain it with an example: in a project I work on, we check we don't have any missing translation. Sometimes we have violation errors bound to form ; then current code will get Form `label`, which in `null` in form type classes (which is quite usual I believe), so it will generate one, and pass that one to translator. And we see a lot on erroneous missing translations.
**Proposed fix**
This fix moves all this logic into a `if`, so `ViolationMapper` call the translator component only if `{{ label }}` placeholder is used in constraint error message.
On top of fixing BC, it has the benefit of lowering the performance cost for every violation when the feature is not used.
I added a test, as I believe the behavior should be guaranteed from now on.
Commits
-------
aee5571a71 [Form] fix ViolationMapper was always generating a localized label for each FormType
This PR was merged into the 5.x branch.
Discussion
----------
[Mailer] Fix mailjet image embedding
Filename is not enough to embed the image through cid, ContentID field
has to be set with the cid identifier used in the HTMLPart
| Q | A
| ------------- | ---
| Branch? | 5.1 <!-- see below -->
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | none <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License | MIT
| Doc PR | not needed <!-- 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 5.x.
-->
Hello!
While switching from `Mandrill` to `Mailjet` I noticed that embedding images was not working properly, the `Content-ID` mime is not set properly without setting the `ContentID` key. With this change things will work as described in the [symfony documentation](https://symfony.com/doc/current/mailer.html#embedding-images)
[Mailjet reference](https://dev.mailjet.com/email/guides/send-api-v31/#send-with-attached-files)
Commits
-------
7ab772eeea [Mailer] Fix mailjet image embedding
This PR was merged into the 4.4 branch.
Discussion
----------
[HttpClient] fix reading the body after a ClientException
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
#38508 wasn't enough to fix the issue.
Commits
-------
0159a39709 [HttpClient] fix reading the body after a ClientException
This PR was submitted for the 5.x branch but it was merged into the 3.4 branch instead.
Discussion
----------
[Typography] Fix SerializerAwareInterface description
| Q | A
| ------------- | ---
| Branch? | 5.x<!-- see below -->
| Bug fix? | no
| New feature? | no
| Deprecations? |no
| Tickets | -
| License | MIT
Removed some unneeded comments in Serializer / Normalizer / Encoder directory
Commits
-------
48e6976ac8 [Typography] Remove unneeded description comments
* 5.1:
Disable the PhpUnit bridge when testing it
[PropertyInfo] Support for the mixed type.
Don't unset the inflate resource on close as it might still be needed
[HttpClient] Fix CurlHttpClient memory leak
[Form] Add Bosnian (bs) validators translation
[Form] Add missing Serbian (latn & cyrl) validators translation
[Cache] skip igbinary < 3.1.6
[Ldap] Bypass the use of `ldap_control_paged_result` on PHP >= 7.3
[Form] [Validator] added pt_BR translations
Estonian update
Fix no collection in extract default value
[PhpUnitBridge] fix running parallel tests with phpunit 9
[VarDumper] fix truncating big arrays
* 4.4:
Disable the PhpUnit bridge when testing it
[PropertyInfo] Support for the mixed type.
Don't unset the inflate resource on close as it might still be needed
[HttpClient] Fix CurlHttpClient memory leak
[Form] Add Bosnian (bs) validators translation
[Form] Add missing Serbian (latn & cyrl) validators translation
[Cache] skip igbinary < 3.1.6
[Ldap] Bypass the use of `ldap_control_paged_result` on PHP >= 7.3
[Form] [Validator] added pt_BR translations
Estonian update
[PhpUnitBridge] fix running parallel tests with phpunit 9
[VarDumper] fix truncating big arrays
* 3.4:
Disable the PhpUnit bridge when testing it
[PropertyInfo] Support for the mixed type.
[Form] Add Bosnian (bs) validators translation
[Form] Add missing Serbian (latn & cyrl) validators translation
[Form] [Validator] added pt_BR translations
Estonian update
[PhpUnitBridge] fix running parallel tests with phpunit 9
[VarDumper] fix truncating big arrays
This PR was merged into the 5.x branch.
Discussion
----------
[HttpClient] Fix nesteed stream in AsyncResponse
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | #38509
| License | MIT
| Doc PR | /
When streaming twice (streaming inside streaming) an AsycResponse the second stream will yield the LastChunk, but the first Stream won't have access to it and ends with an exception `A chunk passthru must yield an "isLast()" chunk`
This PR adds a state in AsyncRsponse to remember if the lastChunk has been yielded.
Reproducer:
```php
// a Simple Client that return an AsyncResponse
$client = new class(HttpClient::create()) implements HttpClientInterface {
use AsyncDecoratorTrait;
private $client;
public function __construct(HttpClientInterface $client)
{
$this->client = $client;
}
public function request(string $method, string $url, array $options = []): ResponseInterface
{
return new AsyncResponse($this->client, $method, $url, $options, null);
}
};
$response = $client->request('GET', 'https://httpbin.org/status/200');
foreach ($client->stream($response) as $chunk) { // will end in a FirstChunk <== bug
foreach ($client->stream($response) as $chunk) { // This will correctly handle the LastChunk
}
}
```
Commits
-------
8ba54c17be Fix nesteed stream in AsyncResponse
This PR was merged into the 4.4 branch.
Discussion
----------
[HttpClient] Content doesn't get decoded when fetched from an exception
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
My previous PR #38493 created a new bug, the content doesn't get decoded when fetched from an exception because the inflate resource gets unset:
```php
<?php
use Symfony\Component\HttpClient\CurlHttpClient;
use Symfony\Contracts\HttpClient\Exception\ClientExceptionInterface;
include __DIR__.'/symfony/vendor/autoload.php';
$client = new CurlHttpClient();
try {
$client->request('GET', 'http://example.com/404');
} catch (ClientExceptionInterface $exception) {
echo $exception->getResponse()->getContent(false);
}
```
I've removed the part where the resource gets unset since it still might be used at some point.
The test is implementation independent so I believe it should go in contracts, please correct me if I wrong. Also, I was unable to find a way to do the test without adding a new endpoint this time, any suggestions would be appreciated.
Commits
-------
8fa4f85013 Don't unset the inflate resource on close as it might still be needed
This PR was squashed before being merged into the 5.x branch.
Discussion
----------
[Validator] Upgraded constraints to enable named arguments and attributes
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | #38096
| License | MIT
| Doc PR | TODO with symfony/symfony-docs#14305
This PR enables all remaining atomic (!= composite) constraints to be used as attributes.
The only exception is `UniqueEntity` from Doctrine bridge because we don't have a Doctrine ORM release yet that supports PHP 8. So I could migrate that one as well, but I cannot really test it.
Commits
-------
fb99eb2052 [Validator] Upgraded constraints to enable named arguments and attributes
This PR was merged into the 5.x branch.
Discussion
----------
[Security][Login Link] Allow null and DateTime objects to be used as signatureProperties
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Returning `DateTime` objects seems like a common use-case to automatically expire all login links when one is used or to only allow the login link to be used once.
**Before**
```php
class User
{
private ?\DateTime $lastAuthenticatedAt = null;
// ...
public function getLastAuthenticatedAtString(): string
{
return null === $this->lastAuthenticatedAt ? '' : $this->lastAuthenticatedAt->format('c');
}
}
```
```yaml
security:
firewalls:
main:
login_link:
# ...
signature_properties: ['lastAuthenticatedAtString']
````
**After**
```php
class User
{
private ?\DateTime $lastAuthenticatedAt = null;
// ...
public function getLastAuthenticatedAt(): ?\DateTime
{
return $this->lastAuthenticatedAt;
}
}
```
```yaml
security:
firewalls:
main:
login_link:
# ...
signature_properties: ['lastAuthenticatedAt']
````
---
The disadvantage of this patch is that there needs to be some boundary as to which objects we want to support casting to a scalar, but I'm convinced that `DateTime` objects will commonly be used as signature properties.
cc @weaverryan
Commits
-------
0f947b2e84 Allow null and DateTime objects to be used as signatureProperties
This PR was merged into the 5.x branch.
Discussion
----------
[Security] Add error message when using LoginLinkHandler outside a firewall
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Add a more friendly error message when autowiring `LoginLinkHanderInterface` in a route outside the firewall. Current error was `Call to a member function getName() on null`.
Commits
-------
f807b5fc15 Add error message when using LoginLinkHandler outside a firewall
This PR was merged into the 5.x branch.
Discussion
----------
[Bug] Fix RateLimiter framework configuration
| Q | A
| ------------- | ---
| Branch? | 5.x for features <!-- see below -->
| 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 #", no need to create an issue if none exist, explain below instead -->
| License | MIT
| Doc PR | -
Small mistake in the rate limiter configuration, instead of unsetting the `storage_service` option the never existing `storage` option was unset, resulting into an application error when trying to use a Limiter in your application.
The exception was:
```
Uncaught PHP Exception: The option "storage_service" does not exist. Defined options are: "id", "interval", "limit", "rate", "strategy"."
```
This was introduced in #38204, so a highlight for @wouterj to check this :)
Commits
-------
b360320890 [Bug] Fix RateLimiter framework configuration
