This PR was merged into the 3.3-dev branch.
Discussion
----------
[FrameworkBundle] Add new "controller.service_arguments" tag to inject services into actions
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | no
| Tests pass? | (no test yet)
| Fixed tickets | -
| License | MIT
| Doc PR | -
Talking with @simensen and @weaverryan, we wondered if we could leverage the `ArgumentResolver` mechanism to make it inject services on demand, using e.g. autowiring.
```php
class PostController
{
public function indexAction(Request $request, PostRepository $postRepository)
{
// PostRepository comes from the container
$postRepository->findAll(); // ...
}
}
```
This PR achieves that, using a new "controller.service_arguments" tag. Typically:
```yaml
services:
AppBundle\Controller\PostController:
autowire: true
tags:
- name: controller.service_arguments
```
It also supports with explicit wiring (thus doesn't necessarily require autowiring if you don't want to use it):
```yaml
services:
AppBundle\Controller\PostController:
tags:
- name: controller.service_arguments
action: fooAction
argument: logger
id: my_logger
```
~~The attached diff is bigger than strictly required for now, until #21770 is merged.~~
Todo:
- [x] rebase on top of #21770 when merged
- [x] add tests
- [x] add cleaning pass to remove empty service locators
Commits
-------
9c6e672780 [FrameworkBundle] Add new "controller.service_arguments" tag to inject services into actions
This PR was merged into the 3.3-dev branch.
Discussion
----------
[lock] Rename Quorum into Strategy
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yes (not consistent naming)
| New feature? | no
| BC breaks? | yes (but version 3.4 not yet released)
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | none
| License | MIT
| Doc PR |
The term `Quorum` in Interface is confusing an not consistent with the Symfony project.
This PR switch to naming `Strategy\StrategyInterface` (like in adapter i `Cache` and `Ldap` component)
Commits
-------
1e9671b993 Rename Quorum into Strategy
This PR was merged into the 3.3-dev branch.
Discussion
----------
[Lock] Don't call blindly the redis client
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets |
| License | MIT
| Doc PR |
Actual code rely on controls on the constructor. This PR add an assertion to avoid futur bugs
Commits
-------
e4db018b6d Don't call blindly the redis client
* 3.2:
Fixed pathinfo calculation for requests starting with a question mark.
[HttpFoundation] Fix missing handling of for/host/proto info from "Forwarded" header
[Validator] Add object handling of invalid constraints in Composite
[WebProfilerBundle] Remove uneeded directive in the form collector styles
removed usage of $that
HttpCache: New test for revalidating responses with an expired TTL
[Serializer] [XML] Ignore Process Instruction
[Security] simplify the SwitchUserListenerTest
Revert "bug #21841 [Console] Do not squash input changes made from console.command event (chalasr)"
[HttpFoundation] Fix Request::getHost() when having several hosts in X_FORWARDED_HOST
This PR was merged into the 3.3-dev branch.
Discussion
----------
[Security][SecurityBundle] Enhance automatic logout url generation
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | yes
| Tests pass? | yes
| Fixed tickets | N/A
| License | MIT
| Doc PR | N/A
This should help whenever:
- [the token does not implement the `getProviderKey` method](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Security/Http/Logout/LogoutUrlGenerator.php#L89-L99)
- you've got multiple firewalls sharing a same context but a logout listener only define on one of them.
##### Behavior:
> When not providing the firewall key:
>
>- Try to find the key from the token (unless it's an anonymous token)
>- If found, try to get the listener from the key. If the listener is found, stop there.
>- Try from the injected firewall key. If the listener is found, stop there.
>- Try from the injected firewall context. If the listener is found, stop there.
>
>The behavior remains unchanged when providing explicitly the firewall key. No fallback.
Commits
-------
5b7fe852aa [Security][SecurityBundle] Enhance automatic logout url generation
This PR was squashed before being merged into the 3.3-dev branch (closes#22112).
Discussion
----------
Minor PR fixes
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yes-ish
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #... <!-- #-prefixed issue number(s), if any -->
| License | MIT
| Doc PR | symfony/symfony-docs#... <!--highly recommended for new features-->
cc @fabpot my bad :)
Commits
-------
0728fb91b8 typo
036b0414d6 Minor PR fixes
This PR was merged into the 3.3-dev branch.
Discussion
----------
[WebProfilerBundle] Improved cookie traffic
| Q | A
| ------------- | ---
| Branch? | "master"
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | comma-separated list of tickets fixed by the PR, if any
| License | MIT
| Doc PR | reference to the documentation PR, if any
Relates to #20569 in terms of getting _all_ the cookies.
Commits
-------
171c6d100e [WebProfilerBundle] Improved cookie traffic
This PR was squashed before being merged into the 3.3-dev branch (closes#19887).
Discussion
----------
Sort alternatives alphabetically when a command is not found
| Q | A |
| --- | --- |
| Branch? | master |
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #10893 |
| License | MIT |
| Doc PR | - |
Commits
-------
ba6c9464ea Sort commands like a human would do
f04b1bd72f Sort alternatives alphabetically when a command is not found
This PR was merged into the 3.3-dev branch.
Discussion
----------
[Security] json auth listener should not produce a 500 response on bad request format
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | N/A
| License | MIT
| Doc PR | N/A
To me, it looks wrong to simply throw a `BadCredentialsException` in the wild, which produces a 500 (unless an entrypoint handles it, which you probably don't have on a json login firewall). There isn't any server error, the client request originated the error due to a wrong format.
Instead, the listener should give a chance to the failure handler to resolve it, and return a proper 4XX response. (BTW, the `UsernamePasswordFormAuthenticationListener` also throws a similar `BadCredentialsException` on a too long submitted username, which is caught and forwarded to the failure handler)
Better diff: https://github.com/symfony/symfony/pull/22034/files?w=1
BTW, should we have another exception type like `BadCredentialsFormatException` or whatever in order to distinct a proper `BadCredentialsException` from a format issue in a failure listener?
Commits
-------
cb175a41c3 [Security] json auth listener should not produce a 500 response on bad request format
This PR was merged into the 2.7 branch.
Discussion
----------
[Security] simplify the SwitchUserListenerTest
| Q | A
| ------------- | ---
| Branch? | 2.7
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets |
| License | MIT
| Doc PR |
While working on #22048 I noticed that the `SwitchUserListenerTest` was more complicated than necessary by mocking a lot of stuff that didn't need to be mocked.
Commits
-------
923bbdbf9f [Security] simplify the SwitchUserListenerTest
This PR was squashed before being merged into the 3.3-dev branch (closes#20885).
Discussion
----------
[Console] Option to disable stty
| Q | A
| ------------- | ---
| Branch? |
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | no
| Tests pass? |
| Fixed tickets |
| License | MIT
| Doc PR | reference to the documentation PR, if any
Shall fix problems in Windows based environments if e.g. git is installed and stty is therefore found but writes only cryptic rubbish into the cmd. In the case of console questions it is also possible that input can't be read properly by console component.
Commits
-------
a189a6c52e [Console] Option to disable stty
* 2.8:
[HttpFoundation] Fix missing handling of for/host/proto info from "Forwarded" header
[Validator] Add object handling of invalid constraints in Composite
[WebProfilerBundle] Remove uneeded directive in the form collector styles
Revert "bug #21841 [Console] Do not squash input changes made from console.command event (chalasr)"
[HttpFoundation] Fix Request::getHost() when having several hosts in X_FORWARDED_HOST
This PR was merged into the 2.8 branch.
Discussion
----------
[HttpFoundation] Fix missing handling of for/host/proto info from "Forwarded" header
| Q | A
| ------------- | ---
| Branch? | 2.8
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
We're missing handling of for/host/proto info embedded in the `Forwarded` header, as eg in:
`Forwarded: for=1.1.1.1:443, host=foo.example.com:1234, proto=https, for=2.2.2.2, host=real.example.com:8080`
Commits
-------
04caacb757 [HttpFoundation] Fix missing handling of for/host/proto info from "Forwarded" header
This PR was squashed before being merged into the 2.7 branch (closes#21968).
Discussion
----------
Fixed pathinfo calculation for requests starting with a question mark.
| Q | A
| ------------- | ---
| Branch? | 2.7
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #21967
| License | MIT
| Doc PR |
With improper `strpos` result check calculated pathinfo for requests starting with '?' equals to request itself.
Correct pathinfo for those requests should be '/'.
Commits
-------
43297b45de Fixed pathinfo calculation for requests starting with a question mark.
This PR was merged into the 2.8 branch.
Discussion
----------
Revert "bug #21841 [Console] Do not squash input changes made from console.command event (chalasr)"
| Q | A
| ------------- | ---
| Branch? | 2.8
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #21953, https://github.com/symfony/symfony/issues/22050
| License | MIT
| Doc PR | n/a
A bit frustrated to revert this change since the BC break report lacks of information, making us unable to reproduce nor to look at improving the situation.
I'm going to re-propose this on master, covering the BC break that is identified, fixed and tested using the changes made in #21953. That will let the choice for the reporter to upgrade using the 1 required LOC.
Commits
-------
5af47c40dc Revert "bug #21841 [Console] Do not squash input changes made from console.command event (chalasr)"
* 2.7:
[Validator] Add object handling of invalid constraints in Composite
[HttpFoundation] Fix Request::getHost() when having several hosts in X_FORWARDED_HOST
This PR was merged into the 2.7 branch.
Discussion
----------
[HttpFoundation] Fix Request::getHost() when having several hosts in X_FORWARDED_HOST
| Q | A
| ------------- | ---
| Branch? | 2.7
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
The first "host" in the list provided by `X_FORWARDED_HOST` should be the one, not the last.
Already the case for "port" and "scheme".
Commits
-------
9a2b2de64f [HttpFoundation] Fix Request::getHost() when having several hosts in X_FORWARDED_HOST
This PR was squashed before being merged into the 3.3-dev branch (closes#21926).
Discussion
----------
[Routing] Optimised dumped matcher
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
TL;DR: I've optimised the PhpMatcherDumper output for a <del>60x</del> 4.4x performance improvement on a collection of ~800 routes by inducing cyclomatic complexity.
[EDIT] The 60x performance boost was only visible when profiling with blackfire, which is quite possibly a result of the cost of profiling playing a part. After doing some more profiling the realistic benefit of the optimisation is more likely to be in the ranges is 1.3x to 4.4x.
After the previous optimisation I began looking at how the PrefixCollection was adding its performance boost. I spotted another way to do this, which has the same theory behind it (excluding groups based on prefixes). The current implementation only groups when one prefix resides in the other. In this new implementation I've created a way to detect common prefixes, which allows for much more efficient grouping. Every time a route is added to the group it'll either merge into an existing group, merge into a new group with a route that has a common prefix, or merge into a new group with an existing group that has a common prefix.
However, when a parameter is present grouping must only be done AFTER that route, this case is accounted for. In all other cases, where there's no collision routes can be grouped freely because if a group was matched other groups wouldn't have matched.
After all the groups are created the groups are optimised. Groups with fewer than 3 children are inlined into the parent group. This is because a group with 2 children would potentially result in 3 prefix checks while if they are inlines it's 2 checks.
Like with the previous optimisation I've profiled this using blackfire. But the match function didn't show up anymore. I've added `usleep` calls in the dumped matcher during profiling, which made it show up again. I've verified with @simensen that this is because the wall time of the function was too small for it to be of any interest. When it DID get detected, because of more tasks running, it would show up with around 250 nanoseconds. In comparison, the previous speed improvement brought the wall time down from 7ms to ~2.5ms on a set of ~800 routes.
Because of the altered grouping behaviour I've not modified the PrefixCollection but I've created a new StaticPrefixCollection and updated the PhpMatcherDumper to use that instead.
Commits
-------
449b6912dc [Routing] Optimised dumped matcher
This PR was merged into the 3.3-dev branch.
Discussion
----------
[Yaml] ParseException: pcre.backtrack_limit reached
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | no
| Fixed tickets | -
| License | MIT
| Doc PR | -
while merging 3.2 into master, I noticed that `testCanParseVeryLongValue` is triggering this error on master, due to this regexp that we added for handling yaml tags. This regexp needs to be fixed so that we can merge the test case.
ping @GuilhemN
Commits
-------
f0256f1aa5 [Yaml] Fix pcre.backtrack_limit reached
This PR was merged into the 3.3-dev branch.
Discussion
----------
[DI] Deprecate Container::isFrozen and introduce isCompiled
| Q | A |
| --- | --- |
| Branch? | "master" |
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | yes |
| Tests pass? | yes |
| Fixed tickets | comma-separated list of tickets fixed by the PR, if any |
| License | MIT |
| Doc PR | reference to the documentation PR, if any |
This deprecates the concept of freezing a container, implied by `Container::isFrozen`. However, freezing happens due compilation (`Container::compile`). So having just `isCompiled` instead seems more intuitive, and plays along well with `ContainerBuilder`.
Before/After;
- `Container::isFrozen`
- Checks if the parameter bag is frozen, but is deprecated in 3.2
- In 4.0 this methods does not exists and can be replaced with `getParameterBag() instanceof FrozenParameterBag` _or_ `isCompiled()`. Depending on what you want (to clarify; the behavior is different when passing a frozen bag to the constructor)
- `Container::isCompiled`
- Truly checks if `compile()` has ran, and is a new feature
- `ContainerBuilder::merge` etc.
- Now uses `isCompiled` instead of `isFrozen`, ie. we allow for it till compilation regarding the state of the paramater bag
Commits
-------
6abd312800 [DI] Deprecate Container::isFrozen and introduce isCompiled
This PR was merged into the 3.3-dev branch.
Discussion
----------
[Console] Exclude empty namespaces in text descriptor
| Q | A |
| --- | --- |
| Branch? | "master" |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | comma-separated list of tickets fixed by the PR, if any |
| License | MIT |
| Doc PR | reference to the documentation PR, if any |
Before:
```
$ bin/console
doctrine
doctrine:mapping:convert [orm:convert:mapping] Convert mapping information between supported formats.
orm <----
router
router:match Helps debug routes by simulating a path info match
$ bin/console list orm
[Symfony\Component\Debug\Exception\ContextErrorException]
Warning: max(): Array must contain at least one element
$ bin/console list generate
Available commands for the "generate" namespace:
generate:bundle Generates a bundle
generate:command Generates a console command
generate:controller Generates a controller
```
After:
```
$ bin/console
doctrine
doctrine:mapping:convert [orm:convert:mapping] Convert mapping information between supported formats.
router
router:match Helps debug routes by simulating a path info match
$ bin/console list orm
Available commands for the "orm" namespace:
orm:convert:mapping Convert mapping information between supported formats.
$ bin/console list generate
Available commands for the "generate" namespace:
generate:bundle Generates a bundle
generate:command Generates a console command
generate:controller Generates a controller
generate:doctrine:crud Generates a CRUD based on a Doctrine entity
generate:doctrine:entities Generates entity classes and method stubs from your mapping information
generate:doctrine:entity Generates a new Doctrine entity inside a bundle
generate:doctrine:form Generates a form type class based on a Doctrine entity
```
Overrules #19776 but also includes other fixes related to aliases that popped up when writing tests 👍
Commits
-------
d5a7608036 [Console] Exclude empty namespaces in text descriptor
This PR was squashed before being merged into the 3.3-dev branch (closes#21093).
Discussion
----------
[Lock] Create a lock component
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | no
| Tests pass? | they will
| Fixed tickets | #20382
| License | MIT
| Doc PR | symfony/symfony-docs#7364
This PR aim to add a new component Lock going further than the FileSystem\LockHandler by allowing remote backend (like Redis, memcache, etc)
Inspired by
## Usage
The simplest way to use lock is to inject an instance of a Lock in your service
```php
class MyService
{
private $lock;
public function __construct(LockInterface $lock)
{
$this->lock = $lock;
}
public function run()
{
$this->lock->acquire(true);
// If I'm here, no exception had been raised. Lock is acquired
try {
// do my job
} finally {
$this->lock->release();
}
}
}
```
Configured with something like
```yaml
services:
app.my_service:
class: AppBundle\MyService
arguments:
- app.lock.my_service
app.lock.my_service:
class: Symfony\Component\Lock\Lock
factory: ['@locker', createLock]
arguments: ['my_service']
```
If you need to lock serveral resource on runtime, wou'll nneed to inject the LockFactory.
```php
class MyService
{
private $lockFactory;
public function __construct(LockFactoryInterface $lockFactory)
{
$this->lockFactory = $lockFactory;
}
public function run()
{
foreach ($this->items as $item) {
$lock = $this->lockFactory->createLock((string) $item);
try {
$lock->acquire();
} catch (LockConflictedException $e) {
continue;
}
// When I'm here, no exception had been, raised. Lock is acquired
try {
// do my job
} finally {
$lock->release();
}
}
}
}
```
Configured with something like
```yaml
services:
app.my_service:
class: AppBundle\MyService
arguments:
- '@locker'
```
This component allow you to refresh an expirable lock.
This is usefull, if you run a long operation split in several small parts.
If you lock with a ttl for the overall operatoin time and your process crash, the lock will block everybody for the defined TTL.
But thank to the refresh method, you're able to lock for a small TTL, and refresh it between each parts.
```php
class MyService
{
private $lock;
public function __construct(LockInterface $lock)
{
$this->lock = $lock;
}
public function run()
{
$this->lock->acquire(true);
try {
do {
$finished = $this->performLongTask();
// Increase the expire date by 300 more seconds
$this->lock->refresh();
} while (!$finished)
// do my job
} finally {
$this->lock->release();
}
}
}
```
## Naming anc implementation choise
```
$lock->acquire()
vs
$lock->lock()
```
Choose to use acquire, because this component is full of `lock` Symfony\Component\Lock\Lock::Lock` raised a E_TOO_MANY_LOCK in my head.
```
$lock->acquire(false);
$lock->acquire(true);
vs
$lock->aquire()
$lock->waitAndAquire()
```
Not a big fan of flag feature and 2. But I choose to use the blocking flag to offer a simple (and common usecase) implementation
```
$lock = $factory->createLock($key);
$lock->acquire();
vs
$lock->aquire($key)
```
I choose to a the pool of locks implementation. It allow the user to create 2 instances and use cross lock even in the same process.
```
interface LockInterface
final class Lock implements LockInterface
vs
final class Lock
```
I choose to use a Interface even if there is only one implementaiton to offer an extension point here
# TODO
## In this PR
* [x] tests
* [x] add logs
* [x] offer several redis connectors
* [x] try other store implementation to validate the architecture/interface
## In other PR
* documentation
* add configuration in framework bundle
* add stop watch in the debug bar
* improve the combined store (takes the drift into account and elapsed time between each store)
* implement other stores (memcache, ...)
* use this component in session manipulation (fixes#4976)
Commits
-------
018e0fc330 [Lock] Create a lock component
