Commits
-------
e06cea9 [HttpFoundation] Cookie values should not be restricted
Discussion
----------
[HttpFoundation] Cookie values should not be restricted
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
The restriction I removed makes no sense IMO because we do not use setrawcookie() to send cookies. setrawcookie() does throw a warning when the cookie value contains incorrect characters, but not setcookie(). The latter will just urlencode() the value so it becomes valid. This is also what is done by `Cookie::__toString`, so this could be used in combination with header() to just send raw cookies that are valid, even with values that are invalid in their decoded form.
PHP urldecodes cookies on input, so it all works fine.
Commits
-------
36c7d03 Fixed GH-2720 - Fix disabled atrribute handling for radio form elements
Discussion
----------
Fixed GH-2720 - Fix disabled atrribute handling for radio form elements
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: - GH-2720
I wasn't really sure about the correct approach. This one is very minimalistic and following the existing concept of not duplicating nodes of the same name, but only storing multiple values for the same node. If you think that should be changed, let me know. Hints appreciated.
Thanks
Commits
-------
63e2a99 [CssSelector] Fixed Issue for XPathExprOr: missing prefix in string conversion
Discussion
----------
[CssSelector] Issue for XPathExprOr: missing prefix in string conversion
Hi there,
I created a small and dumb test for the issue. I looked at the original implementation and i think the problem is, that private properties are used in the parent class for xPathExprOr. so that the prefix cannot be accessd with $this->prefix in XPathExprOr
However I think the distribution for the prefix should be put in the parts of the or-sub-expressions the way it is shown in the test.
Hope this helps.
Best regards
Philipp
Bug fix: yes
Feature add: no
Symfony2 tests pass: yes
Symfony2 tests added: yes
In general without this exception generated by php dumper container class, will cause PHP fatal error, bacause method call will look like this: `$instance->(/* arguments*/);`.
When controller is a Closure ControllerResolver::getArguments tries to
make a ReflectionMethod of the __invoke method. But because it's an
internal function, the parameters method isDefaultValueAvailable will
return always false, even if isOptional return true.
Commits
-------
11b6156 updated unittest
a931e21 get correct client IP from X-forwarded-for header
Discussion
----------
[HttpFoundation] Get correct client IP when using trusted proxy (Varnish)
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
Note: This is reopened PR #2686 for 2.0 branch.
If using trusted proxy (Varnish, ...) the client IP must be identified from X-Forwarded-For header. The header has de-facto standard format:
X-Forwarded-For : client1, proxy1, proxy2,
where the value is a comma+space separated list of IP addresses, the left-most being the farthest downstream client, and each successive proxy that passed the request adding the IP address where it received the request from. See: http://en.wikipedia.org/wiki/X-Forwarded-For
Function getClientIp should return only one client IP, not a list of all nonimportant IPs as it's now. Similar example can be seen in Cake framework: http://api.cakephp.org/view_source/request-handler-component/#line-477
There are many ways how to chose the first IP from X-Forwarded-For header. Any other faster and more reliable way is welcome.
Commits
-------
b6bf018 tweaked error handling for the forward compatibility
dd606b5 added note about the purpose of this class
c1426ba added locale handling forward compatibility
10eed30 added MessageDataCollector forward compatibility
Discussion
----------
Forward compat
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2522
Commits
-------
78e9b2f [Form] Fixed textarea_widget (W3C standards)
Discussion
----------
[Form] Fixed textarea_widget (W3C standards)
Textarea widget included the "pattern" attribute but is not valid by W3C standards.
(See PR 2666 - New PR because rebase inside the 2.0 branch)
---------------------------------------------------------------------------
by fabpot at 2011/11/18 09:01:41 -0800
@hlecorche: Thanks for your work on this issue. Can you update the unit tests to be sure that this case is covered? If you're not comfortable with this, just tell me and I will do it myself
---------------------------------------------------------------------------
by hlecorche at 2011/11/19 02:51:06 -0800
@fabpot: I did'nt commited because I am not sure. I changed the "tests/Symfony/Tests/Component/Form/AbstractLayoutTest.php" file :
public function testTextarea()
{
$form = $this->factory->createNamed('textarea', 'na&me', 'foo&bar', array(
'property_path' => 'name',
'pattern' => 'foo',
));
$this->assertWidgetMatchesXpath($form->createView(), array(),
'/textarea
[@name="na&me"]
[not(@pattern)]
[.="foo&bar"]
'
);
}
Is it correct?
Commits
-------
36cebf0 Fix infinite loop on circullar reference in form factory
Discussion
----------
[BugFix][Form]Throw exception on form name circulal ref
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Closes: #2673
When FormType method `getName()` returns the same value as `getParent()` we're asking about trouble, and land into infinite loop.
Commits
-------
57e1aeb Fixed undefined index notice in readProperty() method (PropertyPath)
Discussion
----------
Fixed undefined index notice in readProperty() method (PropertyPath)
Hi,
For some reasons, I get `notice` errors on `readProperty()` with Propel:
Notice: Undefined index: 0 in /Users/william/projects/Propel/testProjects/symfony2/vendor/symfony/src/Symfony/Component/Form/Util/PropertyPath.php line 284
The `PropelObjectCollection` implements `ArrayAccess`, the `readProperty()` method does not check if the given `index` exists so the `notice` error is thrown. I suppose to check whether the index exists or not has to be added.
Regards,
William
---------------------------------------------------------------------------
by fabpot at 2011/09/27 23:42:07 -0700
The patch is probably not what we want to do. First, I suppose that you are not creating the propertyPath by hand. If that is the case, we need to understand why the property path does not exist. Then, even if we might want to check the existence of the index, if it does not exist, we should probably throw an exception instead of just ignoring the problem.
---------------------------------------------------------------------------
by willdurand at 2011/09/28 01:14:49 -0700
My bad. This is a Propel bug due to `ArrayObject`. It throws a notice error if the index is not found in `offsetGet()` which is wrong according to the `ArrayAccess` interface. If the index is not found, we have to return `null`.
@fabpot Are you agree with that (for the `null` value) ?
---------------------------------------------------------------------------
by fabpot at 2011/09/28 01:17:09 -0700
My point is that it should never happen under normal circumstances.
---------------------------------------------------------------------------
by willdurand at 2011/09/28 01:23:55 -0700
@fabpot Not sure to get it.
The fact is that it tries to get the value (`getValue()`) of a fresh object, just added to the `collection` when I'm submitting a form with a `CollectionType` and a new entry in it.
I mean it tries to get this new object (not yet persisted, not yet in the collection) in the collection (`getValue()` -> `readProperty()`) which implements `ArrayAccess` but this object cannot be in the collection at this time.
Am I wrong ?
And, without this notice error thrown by Propel, I probably never opened this issue...
---------------------------------------------------------------------------
by willdurand at 2011/09/29 06:40:34 -0700
@fabpot: you can try this example: http://www.propelorm.org/cookbook/symfony2/mastering-symfony2-forms-with-propel.html#manytomany_relations in order to make your own tests. Will it be enough?
As I said, it throws a weird notice for the reasons above.
---------------------------------------------------------------------------
by jaugustin at 2011/10/04 12:58:10 -0700
any news on this ?
@fabpot did you have time to look at the test case ?
---------------------------------------------------------------------------
by cedriclombardot at 2011/11/09 14:29:42 -0800
@fabpot: can we have news about this ?
Commits
-------
79ae3fc [Form] fixed radio and checkbox when data is not bool
Discussion
----------
[Form] fixed checkbox view
The checkbox view was being built based on app data, not client data. This fixes it.
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
---------------------------------------------------------------------------
by fabpot at 2011/11/16 13:31:09 -0800
`RadioType` suffers from the same problem, no?
---------------------------------------------------------------------------
by kriswallsmith at 2011/11/16 13:32:50 -0800
Yeah, I'll fix that too.
---------------------------------------------------------------------------
by kriswallsmith at 2011/11/16 13:43:29 -0800
Updated to include `RadioType`.
Commits
-------
8399574 Fixes a small php doc issue of Symfony\Component\Console\Command\Command::setDefinition()
Discussion
----------
Fixes a small php doc issue of Symfony\Component\Console\Command\Command::setDefinition()
Have setDefinition() accept InputDefinition instead of Definition.
Commits
-------
29e12af [TwigBundle] Extract output buffer cleaning to method
ed1a6c2 [TwigBundle] Do not clean output buffering below initial level
Discussion
----------
[TwigBundle] Do not clean output buffering below initial level
This resulted in issues with PHPUnit 3.6, which will buffer all output and clean them in the end. Since
we cleaned their buffer, the subsequent clean would raise a warning. This is documented in [issue 390](https://github.com/sebastianbergmann/phpunit/issues/390) of
the PHPUnit tracker.
Closes#2531.
This also affects FOSRestBundle's ExceptionController /cc @lsmith.
---------------------------------------------------------------------------
by fabpot at 2011/11/11 07:33:24 -0800
I have a similar fix locally but I have not merged it yet as it looks a bit dirty (but I've not a better idea yet). Anyway, your PR is better than mine as you've added some unit tests already.
Commits
-------
2582fcb Added tests for string fix in DateTimeToArrayTransformer (8351a11286).
8351a11 Added check for array fields to be integers in reverseTransform method. This prevents checkdate from getting strings as arguments and throwing incorrect ErrorException when submitting form with malformed (string) data in, for example, Date field. #2609
Discussion
----------
Fix for #2609
Second take for fix for #2609, hope it's ok now. Tests are failing without my fix and passing with it.
This resulted in issues with PHPUnit 3.6, which will buffer all output and clean them in the end. Since
we cleaned their buffer, the subsequent clean would raise a warning. This is documented in issue 390 of
the PHPUnit tracker.
Closes#2531.
Commits
-------
f9befb6 Remove only the security token instead of the session cookie.
348bccb Clear session cookie if user was deleted, is disabled or locked to prevent infinite redirect loops to the login path (fixes#1798).
Discussion
----------
Fix for issue 1798
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Clear session cookie if user was deleted, is disabled or locked to prevent infinite redirect loops to the login path (fixes#1798).
---------------------------------------------------------------------------
by snc at 2011/11/01 04:01:49 -0700
@stof I have changed the code so that it only removes the token... do we still need any hook support?
---------------------------------------------------------------------------
by stof at 2011/11/01 04:07:17 -0700
well, the hook is for your own use case but it would be for 2.1 only anyway, not for 2.0
---------------------------------------------------------------------------
by snc at 2011/11/07 15:11:52 -0800
Now that #2414 is merged to 2.1, this could be simplified for the master branch...
Commits
-------
7346896 Changed Serialized#supportsNormalization to PRIVATE
e851efc Updated SerializerTest with "normalizeTraversable" & "testNormalizeGivesPriorityToInterfaceOverTraversable"
d789f94 Serializer#normalize gives precedence to objects that support normalization
9e6ba9a Added protected Serializer#supportsNormalization
Discussion
----------
[Serializer] `normalize` should use supported normalizer before Traversable
Bug fix: yes
Feature addition: no
Backwards compatibility break: no (discussion needed)
Symfony2 tests pass: yes
Fixes the following tickets: #2295
**Same as PR #2539, except rebased onto `2.0`**
Should I abstract out a `supportsDenormalization` function just for symmetry?
Commits
-------
ffa537c replace occurences of "an UserInteface" with "a UserInterface"
Discussion
----------
replace occurences of "an UserInteface" with "a UserInterface"
Commits
-------
89cd64a Set error code when number cannot be parsed. Fixes#2389
Discussion
----------
Set error code when number cannot be parsed in StubNumberFormatter
The stub implementation of NumberFormatter never sets an error code and instead always returns the "no-error" code. This causes unexpected results when a transformer gives it bad arguments, such as transforming the input value to boolean false and causing exceptions further down the line, as in #2389.
Instead, it should set an error code when appropriate and return it when requested so that a TransformationFailedException can be raised and the input value left unaltered.
There may be other instances where an error should be set. This covers the common use case of non-numeric input.
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2389
Commits
-------
c9d05d7 Let NumberFormatter handle integer type casting
Discussion
----------
Let NumberFormatter handle integer type casting
The integer to localised string transformer is currently casting everything it gets to an integer, even if it is not a number. This responsibility should be passed off to NumberFormatter.
Partially addresses #2389 by not mistakenly typecasting a boolean false into an integer 0
---------------------------------------------------------------------------
by mrtorrent at 2011/10/30 15:04:28 -0700
Apologies, forgot the template:
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2389 (partial)
While Definition::getArgument() could be used to fetch replaced values, it relied upon bad comparison logic (e.g. "index_1" > 1). Additionally, storing original arguments and replacements in the same array makes Definition::getArguments()'s bounds check unreliable. A single argument and its replacement would count twice, allowing getArgument(2) to pass the bounds check and result in an array index error.
With this new method, fetching of replacement arguments is more straightforward and bounds checking functions as it should.
Commits
-------
d3f137b cosmetic tweak
2877883 anything in front of ;q= is part of the mime type, anything after may be ignored
Discussion
----------
[HttpFoundation] fix splitHttpAcceptHeader() parsing of parameters
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
anything in front of ;q= is part of the mime type, anything after may be ignored
see http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.1
---------------------------------------------------------------------------
by lsmith77 at 2011/10/09 04:00:12 -0700
i must admit .. i am not 100% that my implemention is correct either .. but i am sure the current one isn't.
---------------------------------------------------------------------------
by lsmith77 at 2011/10/09 07:57:33 -0700
@fabpot: I am also not sure if getFormat() should optionally not support matching parameters, aka anything before ``;q=..``
Commits
-------
edfa29b session data needs to be encoded because it can contain non binary safe characters e.g null. Fixes#2067
Discussion
----------
session data needs to be encoded because it can contain non binary safe characters e.g null.
Bug fix: yes
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: #2067
I'm marking this as a compatibility break because session table should be cleared and even if not cleared all currently logged in users will be logged out.
---------------------------------------------------------------------------
by mvrhov at 2011/10/11 12:52:25 -0700
P.S. I know there was a talk about doctrine based session storage but I cannot find this in core. It probably has the same problem.
---------------------------------------------------------------------------
by eventhorizonpl at 2011/10/11 14:34:08 -0700
Thanks for tracking down and fixing this issue!
Best regards,
Michal
---------------------------------------------------------------------------
by stof at 2011/10/11 16:24:18 -0700
@mvrhov The Doctrine based storage is only available in master, not in 2.0
Commits
-------
808088a added the ability to use dot and single quotes in the keys and values
Discussion
----------
[2.0][Bugfix][DependencyInjection] added the ability to use dot and single quotes in the keys and values
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
We can not set a specific combination of dots and single quotes in the values and keys of the arguments. I.E.
```xml
<?xml version="1.0" ?>
<container xmlns="http://symfony.com/schema/dic/services"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/dic/serviceshttp://symfony.com/schema/dic/services/services-1.0.xsd">
<services>
<service id="key_escaper" class="MyNamespace\MyClass">
<call method="setCollection">
<argument type="collection">
<argument key="only dot">.</argument>
<argument key="concatenation as value">.''.</argument>
<argument key="concatenation from the start line">''.</argument>
<argument key=".">is the same problem for the keys?</argument>
</argument>
</call>
</service>
</services>
</container>
```
As a result we have such a dump:
```php
<?php
class appDevDev_formapro1DebugProjectContainer extends Container
{
protected function getKeyEscaperService()
{
$this->services['key_escaper'] = $instance = new \MyNamespace\MyClass();
$instance->setCollection(array('only dot' => , 'concatenation as value' => '.\'\, 'concatenation from the start line' => '\'\, => 'is the same problem for the keys?'));
return $instance;
}
}
```
Commits
-------
8bd0e42 [Form] Use proper parent (text) for EmailType and TextareaType
Discussion
----------
[2.0][Form] Use proper parent (text) for EmailType and TextareaType
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Commits
-------
95049ef [Form] Added type check to `ScalarToChoiceTransformer`
Discussion
----------
[2.0][Form] Added type check to ScalarToChoiceTransformer
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Commits
-------
731b28b [composer] add missing deps for FrameworkBundle
9c8f100 [composer] change ext/intl to the new ext-intl syntax
d535afe [composer] fix monolog-bridge composer.json, add more inter-component deps
9ade639 [composer] add composer.json
Discussion
----------
Composer
This PR adds a composer.json file for [composer](https://github.com/composer/composer) ([more info](packagist.org/about-composer)).
For discussion you can also go into #composer-dev on freenode and argue with naderman, seldaek and everzet.
---------------------------------------------------------------------------
by naderman at 2011/09/26 15:51:51 -0700
You haven't entered any keywords, they might come in handy when searching for packages on packagist.
But really this is just a +1 ;-)
---------------------------------------------------------------------------
by stof at 2011/09/26 16:12:21 -0700
See my comments on your previous (non-rebased) commit: f1c0242b5a
---------------------------------------------------------------------------
by igorw at 2011/09/27 00:04:36 -0700
Following dependencies do not have a composer.json yet: Twig, Doctrine (orm, dbal, common), swiftmailer.
Also missing from the standard edition: assetic, twig-extensions, jsm-metadata, SensioFrameworkExtraBundle, JMSSecurityExtraBundle, SensioDistributionBundle, SensioGeneratorBundle, AsseticBundle.
The point is, those can be added later on. Having the components composerized is already a leap forward. Also, doctrine depends on some symfony components, we've got to start somewhere.
---------------------------------------------------------------------------
by Seldaek at 2011/09/27 00:36:41 -0700
Also, just for information, the plan is to have `symfony/framework-bundle` be the "framework", with all dependencies to doctrine etc, though we should really only have strict requirements in there, and then in symfony-standard we ship a composer.json that requires the framework-bundle, doctrine-orm and things like that that are not essential to core. Otherwise people don't have a choice about what they use anymore.
Just a comment btw, the json is invalid, all / should be escaped. However json_decode is nice enough to parse those without complaining, browsers do too, even Crockford's json2.js does, so I'm not sure if we should privilege readability over strictness, since it seems nobody really cares about this escaping.
---------------------------------------------------------------------------
by igorw at 2011/09/27 00:41:39 -0700
So, I've implemented all of @stof's suggestions, except (for reasons stated above):
* doctrine to DoctrineBundle
* swiftmailer to SwiftmailerBundle
* twig to TwigBundle
* doctrine-common to Validator
* FrameworkBundle (what exactly does it depend on?)
---------------------------------------------------------------------------
by stof at 2011/09/27 00:52:31 -0700
@igorw at least HttpKernel, Routing, Templating, EventDispatcher, Doctrine Common (annotations cannot be disabled), Translator, Form (optional), Validator (optional), Console (optional). See the service definitions to see the others
@Seldaek FrameworkBundle does not depend on Doctrine, except for Common
---------------------------------------------------------------------------
by beberlei at 2011/09/27 03:15:34 -0700
What does the symfony/ or ext/ prefix control in composer?
---------------------------------------------------------------------------
by Seldaek at 2011/09/27 03:33:52 -0700
symfony/ is just the (mandatory) vendor namespace. Also ext/ has been renamed to ext- now, so it's not in any vendor, and should avoid potential issues.
---------------------------------------------------------------------------
by beberlei at 2011/09/27 05:07:03 -0700
@Seldaek Mandatory? So every package name is "vendor/package"? I like that because previously i thought package names are not namespaced, and thus clashes could occur between different communities easily.
---------------------------------------------------------------------------
by Seldaek at 2011/09/27 05:16:20 -0700
@beberlei: Mandatory. As of yesterday http://packagist.org/ will tell you you have an invalid package name if there's no slash in it. See 1306d1ca82 (diff-3)
Commits
-------
908a7a3 [HttpFoundation] Fix bug in clearCookie/removeCookie not clearing cookies set with a default '/' path, unless it was explicitly specified
Discussion
----------
[HttpFoundation] Fix bug in clearCookie/removeCookie not clearing cookies
[HttpFoundation] Fix bug in clearCookie/removeCookie not clearing cookies set with a default '/' path, unless it was explicitly specified
---------------------------------------------------------------------------
by Seldaek at 2011/08/02 10:31:44 -0700
The reason is that Cookie::__construct defaults to '/' btw, so if you don't specify it, and then call clearCookie without specifying again, the paths don't match.
---------------------------------------------------------------------------
by Koc at 2011/08/07 00:06:13 -0700
I think that correctrly use base path. Is it possible?
For example we have 2 apps
* site.com/app1/index.php
* site.com/app2/index.php
and app2 will remove cookies of app1
---------------------------------------------------------------------------
by Seldaek at 2011/08/07 02:58:10 -0700
IMO if people want that they should specify the path manually, by default cookies are always set for the entire host and I think it should stay like that.
---------------------------------------------------------------------------
by Koc at 2011/08/07 04:26:47 -0700
It is hard to specify path manually everywhere when set/remove cookies.
---------------------------------------------------------------------------
by Seldaek at 2011/09/27 07:01:43 -0700
@fabpot: ping? You said this was ok, but it was never merged.
Commits
-------
022a9a7 [Security] Make saving target_path extendible
Discussion
----------
[Security] Make saving target_path extendible
The problem lies in how Security component handles ``target_path`` - the latest request URI is always stored. This can lead to problems in following scenarios:
a) The response type of the request is not HTML (think JSON, XML ..)
b) The URI matches a route that does not listen to HTTP GET
I opened a [PR](https://github.com/symfony/symfony/pull/604) months ago, to partly solve scenario A, which did not make it. Now I am proposing a different solution - user can extend ``ExceptionListener`` and override the logic behind setting the ``target_path`` to match his precise needs.
In my simplified scenario, I would be using:
```
protected function setTargetPath(Request $request)
{
if ($request->isXmlHttpRequest() || 'GET' !== $request->getMethod()) {
return;
}
$request->getSession()->set('_security.target_path', $request->getUri());
}
```
@Seldaek, @schmittjoh, @lsmith77, thoughts?
---------------------------------------------------------------------------
by Seldaek at 2011/09/21 02:37:02 -0700
Seems like a better solution for flexibility's sake. Would be quite awesome if you could add a cookbook entry to symfony/symfony-docs about this, otherwise I'm afraid we'll have to explain it over and over again :)
---------------------------------------------------------------------------
by helmer at 2011/09/21 03:38:57 -0700
[Cookbook](b22c5e666e) entry done. Perhaps though I rushed ahead ..
---------------------------------------------------------------------------
by Seldaek at 2011/09/21 03:52:01 -0700
Thanks. You can already do a pull request against symfony-docs, just reference this pull request in it so it's not merged before this is merged.
