The regexp in MoneyType doesn't work if currency format has no decimal
(like JPY) and doesn't work either if the currency symbol is unicode
This change fixes both issues and adds a unit test
Commits
-------
ed9c348 Authentication(Success|Failure)Handler can now return null
Discussion
----------
[Security] Authentication(Success|Failure)Handler can now return null
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Related to the following ticket: #838
[![Build Status](https://secure.travis-ci.org/odolbeau/symfony.png)](http://travis-ci.org/odolbeau/symfony)
Correct me if I'm wrong but for now it's not possible to handle Authentication(Success|Failure) in some case only (for example to handle XmlHttpRequest on login form).
With this change, if the handler return null, the default behavior is kept.
---------------------------------------------------------------------------
by stof at 2012-01-24T17:28:49Z
👍
Commits
-------
746170b Make method non static
c3f637b PATCH support and tests for DELETE support
2dd4bf1 Support for PATCH method in forms
Discussion
----------
[form] Support for PATCH method in forms
Bug fix: no
Feature addition: yes
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
Adds support for PATCH method. Accidentally I refactored the test RequestTest a bit to verify PUT, DELETE and PATCH and not only PUT.
---------------------------------------------------------------------------
by lanthaler at 2012-01-25T13:26:26Z
Why do you classify this as something that breaks backwards compatibility? I don't think it breaks anything.
---------------------------------------------------------------------------
by lstrojny at 2012-01-25T13:28:28Z
It’s not that relevant here, but before using patch throws an exception which it no longer does.
---------------------------------------------------------------------------
by lanthaler at 2012-01-25T13:31:35Z
Oh OK..
---------------------------------------------------------------------------
by vicb at 2012-01-25T13:36:39Z
Maybe you can update the [DomCrawler](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/DomCrawler/Form.php) at the same time ?
---------------------------------------------------------------------------
by lstrojny at 2012-01-25T13:55:21Z
Done
Commits
-------
43e0db5 [DomCrawler] Add support for multivalued form fields (fix#1579, #3012)
Discussion
----------
[DomCrawler] Support for multivalued fields
This is a tentative fix for #1579 by @kriswallsmith, also see #3012 for more info.
Any feedback is appreciated.
---------------------------------------------------------------------------
by vicb at 2012-01-05T08:44:51Z
@stof thanks for the valuable feedback I think most of it should be implemented should we use this solution.
The one thing I don't agree is PSR-0, I don't want this class to be public, that's is just a "private" helper class.
There are also missing type hints in the helper class, that should be added.
---------------------------------------------------------------------------
by alessandro1997 at 2012-01-05T10:05:15Z
Well, @vicb, I think it's up to the developer to not use "private" classes. Just write it in the documentation. But declaring two classes in the same file would be a big violation of the standards.
---------------------------------------------------------------------------
by vicb at 2012-01-05T11:28:53Z
What "standard"s ?
PSR-0 is about auto-loading, I don't want/need this to be autoloaded.
Sf coding standards ? Well relying on a developer reading the doc is more error prone than the current implementation. I sometimes favor pragmatism over theory.
edit: I am not trying to say I am right here but only that I don't see any added value in moving the helper class to a dedicated file. I appreciate any feedback, really.
---------------------------------------------------------------------------
by fabpot at 2012-01-06T11:55:09Z
FYI, we already have such a "private" class in https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Security/Http/Firewall/DigestAuthenticationListener.php#L135
---------------------------------------------------------------------------
by vicb at 2012-01-06T16:36:04Z
@alessandro1997 if you need an example on why it is not safe to rely on developers reading comments, see #2892
---------------------------------------------------------------------------
by vicb at 2012-01-09T22:19:52Z
@fabpot I am waiting for your feedback on the [proposed API](https://github.com/symfony/symfony/pull/3035/files#L1R57) before finishing this PR.
---------------------------------------------------------------------------
by drak at 2012-01-10T05:12:16Z
@fabpot
> FYI, we already have such a "private" class in https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Security/Http/Firewall/DigestAuthenticationListener.php#L135
Why on is that necessary, it could just be another class file in the namespace. Unless you are making some kind of forward compatibility, e.g. for with a new class in PHP 5.4 then I see no reason to do that.
---------------------------------------------------------------------------
by vicb at 2012-01-10T07:40:32Z
What would be a good reason not to allow "private" classes ?
If the Sf coding standards are the only good reason let's change them then.
[Java](http://stackoverflow.com/questions/968347/can-a-java-file-have-more-than-one-class) and [ActionScript3](http://livedocs.adobe.com/flex/3/html/help.html?content=03_Language_and_Syntax_05.html) allow such construction
I would no say any better than the above link on Stack Overflow:
> The purpose of including multiple classes in one source file is to bundle related support functionality (internal data structures, support classes, etc) together with the main public class. Note that it is always ok not to do this -- the only effect is on the readability (or not) of your code.
---------------------------------------------------------------------------
by Tobion at 2012-01-10T09:35:09Z
There are also many private classes in the test cases.
---------------------------------------------------------------------------
by stof at 2012-01-10T13:29:08Z
@Tobion for tests, it is logical because there is no autoloader for the test classes.
---------------------------------------------------------------------------
by vicb at 2012-01-10T13:31:53Z
@stof by definition you do not want a "private" class to be autoloaded anyway.
---------------------------------------------------------------------------
by alessandro1997 at 2012-01-10T14:11:42Z
Sure, but what you're doing here is just making instantiating the class a bit more difficult. If a stubborn developer wants to use it, then he (or her) can include the file manually or autoload the "main class".
PHP does NOT have support for private/inner classes, and, until it does, all classes should be istantiable normally.
---------------------------------------------------------------------------
by stof at 2012-01-10T14:23:30Z
@vicb what about someone wanting to serialize the object ? (well, serializing is not the issue. unserializing is)
---------------------------------------------------------------------------
by vicb at 2012-01-10T14:57:52Z
@alessandro1997 you are absolutely right, it's not meant to be instantiated from the outside (it's **private**). You could argue the same with private properties & methods (using Reflection). Dead-end.
@stof Is unserializing really an issue as the file would have been loaded already ?
---------------------------------------------------------------------------
by fabpot at 2012-01-22T09:38:13Z
@vicb: I'm fine with the proposed API, but I fail to see why it would be more BC than #3012.
---------------------------------------------------------------------------
by vicb at 2012-01-22T10:06:56Z
For BC I have to check #3012 again but at some point if I remember correctly the public API had changed (not sure about the latest version in your branch)
By introducing the private helper class, it is quite easy to see that the public API is not modified by this PR.
Next steps:
* Stof the code,
* Add/fix phpdoc,
* Add tests for the helper class,
* Add/refactor tests for the `Form` class.
@fabpot if you agree with the above steps it could be ready sometime next week.
---------------------------------------------------------------------------
by fabpot at 2012-01-22T10:21:16Z
The API is perhaps not changed but the behavior will certainly changed. I agree with your steps.
---------------------------------------------------------------------------
by vicb at 2012-01-22T10:45:10Z
Which leads to the question: should we consider this as a change in behavior (2.1) or a bug fix (2.0) ?
_I am thinking of a form with multiple fields named `field[]`_
---------------------------------------------------------------------------
by fabpot at 2012-01-22T11:32:04Z
@vicb: this change should be done on master
---------------------------------------------------------------------------
by vicb at 2012-01-24T07:59:40Z
Should be ready now, let me know when I should squash after review.
---------------------------------------------------------------------------
by fabpot at 2012-01-24T08:18:03Z
@vicb: yes, can you squash your commits?
---------------------------------------------------------------------------
by vicb at 2012-01-24T08:29:58Z
@fabpot done
Commits
-------
cc31a15 tweaked the exception message
3a1699a handle disaled csrf protection in the Twig FormExtension
2a998e0 handle disabled csrf protection in the PHP templating form helper
Discussion
----------
handle disabled csrf protection in the PHP templating form helper
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: ![Build Status](https://secure.travis-ci.org/lsmith77/symfony.png?branch=form_helper_csrf_off)
Fixes the following tickets: -
---------------------------------------------------------------------------
by fabpot at 2012-01-23T17:00:31Z
The same should be done for Twig, no?
---------------------------------------------------------------------------
by lsmith77 at 2012-01-23T17:04:24Z
hmm i guess so .. will apply the change there too .. once someone tells me how to format that ``instanceof`` check :)
---------------------------------------------------------------------------
by lsmith77 at 2012-01-23T17:07:11Z
updated the ``FormExtension`` too
---------------------------------------------------------------------------
by lsmith77 at 2012-01-23T21:28:52Z
hopefully all ready to go now ..
---------------------------------------------------------------------------
by lsmith77 at 2012-01-24T08:12:38Z
@fabpot: don't want to rush you .. but if this could be merged soon, it would make my life easier :)
ad (1): HTML4 "id" attributes are limited to strings starting with a letter and containing only letters, digits, underscores, hyphens, periods and colons.
ad (2): Property paths contain three special characters needed for correct parsing: left/right bracket and period.
The rules for form naming are:
* Names may start with a letter, a digit or an underscore. Leading digits or underscores will be stripped from the "id" attributes.
* Names must only contain letters, digits, underscores, hyphens and colons.
* Root forms may have an empty name.
Solves #1919 and #3021 on a wider scope.
Commits
-------
e37783f [DoctrineBridge] Refactored the query sanitization in the collector
3b260d2 Refactored the collector to separate the loggers per connection
Discussion
----------
Doctrine collector
Bug fix: no
Feature addition: yes
Backwards compatibility break: yes (for the end user, it will require deleting old profiler data)
Symfony2 tests pass: yes ![Build Status](https://secure.travis-ci.org/stof/symfony.png?branch=doctrine_collector)
This refactors the Doctrine collector to allow implementing doctrine/DoctrineBundle#7
The first commit splits the logging of queries per connection to be able to know which connection was used instead of using a shared stack.
The second commit refactors the sanitation of the parameters to apply the DBAL conversion and then keep the param whenever possible (i.e. when we are sure it is serializable). Such queries will then be explainable in the profiler as we will be able to use the parameters again. Due to the way PDO works, the only cases where we would get an unexplainable queries due to the parameters are queries using a LOB parameter (as it is a resource) or broken queries (passing an object to PDO for instance). And this second case does not make sense to explain the query of course.
---------------------------------------------------------------------------
by stof at 2012-01-23T12:32:16Z
Merging this PR should be synchronized with the DoctrineBundle PR due to the BC break in the collector
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.
Commits
-------
753c067 [FrameworkBundle] added $view['form']->csrfToken() helper
e1aced8 [Twig] added {{ csrf_token() }} helper
Discussion
----------
[Twig] [FrameworkBundle] added CSRF token helper
I've added a templating helper and Twig function for generating a CSRF token without the overhead of creating a form.
```html+jinja
<form action="{{ path('user_delete', { 'id': user.id }) }}" method="post">
<input type="hidden" name="_method" value="delete">
<input type="hidden" name="_token" value="{{ csrf_token('delete_user_' ~ user.id) }}">
<button type="submit">delete</button>
</form>
```
```php
<?php
class UserController extends Controller
{
public function delete(User $user, Request $request)
{
$csrfProvider = $this->get('form.csrf_provider');
if (!$csrfProvider->isCsrfTokenValid('delete_user_'.$user->getId(), $request->request->get('_token')) {
throw new RuntimeException('CSRF attack detected.');
}
// etc...
}
}
```
The test that is failing on Travis appears to be unrelated, but I may be wrong?
```
1) Symfony\Bundle\SecurityBundle\Tests\Functional\LocalizedRoutesAsPathTest::testLoginLogoutProcedure with data set #1 ('de')
RuntimeException: OUTPUT:
Catchable fatal error: Argument 3 passed to Symfony\Bundle\FrameworkBundle\Controller\TraceableControllerResolver::__construct() must be an instance of Symfony\Component\HttpKernel\Debug\Stopwatch, instance of Symfony\Bundle\FrameworkBundle\Controller\ControllerNameParser given, called in /tmp/2.1.0-DEV/StandardFormLogin/cache/securitybundletest/appSecuritybundletestDebugProjectContainer.php on line 94 and defined in /home/vagrant/builds/kriswallsmith/symfony/src/Symfony/Bundle/FrameworkBundle/Controller/TraceableControllerResolver.php on line 37
```
---------------------------------------------------------------------------
by pablodip at 2012-01-10T14:18:45Z
As you don't need forms to use the csrf provider, how about putting its service without the form prefix? It could even make sense to put the CsrfProvider as a component since you can use it standalone and in more cases than only forms. It would be a small component though.
---------------------------------------------------------------------------
by Tobion at 2012-01-10T17:54:14Z
I think it would be more clear to generate the token in the controller. Doing so in the template will spread the CSRF intention across template and controller. So I don't think this extension is necessary.
---------------------------------------------------------------------------
by kriswallsmith at 2012-01-10T17:58:14Z
@pablodip I'm open to the idea of a Csrf component. This would be a good place for some nonce classes as well.
@Tobion I disagree. One use case is for a list of users, each with a delete form. Iterating over the users in the controller and generating a token for each, just to iterate over them again in the view is a waste and adds complexity.
---------------------------------------------------------------------------
by Tobion at 2012-01-10T18:05:14Z
I see. But I don't understand why the intention needs to be different for each user to delete. Usually the intention is the same for each form type. I thought this is enough.
---------------------------------------------------------------------------
by kriswallsmith at 2012-01-10T18:06:13Z
Yes, a static intention would suffice.
---------------------------------------------------------------------------
by Tobion at 2012-01-10T18:07:08Z
Then your use case is not valid anymore.
---------------------------------------------------------------------------
by Tobion at 2012-01-10T18:12:25Z
I would suggest to make a cookbook article out of it about how to create a simple form without the form component.
And include such things as validating the result using the validator component and checking the CSRF.
---------------------------------------------------------------------------
by kriswallsmith at 2012-01-10T21:32:50Z
This helper makes it easier to use CSRF protection without a form and we should make it as easy as possible. Spreading the intention across controller and template is not concerning to me. Either way, a cookbook entry is a great idea.
---------------------------------------------------------------------------
by Tobion at 2012-01-10T21:47:12Z
Well, it's just one line more without this helper. So I disagree it makes it really easier when you know how to use the CsrfProvider which is a pre-condition anyway since you must still validate its correctness by hand.
---------------------------------------------------------------------------
by kriswallsmith at 2012-01-13T13:24:15Z
Another use case is when rendering a page with a bunch of simple buttons with different intentions: delete user, delete comment, follow, unfollow... Creating all of these in the controller just leads to spaghetti.
---------------------------------------------------------------------------
by jwage at 2012-01-17T21:55:53Z
👍 lots of use cases for something like this @OpenSky