This PR was merged into the master branch.
Commits
-------
67d7423 Remove use of deprecated HttpKernel LoggerInterface
dca4528 [HttpKernel] Extend psr/log's NullLogger class
1e5a890 [Monolog] Mark old non-PSR3 methods as deprecated
91a86f8 [HttpKernel][Monolog] Add PSR-3 support to the LoggerInterface
Discussion
----------
[HttpKernel][MonologBridge] PSR-3 support
This enables PSR-3 support and monolog 1.3+. The first commit is the main part. The rest deals with deprecation of short-hand methods (warn/err/crit/emerg) that are fully expanded in PSR-3 (warning/error/critical/emergency).
The downside of deprecating them is that for bundles it's a bit harder to support older and newer versions. If that is too much of a hassle you can drop that for now and cherry pick the first commit.
The upside is that it forces people to move towards PSR-3 compatible stuff, which means eventually we could completely drop the LoggerInterface from the framework. In any case I think the documentation should only mention the `Psr\Log\LoggerInterface` and people should start hinting against that. The change should be done in core as well I suppose.
Anyway I wanted to throw this out there as it is to get feedback.
---------------------------------------------------------------------------
by stof at 2013-01-09T09:15:15Z
@Seldaek I also think you should change the typehint to use the PSR LoggerInterface in all classes using the logger
---------------------------------------------------------------------------
by Seldaek at 2013-01-09T09:54:55Z
OK updated according to all the feedback. I tested it in an app and it still seems to work so there shouldn't be any major issues.
---------------------------------------------------------------------------
by Seldaek at 2013-01-09T09:59:55Z
@fabpot if you merge please merge also the bundle PR, otherwise it won't be possible to update without conflict.
---------------------------------------------------------------------------
by frosas at 2013-01-10T14:59:20Z
I'm trying to understand why a `composer update` of a Symfony 2.1.* resulted in a fatal error. Shouldn't a stable version don't break like this?
As @olaurendeau points, why Symfony depends 1.* instead of 1.2.*? Or why Monolog 1.3 breaks its public interface (EDIT: I'm not sure about it)? Or why isn't this PR being merged (into branch 2.1) at the same time Monolog 1.3 is released?
Please, understand I'm not looking for who to blame, it's just I want to know if this situation is unexpected or if otherwise a `composer update` on a stable branch is not as innocent as it seems.
---------------------------------------------------------------------------
by stof at 2013-01-10T15:06:51Z
@frosas it cannot be merged into 2.1 as it is a BC break. The 2.1 branch has been updated to forbid Monolog 1.3 already
---------------------------------------------------------------------------
by Seldaek at 2013-01-10T15:11:58Z
@frosas you can blame me for releasing as 1.3.0 and not 2.0, but technically for monolog this isn't really a BC break, I just added an interface. The problem is due to the way it's used in symfony, it ended up as a fatal error. In any case the situation is now sorted out I think.
---------------------------------------------------------------------------
by frosas at 2013-01-10T15:26:43Z
@stof now I see this `>=1.0,<1.3-dev` change in the 2.1 branch. Now, shouldn't a new (2.1.7) version be released for all of us not in the dev minimum-stability?
@Seldaek then do you see feasible to rely only in X.Y.* versions to avoid this kind of errors?
---------------------------------------------------------------------------
by Seldaek at 2013-01-10T15:45:22Z
@frosas relying on X.Y.* is painful because you always need to wait until someone updates the constraint to get the new version. Of course using ~1.3 like in this PR means if I fuck up and break BC people will update to it, but that's a less likely occurrence than the alternative I think, so I would rather not use X.Y.*
---------------------------------------------------------------------------
by frosas at 2013-01-10T15:50:50Z
@Seldaek you are right about this, but I was thinking more in changing it only for the stable versions. EDIT: I mean, how often do you need a new feature in a branch you only apply fixes to?
---------------------------------------------------------------------------
by stof at 2013-01-10T15:57:32Z
@frosas Monolog and Symfony have separate release cycles. Foorcing Symfony users to use an old version of Monolog until they update to a new version of Symfony whereas the newer Monolog is compatible is a bad idea. Thus, as Monolog keeps BC, it does not maintain bugfix releases for all older versions (just like Twig does too). So it would also forbid you to get the fixes done in newer Monolog versions.
The incompatibility between Symfony 2.1 LoggerInterface and PSR-3 (whereas they expect exactly the same behavior and signature for methods with the same name) is unfortunate and is the reason why we get some issues here.
---------------------------------------------------------------------------
by frosas at 2013-01-10T16:21:06Z
@stof I appreciate you prefer to allow newer versions at the price of having to be constantly monitoring its changes to avoid breaks.
Another similar but safer strategy would be to stick to X.Y.* versions and upgrade to X.Y+1.* once the new version integration is tested, but I understand this is discutible in projects as close to Symfony as Monolog.
Returning to the issue, what do you say to release this 2.1.7 version? Or is it only me who is having issues here?
---------------------------------------------------------------------------
by stof at 2013-01-10T16:26:20Z
@frosas a minor release should not break BC when following smeantic versionning (Symfony warned about the fact it is not strictly followed for the first releases of 2.x). But as far as monolog is concerned, 1.3 is BC with 1.2.
---------------------------------------------------------------------------
by Seldaek at 2013-01-10T16:49:55Z
@frosas sorry I didn't get you still had the problem. I tagged a 2.1.7 of monologbundle which hopefully fixes your issue.
This PR was merged into the master branch.
Commits
-------
0b75f67 Security component Polish message translations
Discussion
----------
[Security] Polish message translations
Security component messages translated from English to Polish.
This PR was merged into the master branch.
Commits
-------
c06e627 Fixed some typos
Discussion
----------
FIxed some typos in the Spainsh translation of the Security component messages
Hi,
In order to show the most clear and less _"robotic"_ messages in Spanish, I've fixed some typos and some incorrect translations in the Spanish translation file of the Security component.
Christian.
This PR was merged into the master branch.
Commits
-------
e84a8d0 fixed some small issues with grammar and used terminology
Discussion
----------
fixed some small issues with grammar and used terminology
This PR was merged into the master branch.
Commits
-------
d5825aa slovenian translations of security component added
Discussion
----------
Slovenian translations of security component added
This PR was merged into the master branch.
Commits
-------
d6f972b Created Slovak translation to Security
Discussion
----------
[security][tranlation] Created Slovak translation to Security
This PR was merged into the master branch.
Commits
-------
6ae8ca8 Update src/Symfony/Component/Security/Resources/translations/security.es.xlf
Discussion
----------
Update src/Symfony/Component/Security/Resources/translations/security.es...
....xlf
This PR was merged into the master branch.
Commits
-------
9b8428d [security][tranlation]Fixed spanish translation
Discussion
----------
[security][tranlation]Fixed spanish translation
This PR was merged into the master branch.
Commits
-------
0b5177b added italian translations
Discussion
----------
added italian translations for the security component
---------------------------------------------------------------------------
by matteosister at 2013-01-10T14:22:54Z
not sure if the file should be named **security.it_IT.xlf** or **security.it.xlf**
---------------------------------------------------------------------------
by fabpot at 2013-01-10T14:25:49Z
security.it.xlf
---------------------------------------------------------------------------
by matteosister at 2013-01-10T14:31:14Z
ok, renamed the files, and squashed the pr to a single commit!
Thanks @fabpot!
This PR was squashed before being merged into the master branch (closes#6660).
Commits
-------
45c8682 [Security][Translation]Created fr translation for Security
Discussion
----------
[Security][Translation]Created fr translation for Security
adds french translations for the security component
This PR was merged into the master branch.
Commits
-------
c3a6659 [Security] added Hungarian translations for exception messages
Discussion
----------
[Security] added Hungarian translations for exception messages
This PR was merged into the master branch.
Commits
-------
2f51961 Created de translation for Security
Discussion
----------
[Security][Translation]Created de translation for Security
This PR was merged into the master branch.
Commits
-------
e35998f Translated to Romanian
Discussion
----------
[Security] Translated to Romanian
Added Romanian version for Security messages.
Thanks!
This PR was merged into the master branch.
Commits
-------
73db84f [Security] Move translations file to 'security' domain
324703a [Security] Switch to English messages as message keys
aa74769 [Security] Fix CS + unreachable code
2d7a7ba [Security] Fix `AuthenticationException` serialization
50d5724 [Security] Introduced `UsernameNotFoundException#get/setUsername`
39da27a [Security] Removed `get/setExtraInformation`, added `get/set(Token|User)`
837ae15 [Security] Add note about changed constructor to changelog
d6c57cf [FrameworkBundle] Register security exception translations
d7129b9 [Security] Fix exception constructors called in `UserChecker`
0038fbb [Security] Add initial translations for AccountStatusException childs
50e2cfc [Security] Add custom `getMessageKey` AccountStatusException childs
1147977 [Security] Fix InsufficientAuthenticationException constructor calls
79430b8 [Security] Fix AuthenticationServiceException constructor calls
42cced4 [Security] Fix AuthenticationException constructor calls
963a1d7 [Security] Add initial translations for the exceptions
ed6eed4 [Security] Add `getMessageKey` and `getMessageData` to auth exceptions
694c47c [Security] Change signature of `AuthenticationException` to match `\Exception`
Discussion
----------
[2.2][Security] AuthenticationException enhancements
Bug fix: semi
Feature addition: yes
Backwards compatibility break: yes
Symfony2 tests pass: [](http://travis-ci.org/asm89/symfony)
Fixes the following tickets: #837
License of the code: MIT
This PR adds the functionality discussed in #837 and changes the constructor of the `AuthenticationException` to match that of `\Exception`. This PR will allow developers to show a translated (save) authentication exception message to the user. :)
*Todo:*
- Add some functional test to check that the exceptions can indeed be translated?
- Get feedback on the current English messages
---------------------------------------------------------------------------
by asm89 at 2012-07-15T14:04:11Z
ping @schmittjoh
---------------------------------------------------------------------------
by schmittjoh at 2012-07-15T14:57:32Z
Looks good to me.
While you are at the exceptions, I think we can also get rid of the "extra information" thing and replace it by explicit getters/setters. Mostly that will mean adding set/getToken, set/getUser, set/getUsername. Bundles might add custom exceptions which have other data. This will make it a bit more useful and predictable.
---------------------------------------------------------------------------
by asm89 at 2012-07-15T15:40:45Z
@schmittjoh I removed the `get/setExtraInformation` and added the more explicit getters/setters as you suggested.
---------------------------------------------------------------------------
by asm89 at 2012-07-15T19:33:15Z
@fabpot Did you reschedule this for 2.2? Why? It was originally a 2.1 ticket. I think it is an important one because at the moment there is no reliable way to show users the cause of an `AuthenticationException` without the threat of exposing sensitive information. This issue has been around for a while, see the original issue this PR refers to, or for example [this TODO comment in FOSUB](https://github.com/FriendsOfSymfony/FOSUserBundle/blob/master/Controller/SecurityController.php#L37).
The PR itself is ready to merge now. My only question that remains is about whether the actual translations should be functional tested?
---------------------------------------------------------------------------
by fabpot at 2012-07-15T19:43:19Z
We need to stop at some point. If not, we never release anything. beta3 was scheduled for today and I don't plan any other one before the first RC and I won't have time to review this PR next week. So, if you, @schmittjoh, @vicb, @stof, and a few other core devs "validate" this PR, I might consider merging it before 2.1.
---------------------------------------------------------------------------
by asm89 at 2012-07-15T19:46:09Z
@fabpot I totally agree with your point of view. I just have been trying to pickup some security issues that were still open. :)
---------------------------------------------------------------------------
by stof at 2012-07-15T19:50:29Z
This looks good to me
---------------------------------------------------------------------------
by asm89 at 2012-08-12T09:06:24Z
Since the beta period is over I assume the window was missed to get this security related PR in 2.1. If I have feedback from @fabpot I'll still try to make it mergeable asap though.
---------------------------------------------------------------------------
by fabpot at 2012-08-13T10:10:32Z
@asm89 This would indeed be considered for merging in 2.2.
---------------------------------------------------------------------------
by Antek88 at 2012-10-03T10:30:46Z
+1
---------------------------------------------------------------------------
by stof at 2012-10-04T21:27:15Z
@asm89 could you rebase this PR ? It conflicts with master
---------------------------------------------------------------------------
by fabpot at 2012-10-05T17:16:44Z
What's the status of this PR? @asm89 Have you taken all the feedback into account?
---------------------------------------------------------------------------
by stof at 2012-10-13T17:48:48Z
@asm89 ping
---------------------------------------------------------------------------
by fabpot at 2012-10-29T09:48:40Z
@asm89 If you don't have time, I can finish the work on this PR, but can you just tell me what's left?
---------------------------------------------------------------------------
by asm89 at 2012-10-29T10:02:22Z
I can pick this up, but I have two outstanding questions:
- One about adding `::create()`? https://github.com/symfony/symfony/pull/4935#discussion_r1358297
- And what is the final verdict on the messages? https://github.com/symfony/symfony/pull/4935#discussion_r1165701 The initial idea was that the exception itself have an exception message which is plain english and informative for the developer. If you want to display the 'safe' user messages you have the optional dependency on the translator. There is a comparison made with the Validator component, but in my opinion that's a different case because the violations always contain the message directed at the user and have no plain english message for the developer. Apart from that the Validator component contains it's own code for replacing `{{ }}` variables in messages (duplication? not as flexible as the translator). Concluding I'd opt for: optional dependency on translator component if you want to show 'safe' user messages + message keys.
@schmittjoh Any things to add?
---------------------------------------------------------------------------
by schmittjoh at 2012-10-29T10:14:09Z
Message keys sound good to me. I wouldn't add the ``create`` method for now.
On Mon, Oct 29, 2012 at 11:02 AM, Alexander <notifications@github.com>wrote:
> I can pick this up, but I have two outstanding questions:
>
> - One about adding ::create()? symfony/symfony#4935<https://github.com/symfony/symfony/issues/4935#discussion_r1358297>
> - And what is the final verdict on the messages? symfony/symfony#4935<https://github.com/symfony/symfony/issues/4935#discussion_r1165701>The initial idea was that the exception itself have an exception message
> which is plain english and informative for the developer. If you want to
> display the 'safe' user messages you have the optional dependency on the
> translator. There is a comparison made with the Validator component, but in
> my opinion that's a different case because the violations always contain
> the message directed at the user and have no plain english message for the
> developer. Apart from that the Validator component contains it's own code
> for replacing {{ }} variables in messages (duplication? not as
> flexible as the translator). Concluding I'd opt for: optional dependency on
> translator component if you want to show 'safe' user messages + message
> keys.
>
> @schmittjoh <https://github.com/schmittjoh> Any things to add?
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/symfony/symfony/pull/4935#issuecomment-9861016>.
>
>
---------------------------------------------------------------------------
by fabpot at 2012-10-29T10:27:37Z
As I said in the discussion about the translations, I'm -1 for the message keys to be consistent with how we manage translations everywhere else in the framework.
---------------------------------------------------------------------------
by stof at 2012-10-29T10:30:50Z
@fabpot When we changed the English translation for the validation errors in 2.1, we had to tag the commit as a BC rbeak as it was changing the source for all other translations. And if you look at the state of the files now, you will see that we are *not* using the English as source anymore in some places as some validation errors have a pluralized translation but the source has not been changed.
So I think using a key is more future-proof.
---------------------------------------------------------------------------
by asm89 at 2012-10-30T19:44:49Z
Any final decision on this? On one hand I have @stof and @schmittjoh +1 on message keys, on the other @fabpot -1. I guess it's your call @fabpot.
Edit: also @vicb seemed to be +1 on message keys earlier on.
---------------------------------------------------------------------------
by drak at 2012-11-01T20:19:00Z
I am also -1, I agree with @fabpot
---------------------------------------------------------------------------
by asm89 at 2012-11-12T09:38:51Z
@fabpot Can you please give a definite answer on this? I personally think @stof and @vicb have good points to do message keys, but with all these different people +1 and -1'ing the PR I'm lost on what it should actually do.
---------------------------------------------------------------------------
by asm89 at 2012-11-14T09:59:06Z
ping @fabpot
---------------------------------------------------------------------------
by asm89 at 2012-11-26T10:01:27Z
ping @fabpot We talked about this in Berlin. Any final thoughts on the PR? :) One idea was to do message keys + opt depend on the translator component if you want to use them, or use your own implementation.
---------------------------------------------------------------------------
by fabpot at 2012-11-26T14:01:37Z
The conclusion is: keep using plain English.
On Mon, Nov 26, 2012 at 11:01 AM, Alexander <notifications@github.com>wrote:
> ping @fabpot <https://github.com/fabpot> We talked about this in Berlin.
> Any final thoughts on the PR? :) One idea was to do message keys + opt
> depend on the translator component if you want to use them, or use your own
> implementation.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/symfony/symfony/pull/4935#issuecomment-10709997>.
>
>
---------------------------------------------------------------------------
by Inori at 2012-11-26T15:00:22Z
is this final? if not, then +1 for message keys
---------------------------------------------------------------------------
by vicb at 2012-11-27T22:33:47Z
@fabpot I can't understand why we keep discussing this for months as this implementation use *both* keys and plain Englis, ie using keys is optional ( if it was not it would not be an issue according to #6129)
---------------------------------------------------------------------------
by asm89 at 2013-01-02T21:43:46Z
@fabpot @vicb I'll rebase this PR, fix the comments and refactor the message keys to use plain English + {{ }} syntax for the placeholders.
---------------------------------------------------------------------------
by asm89 at 2013-01-07T15:00:58Z
@fabpot If I fix this tonight, will it make the beta?
---------------------------------------------------------------------------
by fabpot at 2013-01-07T15:53:00Z
yes, definitely.
---------------------------------------------------------------------------
by asm89 at 2013-01-07T20:13:38Z
@fabpot I switched the implementation to English messages instead of message keys and fixed the final comments + rebased. Anything you want me to do after this?
Still happy with `getMessageKey()`?
This PR was merged into the master branch.
Commits
-------
6703fb5 added changelog entries
1997e2e fix phpdoc of UrlGeneratorInterface that missed some exceptions and improve language of exception message
f0415ed [Routing] made reference type fully BC and improved phpdoc considerably
7db07d9 [Routing] added tests for generating relative paths and network paths
75f59eb [Routing] add support for path-relative and scheme-relative URL generation
Discussion
----------
[2.2] [Routing] add support for path-relative URL generation
Tests pass: yes
Feature addition: yes
BC break: <del>tiny (see below)</del> NO
deprecations: NO
At the moment the Routing component only supports absolute and domain-relative URLs, e.g.
`http://example.org/user-slug/article-slug/comments` and
`/user-slug/article-slug/comments`.
But there are two link types missing: schema-relative URLs and path-relative URLs.
schema-relative: e.g. `//example.org/user-slug/article-slug/comments`
path-relative: e.g. `comments`.
Both of them would now be possible with this PR. I think it closes a huge gap in the Routing component.
Use cases are pretty common. Schema-relative URLs are for example used when you want to include assets (scripts, images etc) in a secured website with HTTPS. Path-relative URLs are the only option when you want to generate static files (e.g. documentation) that can be downloaded as an HTML archive. Such use-cases are currently not possible with symfony.
The calculation of the relative path based on the request path and target path is hightly unit tested. So it is really equivalent. I found several implemenations on the internet but none of them worked in all cases. Mine is pretty short and works.
I also added an optional parameter to the twig `path` function, so this feature can also be used in twig templates.
Ref: This implements path-relative URLs as suggested in #3908.
<del>[BC BREAK] The signature of UrlGeneratorInterface::generate changed to support scheme-relative and path-relative URLs. The core UrlGenerator is BC and does not break anything, but users who implemented their own UrlGenerator need to be aware of this change. See UrlGenerator::convertReferenceType.</del>
---------------------------------------------------------------------------
by jalliot at 2012-04-16T09:56:56Z
@Tobion For completeness, you should add the option to the `url` and `asset` twig functions/template helpers.
---------------------------------------------------------------------------
by stof at 2012-04-16T10:46:06Z
@jalliot adding the option to ``url`` does not make any sense. The difference between ``path`` and ``url`` is that ``path`` generates a path and ``url`` generates an absolute url (thus including the scheme and the hostname)
---------------------------------------------------------------------------
by Tobion at 2012-04-16T12:27:49Z
@stof I guess jalliot meant we could then generate scheme-relative URLs with `url`. Otherwise this would have no equivalent in twig.
---------------------------------------------------------------------------
by jalliot at 2012-04-16T12:34:08Z
@stof Yep I meant what @Tobion said :)
---------------------------------------------------------------------------
by Tobion at 2012-04-18T11:57:04Z
The $relative parameter I added besides the existing $absolute parameter of the `->generate` method was not clear enough. So I merged those into a different parameter `referenceType`. I adjusted all parts of symfony to use the new signature. And also made the default `UrlGenerator` implementation BC with the old style. So almost nobody will recognize a change. The only BC break would be for somebody who implemented his own `UrlGenerator` and did not call the parent default generator.
Using `referenceType` instead of a simple Boolean is much more flexible. It will for example allow a custom generator to support a new reference type like http://en.wikipedia.org/wiki/CURIE
---------------------------------------------------------------------------
by Tobion at 2012-04-18T13:34:58Z
ping @schmittjoh considering your https://github.com/schmittjoh/JMSI18nRoutingBundle/blob/master/Router/I18nRouter.php would need a tiny change
---------------------------------------------------------------------------
by schmittjoh at 2012-04-18T13:37:39Z
Can you elaborate the necessary change?
---------------------------------------------------------------------------
by Tobion at 2012-04-18T13:51:10Z
This PR changes the signature of `generate` to be able to generate path-relative and scheme-relative URLs. So it needs to be
`public function generate($name, $parameters = array(), $referenceType = self::ABSOLUTE_PATH)` and your implementation would need to change `if ($absolute && $this->hostMap) {` to `if (self::ABSOLUTE_URL === $referenceType && $this->hostMap) {`
I can do a PR if this gets merged.
---------------------------------------------------------------------------
by schmittjoh at 2012-04-18T13:52:14Z
If I understand correctly, the old parameter still works, no?
edit: Ah, ok I see what you mean now.
---------------------------------------------------------------------------
by Tobion at 2012-04-18T13:56:33Z
Yeah the old parameter still works but $absolute would also evaluate to true (a string) in your case for non-absolute URLs, i.e. paths.
---------------------------------------------------------------------------
by Tobion at 2012-04-19T21:09:46Z
ping @fabpot
---------------------------------------------------------------------------
by fabpot at 2012-04-20T04:30:18Z
Let's discuss that feature for 2.2.
---------------------------------------------------------------------------
by Tobion at 2012-04-20T10:40:59Z
What are your objections against it? It's already implemented, it works and it adds support for things that are part of a web standard. The BC break is tiny at the moment (almost nobody is affected) because the core UrlGenerator works as before. But if we waited for 2.2 it will be much harder to make the transition because 2.1 is LTS. So I think is makes sense to add it now. Furthermore it makes it much more future-proof as custom generators can more easiliy add support for other link types like CURIE. At the moment a Boolean for absolute URLs is simply too limited and also somehow inconsistent because $absolute = false stands for an absolute path. You see the awkwardness in this naming.
Btw, I added a note in the changelog. And I will add documentation of this feature in symfony-docs once this is merged.
---------------------------------------------------------------------------
by fabpot at 2012-04-20T12:14:32Z
nobody has ever said that 2.1 would be LTS. Actually, I think we are going to wait for 2.3 for LTS.
---------------------------------------------------------------------------
by Tobion at 2012-04-20T12:27:18Z
Well what I meant is, the longer we wait with this, the harder to apply it.
In 04ac1fdba2 you modified `generate` signature for better extensibility that is not even made use of. I think changing `$abolute` param goes in the same direction and has direct use.
I'd like to know your reason to wait for 2.2. Not enough time to review it, or afraid of breaking something, or marketing for 2.2?
---------------------------------------------------------------------------
by stof at 2012-04-20T16:28:27Z
@Tobion the issue is that merging new features forces to postpone the release so that it is tested by enough devs first to be sure there is no blocking bug in it. Big changes cannot be merged when we are hunting the remaining bugs to be able to release.
---------------------------------------------------------------------------
by schmittjoh at 2012-04-20T16:42:11Z
Considering the changes that have been made to the Form component, and are still being made, I think this is in comparison to that a fairly minor change.
Maybe a clearer guideline on the release process, or the direction would help, and avoid confusion, or wrong expectations on contributors' part.
---------------------------------------------------------------------------
by Tobion at 2012-10-05T13:52:11Z
@fabpot this is ready. So if you agree with it, I would create a documentation PR.
---------------------------------------------------------------------------
by stof at 2012-10-13T16:09:47Z
@fabpot what do you think about this PR ?
---------------------------------------------------------------------------
by Crell at 2012-11-01T16:05:01Z
This feels like it's overloading the generate() method to do double duty: One, make a URl based on a route. Two, make a URI based on a URI snippet. Those are two separate operations. Why not just add a second method that does the second operation and avoid the conditionals? (We're likely to do that in Drupal for our own generator as well.)
---------------------------------------------------------------------------
by Tobion at 2012-11-01T16:38:39Z
@crell: No, you must have misunderstood something. The generate method still only generates a URI based on a route. The returned URI reference can now also be a relative path and a network path. Thats all.
---------------------------------------------------------------------------
by Tobion at 2012-12-13T18:30:28Z
@fabpot this is ready. It is fully BC! I also improved phpdoc considerably.
---------------------------------------------------------------------------
by Tobion at 2012-12-14T20:51:38Z
@fabpot Do you want me to write documentation for it? I would also be interested to write about the new features of the routing component in general. I wanted to do that anyway and it would probably be a good fit for your "new in symfony" articles.
---------------------------------------------------------------------------
by fabpot at 2012-12-14T20:58:16Z
Im' going to review this PR in the next coming days. And to answer your second question, more documentation or better documentation is always a good thing, so go for it.
---------------------------------------------------------------------------
by Tobion at 2013-01-02T21:50:20Z
@fabpot ping. I added changelog entries.
* 2.1:
[Console] Fix style escaping parsing
[Console] Make style formatter matching less greedy to avoid having to escape when not needed
[Bundle] [FrameworkBundle] fixed indentation in esi.xml services file.
[Component] [Security] fixed PSR-2 coding violation in ClassUtilsTest class.
[Form] Fixed EntityChoiceList when loading objects with negative integer IDs
[TwigBundle] There is no CSS visibility of display, should be visible instead
[Form] corrected source node for a Danish translation
[DependencyInjection] fixed a bug where the strict flag on references were lost (closes#6607)
[HttpFoundation] Check if required shell functions for `FileBinaryMimeTypeGuesser` are not disabled
[CssSelector] added css selector with empty string
[HttpFoundation] Docblock for Request::isXmlHttpRequest() now points to Wikipedia
[DependencyInjection] refactored code to avoid logic duplication
[Form] Deleted references in FormBuilder::getFormConfig() to improve performance
[HttpFoundation] Update docblock for non-working method
Conflicts:
src/Symfony/Bundle/TwigBundle/Resources/views/Exception/trace.html.twig
src/Symfony/Bundle/TwigBundle/Resources/views/Exception/traces.html.twig
This PR was merged into the master branch.
Commits
-------
184c8e5 Fixed @expectedException definitions to reference absolute exception paths
Discussion
----------
Added leading slashes to @expectedException definitions
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: -
This PR adds leading backslashes `\` to `@expectedException` declarations.
The current version is inconsistent with other class references in doc blocks, which default to *relative* class names unless class name has a leading backslash.
```php
/**
* @param RelativeNs\Class $param1
* @param \AbsoluteNs\Class $param2
*/
but
/**
* @expectedException AbsoluteNs\Class
*/
```
Consequently, PHPStorm does not understand the current statements and marks them as erroneous (and correctly so IMO, even though PHPUnit's interpretation is more relaxed).
* 2.1:
Restrict Monolog version to be in version <1.3
[Console] Make getTerminalWith & getTerminalHeight public
[DependencyInjection] fixed PhpDumper optimizations when an inlined service depends on the current one indirectly
[DependencyInjection] fixed PhpDumper when an inlined service definition has some properties
[DependencyInjection] added some tests for PhpDumper when the container is compiled
[DependencyInjection] fixed CS
[Process] Do not reset stdout/stderr pipes on Interrupted system call
[Locale] Adjust `StubIntlDateFormatter` to have new methods added in PHP 5.5
use the right RequestMatcherInterface
[Locale] Fix failing `StubIntlDateFormatter` tests in PHP 5.5
[Locale] Fix failing `StubIntlDateFormatter` in PHP 5.5
[Form] Fix failing `MonthChoiceList` in PHP 5.5
Update .travis.yml
Conflicts:
src/Symfony/Bridge/Monolog/composer.json
src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php
This PR was merged into the 2.1 branch.
Commits
-------
d601b13 use the right RequestMatcherInterface
Discussion
----------
use the right RequestMatcherInterface
this fixes#6470 by using the right interface
* 2.1: (24 commits)
updated license year
Update src/Symfony/Component/HttpFoundation/Response.php
[Form] Fixed inheritance of "error_bubbling" in RepeatedType
[Form] Fixed DateType when used with the intl extension disabled.
[HttpFoundation] fix return types and handling of zero in Response
[HttpFoundation] better fix for non-parseable Expires header date
Fixed missing plural message in portuguese validator
Fix Expires when the header is -1
[DoctrineBridge] Allowing memcache port to be 0 to support memcache unix domain sockets.
[Console] fixed unitialized properties (closes#5935)
[Process] Prevented test from failing when pcntl extension is not enabled.
Revert "[DoctrineBridge] Improved performance of the EntityType when used with the "query_builder" option"
[Form] Fixed failing tests for DateTimeToStringTransformer.
[Locale] Fixed the StubLocaleTest for ICU versions lower than 4.8.
[Bundle] [FrameworkBundle] fixed typo in phpdoc of the SessionListener.
[Form] Fixed test regression introduced in #6440
[Tests] Fix namespaces
Fixed php doc of GenericEvent::__construct
HttpUtils must handle RequestMatcher too
use preferred_choices in favor of preferred_query
...
Conflicts:
src/Symfony/Bridge/Propel1/Form/ChoiceList/ModelChoiceList.php
* 2.0:
updated license year
Update src/Symfony/Component/HttpFoundation/Response.php
[Console] fixed unitialized properties (closes#5935)
[Bundle] [FrameworkBundle] fixed typo in phpdoc of the SessionListener.
bumped Symfony version to 2.0.21-DEV
updated VERSION for 2.0.21
updated CHANGELOG for 2.0.21
Conflicts:
src/Symfony/Bundle/SwiftmailerBundle/LICENSE
src/Symfony/Component/Filesystem/LICENSE
src/Symfony/Component/HttpFoundation/Response.php
src/Symfony/Component/HttpKernel/Kernel.php
This PR was merged into the 2.1 branch.
Commits
-------
bfccd28 HttpUtils must handle RequestMatcher too
Discussion
----------
HttpUtils must handle RequestMatcher too
2.1 introduced the RequestMatcher as alternative to UrlMatcher. but HttpUtils was not adjusted.
---------------------------------------------------------------------------
by lsmith77 at 2013-01-01T18:15:13Z
@fabpot could you have a look at this PR? would like to know if this will be addressed in core or if we need to find another solution inside the CMF routing.
Improve the timing safe comparison function to better handle cases where input is of different length.
Note that it is now important to always pass any string that the user can directly control to the second parameter of the function. Otherwise, length information may be leaked.
This PR was merged into the 2.0 branch.
Commits
-------
8b2c17f fix double-decoding in the routing system
Discussion
----------
fix double-decoding in the routing system
@fabpot @vicb This should fix it. You know what ;) Don't want to leak more information.
And the good thing, it's no hack nor does it break BC.
* 2.1:
fixed CS
fixed CS
[Security] fixed path info encoding (closes#6040, closes#5695)
[HttpFoundation] added some tests for the previous merge and removed dead code (closes#6037)
Improved Cache-Control header when no-cache is sent
removed unneeded comment
Fix to allow null values in labels array
fix date in changelog
removed the Travis icon (as this is not stable enough -- many false positive, closes#6186)
Revert "merged branch gajdaw/finder_splfileinfo_fpassthu (PR #4751)" (closes#6224)
Fixed a typo
Fixed: HeaderBag::parseCacheControl() not parsing quoted zero correctly
[Form] Fix const inside an anonymous function
[Config] Loader::import must return imported data
[DoctrineBridge] Fixed caching in DoctrineType when "choices" or "preferred_choices" is passed
[Form] Fixed the default value of "format" in DateType to DateType::DEFAULT_FORMAT if "widget" is not "single_text"
[HttpFoundation] fixed a small regression
Conflicts:
src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/MongoDbSessionHandlerTest.php
* 2.0:
fixed CS
removed the Travis icon (as this is not stable enough -- many false positive, closes#6186)
[Config] Loader::import must return imported data
[HttpFoundation] fixed a small regression
Conflicts:
README.md
src/Symfony/Bridge/Twig/Extension/FormExtension.php
src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/attributes.html.php
src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/form_widget.html.php
src/Symfony/Bundle/FrameworkBundle/Templating/Helper/FormHelper.php
src/Symfony/Component/Form/Form.php
src/Symfony/Component/HttpFoundation/Request.php
src/Symfony/Component/HttpFoundation/SessionStorage/PdoSessionStorage.php
tests/Symfony/Tests/Bridge/Doctrine/Logger/DbalLoggerTest.php
This PR was squashed before being merged into the master branch (closes#5860).
Commits
-------
d0057d0 Added failure_path_parameter to mirror target_path_parameter
Discussion
----------
Added failure_path_parameter to mirror target_path_parameter
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
License of the code: MIT
Enable login failure redirect path can be assigned in a form field just like target path.
---------------------------------------------------------------------------
by stof at 2012-10-29T09:40:17Z
Please also open a PR to the doc repo to document this new feature
---------------------------------------------------------------------------
by leevigraham at 2012-10-29T09:56:29Z
@stof @fabpot Done.
* 2.1: (29 commits)
[DependencyInjection] fixed composer.json
[Validator] Fix typos in validators.ru.xlf
Edited some minor grammar and style errors in russian validation file
Updated Bulgarian translation
[Form] improve error message with a "hasser" hint for PropertyAccessDeniedException
[Form] Updated checks for the ICU version from 4.5+ to 4.7+ due to test failures with ICU 4.6
[Form] simplified a test from previous merge
Update src/Symfony/Component/Form/Extension/Core/Type/FileType.php
fixed CS
Xliff with other node than source or target are ignored
small fix of #5984 when the container param is not set
Filesystem Component mirror symlinked directory fix
[Process][Tests] fixed chainedCommandsOutput tests
fixed CS
Use better default ports in urlRedirectAction
Add tests for urlRedirectAction
info about session namespace
fix upgrade info about locale
Update src/Symfony/Component/DomCrawler/Tests/FormTest.php
Update src/Symfony/Component/DomCrawler/Form.php
...
* 2.0:
[DependencyInjection] fixed composer.json
[Form] Updated checks for the ICU version from 4.5+ to 4.7+ due to test failures with ICU 4.6
fixed CS
small fix of #5984 when the container param is not set
fixed CS
Use better default ports in urlRedirectAction
Add tests for urlRedirectAction
Update src/Symfony/Component/DomCrawler/Tests/FormTest.php
Update src/Symfony/Component/DomCrawler/Form.php
[Security] remove escape charters from username provided by Digest DigestAuthenticationListener
[Security] added test extra for digest authentication
fixed CS
[Security] Fixed digest authentication
[Security] Fixed digest authentication
[SecurityBundle] Convert Http method to uppercase in the config
Use Norm Data instead of Data
Conflicts:
src/Symfony/Bridge/Doctrine/Form/EventListener/MergeCollectionListener.php
src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php
src/Symfony/Component/DependencyInjection/composer.json
This PR was squashed before being merged into the master branch (closes#5888).
Commits
-------
2379d86 CS Fixes - Replaced "array of type" by "Type[]" in PHPDoc block
Discussion
----------
CS Fixes - Replaced "array of type" by "Type[]" in PHPDoc block
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: no (but tests doesn't pass on master too). See Travis.
License of the code: MIT
Documentation PR: Not Applicable
Status: Finished
To improve support of the eclipse PDT pluggin (for autocompletion), I propose to change the array notation in PHPDoc blocks to match the phpDocumentor notation for "array of type".
Modifications are made for the following components:
- BrowserKit
- ClassLoader
- Config
- Console
- CssSelector
- DependencyInjection
- DomCrawler
- EventDispatcher (no changes)
- Filesystem (no changes)
- Finder
- Form
- HttpFoundation
- HttpKernel
- Locale
- OptionResolver (no changes)
- Process (no changes)
- Routing (no changes)
- Serializer (no changes)
- Templating
- Translation
- Validator
- Yaml (no changes)
- Security
- Stopwatch (no changes)
See Proposal https://github.com/symfony/symfony/pull/5852
---------------------------------------------------------------------------
by pborreli at 2012-11-01T15:19:27Z
will you make a PR for each component ? why not only one PR with one commit for each component instead ?
---------------------------------------------------------------------------
by raziel057 at 2012-11-01T15:32:39Z
Ok, I'm going try to do it.
---------------------------------------------------------------------------
by raziel057 at 2012-11-01T16:12:56Z
I would like to rename my branch from COMPONENT_Form to changes-phpdoc (as all modifications would be commited in only one branch), so I tried to execute the following command but I have an error.
git remote rename COMPONENT_Form changes-phpdoc
error: Could not rename config section 'remote.COMPONENT_Form' to 'remote.changes-phpdoc'
Do you know how to do it?
---------------------------------------------------------------------------
by pborreli at 2012-11-01T16:14:26Z
don't rename it, you will have to close and make another PR which is useless here, just edit the title.
---------------------------------------------------------------------------
by stof at 2012-11-01T16:16:17Z
and ``git remote rename`` is about renaming a remote repo, not a branch
---------------------------------------------------------------------------
by raziel057 at 2012-11-03T11:36:02Z
Is it normal that all my commit are duplicated? I would like just update my master and merge with my branch.
---------------------------------------------------------------------------
by fabpot at 2012-11-06T10:22:55Z
@raziel057 Can you rebase on master? That should fix your problem.
---------------------------------------------------------------------------
by fabpot at 2012-11-09T13:28:53Z
@raziel057 Can you finish this PR?
---------------------------------------------------------------------------
by Tobion at 2012-11-09T13:34:45Z
I'll do it for the routing component this evening because I know it by heart. ^^
---------------------------------------------------------------------------
by raziel057 at 2012-11-09T15:06:26Z
@Tobion ok Thanks!
@fabpot Yes, I will try to finish it this week end.
---------------------------------------------------------------------------
by raziel057 at 2012-11-11T13:04:07Z
@Tobion Did you already change PHPDoc in the Routing component?
---------------------------------------------------------------------------
by Tobion at 2012-11-11T15:21:18Z
@raziel057 Yes I'm working on it.
---------------------------------------------------------------------------
by Tobion at 2012-11-12T15:16:31Z
@raziel057 Done. See #5994
* 2.1:
fixed comment. The parent ACL is not accessed in this method.
[HttpFoundation] Make host & methods really case insensitive in the RequestMacther
[Validator] fixed Ukrainian language code (closes#5972)
Fixed case of php function
* 2.0:
fixed comment. The parent ACL is not accessed in this method.
[HttpFoundation] Make host & methods really case insensitive in the RequestMacther
[Validator] fixed Ukrainian language code (closes#5972)
Fixed case of php function
Conflicts:
src/Symfony/Bundle/FrameworkBundle/Resources/translations/validators.uk.xliff
src/Symfony/Component/HttpFoundation/RequestMatcher.php
* 2.1: (24 commits)
forced Travis to use source to workaround their not-up-to-date Composer on PHP 5.3.3
[Routing] removed irrelevant string cast in Route
Fixed typo
Make YamlFileLoader and XmlFileLoader file loading extensible
[HttpKernel] fix typo
Fixed singularization of "prices"
[Form] Removed an exception that prevented valid formats from being passed, e.g. "h" for the hour, "L" for the month etc.
[HttpKernel] fixed Client when using StreamedResponses (closes#5370)
fixed PDO session handler for Oracle (closes#5829)
[HttpFoundation] fixed PDO session handler for Oracle (closes#5829)
[Locale] removed a check that is done too early (and it is done twice anyways)
Update src/Symfony/Component/Validator/Resources/translations/validators.fa.xlf
Adding new localized strings for farsi validation.
[HttpFoundation] moved the HTTP protocol check from StreamedResponse to Response (closes#5937)
[Form] Fixed forms not to be marked invalid if their children are already marked invalid
[Form] Excluded some tests in NumberToLocalizedStringTransformerTest which fail on ICU 4.4, but work on ICU 4.8
added missing tests from previous merge
[Form] Fixed NumberToLocalizedStringTransformer to accept both comma and dot as decimal separator, if possible
Fix export-ignore on Windows
Show correct class name InputArgument in error message
...
Conflicts:
.travis.yml
src/Symfony/Component/Form/Extension/Core/DataTransformer/NumberToLocalizedStringTransformer.php
This PR was squashed before being merged into the master branch (closes#5879).
Commits
-------
07bd5c6 Make non-instantiable utils classes consistent with each other
Discussion
----------
Make non-instantiable utils classes consistent with each other
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
License of the code: MIT
As per discussion in #5875 turned out that we don't have a consistent way to define non-instantiatable classes.
I don't like `final` as it removes flexibility with no visible gain.
I don't like `abstract` since it's not specifically clear what is meant by that. Is this class not complete? Should it be extended?
This PR was merged into the master branch.
Commits
-------
af87c2b changed the Firewall to be a proper subscriber
02bd359 changed the remember-me listener to be a proper subscriber
Discussion
----------
Changed some security classes to implement the EventSubscriberInterface interface
---------------------------------------------------------------------------
by fabpot at 2012-11-06T10:11:28Z
That could also be done in 2.1. What do you think?
This PR was merged into the master branch.
Commits
-------
e193590 [Security] removed the 401 error custom status message
Discussion
----------
[Security] removed the 401 error custom status message
see fabpot/Silex#496
---------------------------------------------------------------------------
by pborreli at 2012-10-31T17:29:24Z
@fabpot please fix the test suite, if you don't know how to do it, read http://symfony.com/doc/current/contributing/code/tests.html, thx 😸
This PR was merged into the master branch.
Commits
-------
3e58893 [Security] Tweak UsernamePasswordFormAuthenticationListener
Discussion
----------
[Security] Tweak UsernamePasswordFormAuthenticationListener
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: [](http://travis-ci.org/acasademont/symfony)
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: -
Improvements:
- Do not check twice for the ```only_post``` condition. The condition in the ```attemptAuthentication``` method is useless as this method will never be called if the previous ```requiresAuthentication``` call returns false.
- If the expected request is ```only_post```, check only the POST variables for the username and password parameters. Otherwise, query params and attributes are checked before.
- Use POST instead of post for correctness
- Do not check twice for the only_post condition
- If the expected request is only_post, check only the post variables for the username and password parameters
* 2.1: (28 commits)
Delete use of CreationExeption
[Form] Fixed error message in PropertyPath to not advice to use a non-existing feature
[Form] Fixed creation of multiple money fields with different currencies
[Form] Fixed setting the "data" option to an object in "choice" and "entity" type
Fixed Serbian plural translations.
Fixed IPv6 Check in RequestMatcher
Fix typo
change what I think is a typo
[Console] Fix error when mode is not in PATH
[WebProfilerBundle] fixed macro usage (to be forward compatible with Twig 2.x)
Change monolog require-dev to use the branch alias instead of dev-master
[FrameworkBundle] partially reverted previous merge
[2.1] Added missing error return codes in commands
Made the router lazy when setting the context
[WebProfilerBundle] fixed typos
Fix incorrect variable in FileProfilerStorage
UnitTest fix
UnitTest fix
added a unit test
fixed#5384
...
[Security] changed default iterations of Pbkdf2PasswordEncoder to 1000 instead of 5000
[Security] Improved description of PBKDF2 encoder
[SecurityBundle] added PBKDF2 PasswordEncoder
updated CHANGELOG.md
[Security] Use the build-in hash_pbkdf2() when available
[SecurityBundle] added information about hash_algorithm for configuration
[Security] always check algorithm and fixed CS
* 2.1:
fixed CS
added doc comments
added doc comments
[Validator] Updated swedish translation
Update src/Symfony/Component/Validator/Resources/translations/validators.de.xlf
[2.1] Exclude tests from zips via gitattributes
[HttpKernel][Translator] Fixed type-hints
Updated lithuanian validation translation
[DomCrawler] Allows using multiselect through Form::setValues().
[Translation] forced the catalogue to be regenerated when a resource is added (closes symfony/Translation#1)
Unit test for patched method OptionsResolver::validateOptionValues().
validateOptionValues throw a notice if an allowed value is set and the corresponding option isn't.
[Form] Hardened code of ViolationMapper against errors
[HttpFoundation] Fixed#5611 - Request::splitHttpAcceptHeader incorrect result order.
[Form] Fixed negative index access in PropertyPathBuilder
Update src/Symfony/Component/Validator/Resources/translations/validators.ro.xlf
Conflicts:
src/Symfony/Component/DomCrawler/Form.php
src/Symfony/Component/Process/Process.php
* 2.1:
[2.1] Fix SessionHandlerInterface autoloading
Remove executable bit from HttpKernel/DependencyInjection/ConfigurableExtension.php
[2.0][http-foundation] Fix Response::getDate method
[DoctrineBridge] Require class option for DoctrineType
[HttpFoundation] fixed the path to the SensioHandlerInterface class in composer.json
Support the new Microsoft URL Rewrite Module for IIS 7.0. @see http://framework.zend.com/issues/browse/ZF-4491 @see http://framework.zend.com/code/revision.php?repname=Zend+Framework&rev=24842
fixed undefined variable
hasColorSupport does not take an argument
Improve FilterResponseEvent docblocks Response ref
Commits
-------
1b5ad17 Revert "Removed MySQL-exclusive usage of unsigned integer from table creation"
Discussion
----------
[Security][DBAL] Revert MySQL unsigned removal
Revert "Removed MySQL-exclusive usage of unsigned integer from table creation"
This reverts commit 57694aaa94.
The problem is underlying in Doctrine DBAL change tracking and should
either be fixed or ignored there.
I opened a ticket on Doctrine Jira http://doctrine-project.org/jira/browse/DBAL-322
---------------------------------------------------------------------------
by fabpot at 2012-08-14T06:40:47Z
I will merge this PR after we have a release of DBAL that includes the fix for DBAL-322.
---------------------------------------------------------------------------
by acasademont at 2012-08-20T08:01:48Z
This was already fixed 2 weeks ago in doctrine/dbal#183 so i guess this can be closed
---------------------------------------------------------------------------
by acasademont at 2012-08-20T08:02:06Z
merged i mean
When use_referer is set to true and the request comes from the login page,
the user should not be redirected to the login form again (the referer) but
to the default_target_path. The problem arises when our login_path option
is not a path but a route name, as the ```getUriForPath()``` method is not
made to create routes from route names.
Commits
-------
134cc84 [Security] Fix DocBlock of attemptAuthentication
Discussion
----------
[Security] Fix DocBlock of attemptAuthentication
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets:
Todo: -
License of the code: MIT
Documentation PR: -
Add Response as possible return type of the method because the method AbstractAuthenticationListener::handle() test if $returnValue is an instance of Response (line 148).
Commits
-------
1f2f866 fixed the serialization of the SwitchUserRole
b55930a [Security] Implemented the Serializable interface in the Role class
Discussion
----------
[Security] Implemented the Serializable interface in the Role class
The Role class is serialized in the session for each role of the user. Implementing the Serializable interface allows to reduce the size of the data.
Commits
-------
5e6c06f [Security] Remove hard dependency on $providerKey for default auth success handler
Discussion
----------
[Security] Remove hard dependency on $providerKey for default auth success handler
Bug fix: yes?
Feature addition: yes?
Backwards compatibility break: no
Symfony2 tests pass: [](http://travis-ci.org/asm89/symfony)
License of the code: MIT
In 8ffaafa867 a hard dependency was introduced between the default authentication success handling code and the active firewall. This makes sense. However, for people implementing their own success handler this makes it impossible to extend the default class as the `$providerKey` is set in the extension of the security bundle.
This PR makes the dependency a soft one so people can extend the class and use the default definition as a parent for their own service. However it is the responsibility of the developers to set the appropriate `$providerKey` if they want to use the target url saved in the session. Imo this is the right way as the developer should also set the appropriate options for the parent class in the overriding constructor.
---------------------------------------------------------------------------
by stof at 2012-07-11T19:01:12Z
@asm89 this PR need to be rebased according to github
---------------------------------------------------------------------------
by asm89 at 2012-07-11T19:13:09Z
@stof Done :)
---------------------------------------------------------------------------
by asm89 at 2012-07-12T10:07:53Z
@fabpot Done.
Commits
-------
bb138da [Security] Fix regression after rebase. Target url should be firewall dependent
eb19f2c [Security] Add note to CHANGELOG about refactored authentication failure/success handling [Security] Various CS + doc fixes [Security] Exception when authentication failure/success handlers do not return a response [Security] Add authors + fix docblock
f9d5606 [Security] Update AuthenticationFailureHandlerInterface docblock. Never return null
915704c [Security] Move default authentication failure handling strategy to seperate class [Security] Update configuration for changes regarding default failure handler [Security] Fixes + add AbstractFactory test for failure handler
c6aa392 [Security] Move default authentication success handling strategy to seperate class [Security] Update configuration for changes regarding default success handler [Security] Fix + add AbstractFactory test
Discussion
----------
[Security] Refactor authentication success handling
Bug fix: no
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: [](http://travis-ci.org/asm89/symfony)
License of the code: MIT
This PR extracts the default authentication success handling to its own class as discussed in #4553. In the end the PR will basically revert #3183 (as suggested by @schmittjoh) and fix point one of #838.
There are a few noticeable changes in this PR:
- This implementation changes the constructor signature of the `AbstractAuthentictionListener` and `UsernamePasswordFormAuthenticationListener` by making the `AuthenticationSuccessHandler` mandatory (BC break). If this WIP is approved I will refactor the failure handling logic too and then this will also move one place in the constructor
- This PR reverts the change of making the returning of a `Response` optional in the `AuthenticationSuccessHandlerInterface`. Developers can now extend the default behavior themselves
@schmittjoh Any suggestions? Or a +1 to do the failure logic too?
---------------------------------------------------------------------------
by schmittjoh at 2012-06-17T23:53:07Z
+1 from me
@fabpot, what so you think?
---------------------------------------------------------------------------
by fabpot at 2012-06-19T08:15:48Z
Can you add a note in the CHANGELOG? Thanks.
---------------------------------------------------------------------------
by asm89 at 2012-06-19T10:22:20Z
I will, but I'll first do the same for the failure logic.
---------------------------------------------------------------------------
by travisbot at 2012-06-21T08:03:14Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1671555) (merged 17c8f66f into 55c6df99).
---------------------------------------------------------------------------
by asm89 at 2012-06-21T08:45:38Z
👍 thank you @stof. I think this is good to go now.
---------------------------------------------------------------------------
by travisbot at 2012-06-21T08:50:28Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1671817) (merged 8982c769 into 55c6df99).
---------------------------------------------------------------------------
by asm89 at 2012-06-21T14:23:58Z
@schmittjoh @fabpot The `LogoutListener` currently throws an exception when the successhandler doesn't return a `Response` ([link](9e9519913d/src/Symfony/Component/Security/Http/Firewall/LogoutListener.php (L101))). Should this code check for this too?
---------------------------------------------------------------------------
by schmittjoh at 2012-06-21T14:26:49Z
Yes, this code was removed, but needs to be re-added here as well.
---------------------------------------------------------------------------
by travisbot at 2012-06-21T15:08:59Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1674437) (merged 5afa240d into 55c6df99).
---------------------------------------------------------------------------
by asm89 at 2012-06-26T06:01:02Z
@fabpot Can you make a final decision on this? If you decide on point 3, this code can be merged. I agree with the arguments of @stof about the option handling and it 'only' being a BC break for direct users of the security component. I even think these direct users should be really careful anyway, since the behavior of the success and failurehandlers now change back to how they acted in 2.0.
Now I am thinking about it, can't the optional parameters of this class move to setters anyway? That will make it cleaner to extend.
---------------------------------------------------------------------------
by asm89 at 2012-06-28T10:29:50Z
ping @fabpot
---------------------------------------------------------------------------
by fabpot at 2012-06-28T17:23:02Z
I'm ok with option 1 (the BC break). After doing the last changes, can you squash your commits before I merge? Thanks.
---------------------------------------------------------------------------
by asm89 at 2012-07-06T21:59:54Z
@fabpot I rebased the PR, added the authors and also ported the fix that was done in 8ffaafa867 to be contained in the default success handler. I also squashed all the CS and 'small blabla fix' commits. Is it ok now?
Edit: travisbot will probably say that the tests in this PR fail, but that is because current master fails on form things
---------------------------------------------------------------------------
by asm89 at 2012-07-08T18:53:05Z
I rebased the PR, tests are green now: [](http://travis-ci.org/asm89/symfony).
[Security] Various CS + doc fixes
[Security] Exception when authentication failure/success handlers do not return a response
[Security] Add authors + fix docblock
This is not a problem with Symfony, but when using the component
standalone (Silex for instance), the context listener might be
instantiated even if the firewall does not need to be fired. In that
case, the handle() method is not called, but the response listener is
called, which means that en empty token is stored in the session.
For Silex, it means that when authenticated, if you visit a 404 page,
you would be disconnected automatically.
Commits
-------
8ffaafa Make the session entry for the target url firewall dependent.
Discussion
----------
[Security] Make the session entry for the target url firewall dependent.
Bug fix: yes
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets:
License of the code: MIT
If there are two firewalls (eg. main and admin), calling an protected admin url
will direct you to the login form of the admin. If I ignore this and go to the login
form of the main firewall directly I will end up being redirected to the stored
admin target url, which will lead me to the admin login form again.
---------------------------------------------------------------------------
by travisbot at 2012-05-25T09:33:44Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1431566) (merged 8ffaafa8 into 45849ce3).
---------------------------------------------------------------------------
by uwej711 at 2012-06-09T08:05:54Z
Doesn't this make sense or did this slip through? Or is there something missing?
Commits
-------
f72ba0a Fixed detection of an active session
Discussion
----------
[WIP][HttpFoundation][Session] Fixed detection of an active session
Bug fix: yes
Feature addition: no
Backwards compatibility break: not sure
Symfony2 tests pass: no
Fixes the following tickets: #4529
Todo: Fix failing tests
License of the code: MIT
Documentation PR: ~
This fixes the problem when the session variable inside $request now has always data in it as it's now more powerful but this introduces the problem that the old way of detecting if a session is started or not doesn't work anymore.
---------------------------------------------------------------------------
by travisbot at 2012-06-09T21:53:17Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1578839) (merged 9ae13e12 into 6266b72d).
---------------------------------------------------------------------------
by drak at 2012-06-10T01:57:59Z
Sessions should be started implicitly. The SF auto_start config parameter controls the session listener to start the session.
---------------------------------------------------------------------------
by dlsniper at 2012-06-11T06:46:02Z
So this patch is correct then and I should continue the work on it?
---------------------------------------------------------------------------
by drak at 2012-06-11T07:51:39Z
@dlsniper - no it's not correct. The session should not be auto-started like this, @fabpot and I recently discussed it.
---------------------------------------------------------------------------
by dlsniper at 2012-06-11T07:52:55Z
@Drak, ok I'll remove the patch for auto_start then but the fix for start would still stand, right?
---------------------------------------------------------------------------
by drak at 2012-06-12T18:40:35Z
@dlsniper - I have no objection to the rest of the PR except for the autostart stuff. I've annotated for clarity :)
---------------------------------------------------------------------------
by travisbot at 2012-06-12T19:51:12Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1604158) (merged 3499980e into 37550d23).
---------------------------------------------------------------------------
by travisbot at 2012-06-12T19:52:00Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1604166) (merged dcc73071 into 37550d23).
---------------------------------------------------------------------------
by dlsniper at 2012-06-12T19:56:51Z
Seems Travis doesn't like the squashing of commits that I've did but the PR does pass the normal tests.
@drak is this good for merging now?
Thanks :)
---------------------------------------------------------------------------
by dlsniper at 2012-06-13T09:05:09Z
@fabpot this can be merged safely, I've just applied the patch on my production application and the patch is ok, it's just travis failing.
Thanks
---------------------------------------------------------------------------
by travisbot at 2012-06-13T09:23:46Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1608735) (merged 1a6eabd2 into 37550d23).
---------------------------------------------------------------------------
by travisbot at 2012-06-13T09:28:26Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1608758) (merged 4e3a93c8 into 37550d23).
---------------------------------------------------------------------------
by dlsniper at 2012-06-13T09:29:28Z
I've noticed that this is failing, I'll fix it later on today.
---------------------------------------------------------------------------
by travisbot at 2012-06-13T15:14:01Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1611541) (merged 5504c4b7 into 37550d23).
---------------------------------------------------------------------------
by drak at 2012-06-13T15:23:47Z
It's possible that other tests are failing not related to this PR. Run the tests on the current master, and try rebasing your branch to the current master also.
---------------------------------------------------------------------------
by dlsniper at 2012-06-13T15:44:22Z
I've just reminded why this is failing on builds, I can't do them locally because of this:
```
Installing dev dependencies
Your requirements could not be solved to an installable set of packages.
Problems:
- Problem caused by:
- Installation request for doctrine/orm [>= 2.2.0.0, < 2.4.0.0-dev]: Satisfiable by [doctrine/orm-2.2.2, doctrine/orm-2.2.1, doctrine/orm-2.2.0, doctrine/orm-2.2.x-dev, doctrine/orm-2.3.x-dev].
```
I'll try and install this somehow and see what's wrong with it.
---------------------------------------------------------------------------
by mvrhov at 2012-06-13T18:08:58Z
@dlsniper: as @stof said to me this should be resolved in latest versions of composer, but it seems that is not. The problem is that composer cannot figure out that you are on dev-master if you try to instal dev. dependencies on feature branch. Take a look at the .travis.yml file on how to do a proper dev vendors install.
cc @Seldaek
---------------------------------------------------------------------------
by dlsniper at 2012-06-13T23:08:53Z
@mvrhov Thanks for pointing this out.
@drak I still got two tests not passing but I'm not sure how to fix them as adding $session->start() will either fail with the message that the session has already been started, the headers_sent() call which returns true. Any help with them will be greatly appreciated. Thanks!
Here is what the HttpKernel tests are returning:
```
There were 2 failures:
1) Symfony\Component\HttpKernel\Tests\EventListener\LocaleListenerTest::testDefaultLocaleWithSession
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'es'
+'fr'
/var/www/symfony-orig/src/Symfony/Component/HttpKernel/Tests/EventListener/LocaleListenerTest.php:51
2) Symfony\Component\HttpKernel\Tests\EventListener\LocaleListenerTest::testLocaleFromRequestAttribute
Expectation failed for method name is equal to <string:set> when invoked 1 time(s).
Method was expected to be called 1 times, actually called 0 times.
FAILURES!
Tests: 263, Assertions: 1025, Failures: 2, Skipped: 10.
```
---------------------------------------------------------------------------
by travisbot at 2012-06-13T23:42:59Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1614883) (merged 1004b7c0 into c07e9163).
---------------------------------------------------------------------------
by travisbot at 2012-06-13T23:53:06Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1614897) (merged f72ba0a2 into c07e9163).
---------------------------------------------------------------------------
by dlsniper at 2012-06-16T20:14:41Z
@stof / @vicb Hi, do either of you think that you can either point me out to the right direction for fixing this either ping someone else for home help as @drak doesn't seem available for this and at the moment I'm pretty much clueless in what direction I should take this fix.
Thanks!
---------------------------------------------------------------------------
by dlsniper at 2012-06-19T14:16:29Z
ping @fabpot Can you please provide some input on this one as I'm a bit stuck and seems noone else is available.
---------------------------------------------------------------------------
by drak at 2012-06-20T10:24:43Z
fyi - I'll be able to look again in a few days
---------------------------------------------------------------------------
by fabpot at 2012-07-01T07:53:28Z
I'm +1 to add the `isStarted()` method, but -1 for the change of `Request::hasSession`.
---------------------------------------------------------------------------
by drak at 2012-07-01T09:06:15Z
@fabpot, I agree. `hasSession()` should not be changed, it's semantically incorrect to make it return effectively "hasActiveSession".
If there are two firewalls (eg. main and admin), calling an protected admin url
will direct you to the login form of the admin. If I ignore this and go to the login
form of the main firewall directly I will end up being redirected to the stored
admin target url. This is not what you usually want to happen.
Commits
-------
a450d00 [HttpFoundation] HTTP Basic authentication is broken with PHP as cgi/fastCGI under Apache
Discussion
----------
[HttpFoundation] HTTP Basic authentication is broken with php-cgi under Apache
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #1813
Todo: -
In order to work, add this to the .htaccess:
RewriteEngine on
RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}]
RewriteCond %{REQUEST_FILENAME} !-f
RewriteRule ^(.*)$ app.php [QSA,L]
---------------------------------------------------------------------------
by stof at 2012-03-10T17:34:26Z
you should also add a unit test for this
---------------------------------------------------------------------------
by kepten at 2012-03-11T15:34:04Z
Thanks for the feedback, I committed the changes.
---------------------------------------------------------------------------
by stof at 2012-04-04T01:59:53Z
@fabpot could you review it ?
---------------------------------------------------------------------------
by fabpot at 2012-04-04T07:15:34Z
My comments:
* `ServerBag` represents what we have in the `$_SERVER` global variables. As such, the code should be moved to the `getHeaders()` method instead like the other tweaks we do for the HTTP headers.
* A comment must be added explaining why this is needed and the configuration the user must have to make it work (then remove the Github URLs).
* The code should only be executed when `PHP_AUTH_USER` is not available (to not have any overhead when not needed).
---------------------------------------------------------------------------
by danielholmes at 2012-04-14T13:27:09Z
A quick note on that .htaccess/apache configuration required, if adding to the Symfony SE htaccess file, then it will need to look like this:
```
<IfModule mod_rewrite.c>
RewriteEngine On
RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}]
RewriteCond %{REQUEST_FILENAME} !-f
RewriteRule ^(.*)$ app.php [QSA,L]
</IfModule>
```
NOTE: No **,L** in the Authorization Rewrite as in the original example - it prevents the front controller rewrite from happening
---------------------------------------------------------------------------
by towards at 2012-04-20T16:12:49Z
@kepten you were faster than me applying @fabpot's comments :) nevertheless part of the bug hunt day I also modified the ServerBag class and tested them on a productive LAMP hosting server using Apache and FastCGI
---------------------------------------------------------------------------
by kepten at 2012-04-20T16:15:57Z
ok, so is my PR is useless or should I still fix problems?
---------------------------------------------------------------------------
by towards at 2012-04-20T16:20:26Z
your PR is fine for sure and I don't want to interfere, just wanted to mention that part of the bug hunt day of Symfony I had a go at this PR as an "exercise" but just saw later on that you already fixed the problem, so you can ignore my pushes
---------------------------------------------------------------------------
by vicb at 2012-04-20T16:20:36Z
I have been working with @towards: your PR is useful, please implement his comments and squash your PR.
---------------------------------------------------------------------------
by kepten at 2012-04-20T16:59:07Z
never squashed before, is it okay now? :)
---------------------------------------------------------------------------
by stof at 2012-04-20T17:21:07Z
it is
---------------------------------------------------------------------------
by vicb at 2012-05-20T19:57:51Z
@fabpot this should be ready to be merged
Commits
-------
69e0451 [Security] fixed English grammar in exception message
Discussion
----------
[Security] fixed English grammar in exception message
Commits
-------
c195957 [Components] Tests/Autoloading fixes
Discussion
----------
Fix components
See #4141
----
This PR:
* configures each component to use composer to manage "dev" dependencies instead of env variables;
* adds phpunit configuration file on Filesystem component;
* fixes READMEs.
It's mergeable without any problems, but I would recommend to wait a fix in Composer in order to use `self.version` in `require`/`require-dev` sections.
Note: I kept `suggest` sections because it makes sense but this PR doesn't aim to provide useful explanations for each entry. It could be another PR, not that one.
---------------------------------------------------------------------------
by willdurand at 2012-04-30T20:43:13Z
@fabpot I reviewed each component, one by one. Now `phpunit` always works, even if tests are skipped. A simple `composer install --dev` allows to run the complete test suite. Each commit is well separated from the others. I guess, everything is ok now.
---------------------------------------------------------------------------
by Tobion at 2012-04-30T20:47:00Z
Please squash, as it makes no sense to have the same commit for each component.
---------------------------------------------------------------------------
by fabpot at 2012-05-01T14:26:11Z
Can you squash your commits before I merge? Thanks.
---------------------------------------------------------------------------
by willdurand at 2012-05-01T14:29:38Z
done
---------------------------------------------------------------------------
by fabpot at 2012-05-01T15:48:25Z
It does not seem that the commits are squashed.
---------------------------------------------------------------------------
by willdurand at 2012-05-01T15:54:08Z
done
* Switched to Composer to manage "dev" dependencies
* Fixed READMEs
* Excluded vendor in phpunit.xml.dist files
* Fixed message in bootstrap.php files
* Added autoloader for the component itself
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #1813
Todo: -
In order to work, add this to the .htaccess:
RewriteEngine on
RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}]
RewriteCond %{REQUEST_FILENAME} !-f
RewriteRule ^(.*)$ app.php [QSA,L]
Disabled exception when switching to the user that is already impersonated, exception is now only thrown when trying to switch to a new user.
Added an Excption exception when switching fails because target user does not exist.
Added funtional tests for switching users.
Commits
-------
49a8654 [Security] Use LogoutException for invalid CSRF token in LogoutListener
a96105e [SecurityBundle] Use assertCount() in tests
4837407 [SecurityBundle] Fix execution of functional tests with different names
66722b3 [SecurityBundle] Templating helpers to generate logout URL's with CSRF tokens
aaaa040 [Security] Allow LogoutListener to validate CSRF tokens
b1f545b [Security] Refactor LogoutListener constructor to take options
c48c775 [SecurityBundle] Add functional test for form login with CSRF token
Discussion
----------
[Security] Implement support for CSRF tokens in logout URL's
```
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
```
[](http://travis-ci.org/jmikola/symfony)
This derived from #3006 but properly targeting on the master branch.
This exposes new configuration options to the logout listener to enable CSRF protection, as already exists for the form login listener. The individual commits and their extended messages should suffice for explaining the logical changes of the PR.
In addition to changing LogoutListener, I also created a templating helper to generate logout URL's, which includes a CSRF token if necessary. This may or may not using routing, depending on how the listener is configured since both route names or hard-coded paths are valid options.
Additionally, I added unit tests for LogoutListener and functional tests for both CSRF-enabled form logins and the new logout listener work.
Kudo's to @henrikbjorn for taking the time to document CSRF validation for form login listeners (see [here](http://henrik.bjrnskov.dk/symfony2-cross-site-request-forgery/)). The [Logout CSRF Protection](http://www.yiiframework.com/wiki/190/logout-csrf-protection/) article on the Yii Framework wiki was also helpful in drafting this.
---------------------------------------------------------------------------
by jmikola at 2011-12-31T07:50:31Z
Odd that Travis CI reported a build failure for PHP 5.3.2, but both 5.3 and 5.4 passed: http://travis-ci.org/#!/jmikola/symfony/builds/463356
My local machine passes as well.
---------------------------------------------------------------------------
by jmikola at 2012-02-06T20:05:30Z
@schmittjoh: Please let me know your thoughts on the last commit. I think it would be overkill to add support for another handler service and/or error page just for logout exceptions.
Perhaps as an alternative, we might just want to consider an invalid CSRF token on logout imply a false return value for `LogoutListener::requiresLogout()`. That would sacrifice the ability to handle the error separately (which a 403 response allows us), although we could still add logging (currently done in ExceptionListener).
---------------------------------------------------------------------------
by jmikola at 2012-02-13T17:41:33Z
@schmittjoh: ping
---------------------------------------------------------------------------
by fabpot at 2012-02-14T23:36:22Z
@jmikola: Instead of merging symfony/master, can you rebase?
---------------------------------------------------------------------------
by jmikola at 2012-02-15T00:00:49Z
Will do.
---------------------------------------------------------------------------
by jmikola at 2012-02-15T00:05:48Z
```
[avocado: symfony] logout-csrf (+9/-216) $ git rebase master
First, rewinding head to replay your work on top of it...
Applying: [SecurityBundle] Add functional test for form login with CSRF token
Applying: [Security] Refactor LogoutListener constructor to take options
Applying: [Security] Allow LogoutListener to validate CSRF tokens
Applying: [SecurityBundle] Templating helpers to generate logout URL's with CSRF tokens
Applying: [SecurityBundle] Fix execution of functional tests with different names
Applying: [SecurityBundle] Use assertCount() in tests
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Applying: [Security] Use LogoutException for invalid CSRF token in LogoutListener
[avocado: symfony] logout-csrf (+7) $ git st
# On branch logout-csrf
# Your branch and 'origin/logout-csrf' have diverged,
# and have 223 and 9 different commit(s) each, respectively.
#
nothing to commit (working directory clean)
[avocado: symfony] logout-csrf (+7) $
```
After rebasing, my merge commits disappeared. Is this normal?
---------------------------------------------------------------------------
by stof at 2012-02-15T00:15:07Z
Are you sure they disappeared ? Diverging from the remote branch is logical (you rewrote the history and so changed the commit id) but are you sure it does not have the commits on top of master ? Try ``git log master..logout-scrf``
If your commut are there, you simply need to force the push for the logout-csrf branch (take care to push only this branch during the force push to avoid messing all others as git won't warn you when asking to force)
---------------------------------------------------------------------------
by stof at 2012-02-15T00:17:09Z
ah sorry, you talked only about the merge commit. Yeah it is normal. When reapplying your commits on top of master, the merge commit are not kept as you are reapplying the changes linearly on top of the other branch (and deleting the merge commit was the reason why @fabpot asked you to rebase instead of merging btw)
---------------------------------------------------------------------------
by jmikola at 2012-02-15T00:18:00Z
The merge commits are not present in `git log master..logout-csrf`. Perhaps it used those merge commits when rebasing, as there were definitely conflicts resolved when I originally merged in symfony/master (@fabpot had made his own changes to LogoutListener).
I'll force-push the changes to my PR brange. IIRC, GitHub is smart enough to preserve inline diff comments, provided they were made through the PR and not on the original commits.
---------------------------------------------------------------------------
by jmikola at 2012-02-15T00:19:38Z
That worked well. In the future, I think I'll stick to merging upstream in and then rebasing afterwards. Resolving conflicts is much easier during a merge than interactive rebase.
---------------------------------------------------------------------------
by jmikola at 2012-02-23T18:46:13Z
@fabpot @schmittjoh: Is there anything else I can do for this PR? I believe the exception was the only outstanding question (see: [this comment](https://github.com/symfony/symfony/pull/3007#issuecomment-3835716)).
On the advice of @schmittjoh, this commit adds a LogoutException class for use by LogoutListener if the CSRF token is invalid.
The handling in the Security component's ExceptionListener is modeled after AccessDeniedException, which gets wrapped in an AccessDeniedHttpException in the absence of handler service or error page (I didn't think it was appropriate to re-use those for LogoutException).
This adds several new options to the logout listener, modeled after the form_login listener:
* csrf_parameter
* intention
* csrf_provider
The "csrf_parameter" and "intention" have default values if omitted. By default, "csrf_provider" is empty and CSRF validation is disabled in LogoutListener (preserving BC). If a service ID is given for "csrf_provider", CSRF validation will be enabled. Invalid tokens will result in an InvalidCsrfTokenException being thrown before any logout handlers are invoked.
Commits
-------
4a797df Oracle issues
81d73bb Oracle issues
2316b21 Oracle issues
315bfc4 just update
b20b15b Oracle 10 issues
Discussion
----------
Oracle issues
updated with some adjustments required by stof
---------------------------------------------------------------------------
by fabpot at 2011-12-13T07:24:12Z
@schmittjoh: Can you have a look at this PR?
---------------------------------------------------------------------------
by fabpot at 2011-12-24T08:19:37Z
Can you squash your commit before I merge your PR? Thanks.
As discussed on IRC meetings and in PR #2669 I came up with implementation.
This is option2, I think more elegant.
BC break: yes
Feature addition: no/feature move
Symfony2 test pass: yes
Symfony2 test written: yes
Todo: feedback needed
Commits
-------
1e370d7 typo fix
93d8d44 added some more infos about Config
27efd59 added READMEs for the bridges
34fc866 cosmetic tweaks
d6af3f1 fixed README for Console
6a72b8c added basic README files for all components
Discussion
----------
added basic README files for all components and bridges
heavily based on http://fabien.potencier.org/article/49/what-is-symfony2 and the official Symfony2 documentation
---------------------------------------------------------------------------
by jmikola at 2011/11/03 13:36:07 -0700
Great work. For syntax highlighting on the PHP snippets, you could add "php" after the three backticks.
---------------------------------------------------------------------------
by lsmith77 at 2011/11/03 13:41:29 -0700
done
---------------------------------------------------------------------------
by stealth35 at 2011/11/03 13:49:31 -0700
Nice job, but you also need to add `<?php`
ex :
``` php
<?php
use Symfony\Component\DomCrawler\Crawler;
$crawler = new Crawler();
$crawler->addContent('<html><body><p>Hello World!</p></body></html>');
print $crawler->filter('body > p')->text();
```
---------------------------------------------------------------------------
by lsmith77 at 2011/11/03 13:56:57 -0700
done
---------------------------------------------------------------------------
by ericclemmons at 2011/11/03 19:57:57 -0700
@lsmith77 Well done! This makes consumption of individual components that much easier, *especially* now that `composer.json` files have been added.
---------------------------------------------------------------------------
by lsmith77 at 2011/11/04 01:18:23 -0700
ok .. fixed the issues you mentioned @fabpot
---------------------------------------------------------------------------
by lsmith77 at 2011/11/11 15:00:27 -0800
@fabpot anything else left? seems like an easy merge .. and imho there is considerable benefit for our efforts to spread the word about the components with this PR merged.
---------------------------------------------------------------------------
by drak at 2011/11/11 18:54:13 -0800
You know, it might be a nice idea to put a link to the documentation for each component if there is some at symfony.com
---------------------------------------------------------------------------
by lsmith77 at 2011/11/12 00:59:14 -0800
i did that in some. but i might have missed a few places.
On 12.11.2011, at 03:54, Drak <reply@reply.github.com> wrote:
> You know, it might be a nice idea to put a link to the documentation for each component if there is some at symfony.com
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/2561#issuecomment-2715762
---------------------------------------------------------------------------
by breerly at 2011/11/21 10:28:36 -0800
Pretty excited with this.
---------------------------------------------------------------------------
by dbu at 2011/11/24 00:02:50 -0800
is there anything we can help with to make this ready to be merged?
---------------------------------------------------------------------------
by lsmith77 at 2011/12/18 02:39:23 -0800
@fabpot: seriously .. if you are not going to deliver something "better" and don't provide a reason what is wrong with this .. then its beyond frustrating. i obviously do not claim that these README's are perfect (and certainly still no replacement for proper documentation), but I do claim that in their current form they are a radical step forward to potential users of the Symfony2 components.
This changes helps the common use case of fetching the current user and better complies with the Law of Demeter (http://en.wikipedia.org/wiki/Law_of_Demeter).
Before (still works):
$token = $context->getToken();
$user = $token ? $token->getUser() : null;
After:
$user = $context->getUser();
Commits
-------
7c1cbb9 [Config] Use LoaderResolverInterface for type-hinting
48b084e fixed typo
8ad94fb merged branch hhamon/doctrine_bridge_cs (PR #2775)
240796e [Bridge] [Doctrine] fixed coding conventions.
7cfc392 check for session before trying to authentication details
648fae7 merged branch proofek/domcrawlerform-radiodisabled (PR #2768)
3976b7a [DoctrineBridge] fixed CS
9a04783 merged branch beberlei/SecurityEntityRepositoryIdentifierFix (PR #2765)
3c83b89 [DoctrineBridge] Catch user-error when the identifier is not serialized with the User entity.
36c7d03 Fixed GH-2720 - Fix disabled atrribute handling for radio form elements
Discussion
----------
[Config] Use LoaderResolverInterface for type-hinting
```
Bug fix: no
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: -
```
I've listed this as a BC break because we're changing the argument type-hint, but I think it's unlikely to affect anyone.
