Commits
-------
f093caa [Form] Fixed invalid value passed to the constraint violation by the MinLength and MaxLength validators
Discussion
----------
[Form] Fixed invalid value passed to the constraint violation by the MinLength and MaxLength validators
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #4573
Todo: -
Commits
-------
eba7dfe Revert "[Form] added a circular reference safeguard for form type"
Discussion
----------
Revert "[Form] added a circular reference safeguard for form type"
This reverts commit ea93e4cafa.
Conflicts:
src/Symfony/Component/Form/FormBuilder.php
src/Symfony/Component/Form/FormFactory.php
Commits
-------
df5bb4a [Form] Unified rendering of errors for nested elements
Discussion
----------
[Form] Unified rendering of errors for nested elements
Bug fix: yes
Feature addition: no
Backwards compatibility break: yes?
Symfony2 tests pass: yes
Fixes the following tickets: #4615
Todo: -
Commits
-------
1345360 [Form] Fixed PropertyPath handling of offsetGet() that returns a constant value
6e1462e [Form] Fixed PropertyPath handling of __get() method that returns a constant
Discussion
----------
[Form] Fixed "Indirect modification.." exceptions in PropertyPath
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #4450, #4535?, #4612
Todo: -
Commits
-------
3462afc Tests for PluralizationRules.
Discussion
----------
[WIP][Translations][tests] Tests for PluralizationRules.
Currently we have no tests PluralizationRules. This patch is an initial one to show we have not enough langcodes in PluralizationRules.
I hope this gets in so others can fix the missing langcodes.
I'm waiting for [RFC [MessageCatalogue*] Add Metadata to MessageCatalogue](https://github.com/symfony/symfony/pull/4399) to get in to continue working on the [[WIP]: Allow Drupal to use Translate component](https://github.com/symfony/symfony/pull/4249).
---------------------------------------------------------------------------
by travisbot at 2012-05-25T14:38:37Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1433558) (merged 3462afc0 into 023dbf80).
---------------------------------------------------------------------------
by drak at 2012-07-01T09:47:05Z
Is there anything pending in this PR?
Commits
-------
28b6557 class_exists was returning fals for the container aware interface
Discussion
----------
class_exists was returning fals for the container aware interface
This test was marked skipped when it should not have been.
Commits
-------
e6b5595 [Form] Fixed display of empty values in DateType, TimeType and DateTimeType if they are not required
14e293f [Form] Refactored processing of the "empty_value" option in DateType
Discussion
----------
[Form] Fixed display of empty values in DateType, TimeType and DateTimeType
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #4596
Todo: -
Commits
-------
dbeff69 [TwigBundle] added support for custom loader paths
Discussion
----------
[TwigBundle] added support for custom loader paths
Before this commit, there was no ability to specify custom
search paths for Twig loader. Lets say we have twig templates
outside bundles directories (parts of the domain logic, not
application) - we want to be able to load them.
This commit adds `loader_paths` parameter to twig config,
which is used to set custom paths to the loader.
---------------------------------------------------------------------------
by travisbot at 2012-06-25T09:50:44Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1699654) (merged dbeff697 into 03c8d4d2).
---------------------------------------------------------------------------
by vicb at 2012-06-26T06:14:30Z
You also need to support xml in the configuration and update the xsd file.
edit: adding some DI unit tests is probably a good idea.
---------------------------------------------------------------------------
by everzet at 2012-06-26T08:49:20Z
@vicb agree, was just a fast stabbing ;)
---------------------------------------------------------------------------
by fabpot at 2012-06-28T14:06:02Z
I'm +1. Can you "finish" the PR?
---------------------------------------------------------------------------
by fabpot at 2012-07-02T13:23:33Z
@everzet If you don't have time, I can do the remaining work.
Commits
-------
b4d7a7e [Component][Finder][SplFileInfo] file_get_contents=>fpassthru
Discussion
----------
[Component][Finder][SplFileInfo] file_get_contents=>fpassthru
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: https://github.com/symfony/symfony/pull/4335/files#r1016152
Todo: -
License of the code: MIT
Documentation PR: -
Commits
-------
040ba8f [Form] Fixed: ChoiceType omits the "empty_value" option if the choices contain an empty element
Discussion
----------
[Form] Fixed: ChoiceType omits the "empty_value" option if the choices contain an empty element
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #3854, #3864
Todo: -
Commits
-------
6d94f3e Ensure there is a session before getting the session id
Discussion
----------
[Form] Ensure there is a session before getting the session id
Solves "The CSRF token is invalid. Please try to resubmit the form" error when a form is generated before the session is started.
---------------------------------------------------------------------------
by fabpot at 2012-07-09T10:23:32Z
Adding a CSRF token only makes sense if you are on a page with a "user". If not (and if you don't use HTTP auth or whatever), then there is no need for a CSRF token.
---------------------------------------------------------------------------
by frosas at 2012-07-09T14:42:40Z
This PR doesn't change any logic on whether a CSRF token is added or not, it just fixes a bug when a token is requested.
Commits
-------
6c9c2ec [ClassCollectionLoader] fixed comment striping on classes in global namespace
Discussion
----------
[ClassCollectionLoader] fixed comment striping on classes in global namespace
Comments aren't striped when the class is in the global namespace, this adds a fake namespace to let the fixNamespaceDeclaration method handling the formating, so we can remove a method.
I've also put dev as minimum policy in composer, otherwise I couldn't install dependencies.
Commits
-------
1fa22d9 [Form] Output a more usable error when PropertyPath has tried to find adders and getters, but failed to find them
Discussion
----------
[Form] Output a more usable error when PropertyPath has tried to find ad...
...ders and getters, but failed to find them
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
I've refactored the writeProperty method of propertypath in order to supply a better error message when writing has failed.
The writeProperty method itself now finds singulars (if a singular was not passed) for the private findAdderAndRemover method which allowed for some duplicate code to be removed and since the writeProperty now holds this data, it can provide a more verbose exception message.
---------------------------------------------------------------------------
by bschussek at 2012-07-09T13:54:35Z
Apart from the typo this PR looks good.
---------------------------------------------------------------------------
by Burgov at 2012-07-09T14:01:04Z
fixed&squashed
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: [![Build Status](https://secure.travis-ci.org/asm89/symfony.png?branch=refactor-authentication-success-handling)](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: [![Build Status](https://secure.travis-ci.org/asm89/symfony.png?branch=refactor-authentication-success-handling)](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
Commits
-------
6de6806 [Component][HttpKernel] fixed testGetRootDir() on Win
Discussion
----------
[Component][HttpKernel] fixed testGetRootDir() on Win
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/gajdaw/symfony.png?branch=kernel_test_fix)](http://travis-ci.org/gajdaw/symfony)
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: -
Method `getRootDir()` returns path containing slashes. On Windows machine `__DIR__` and `DIRECTORY_SEPARATOR` are backslashes. To pass the test on win machine we have to translate `\` into `/`.
Commits
-------
8997853 [Security] fixed in_memory provider example
Discussion
----------
[Security] fixed in_memory provider example
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
This fixes the in_memory provider configuration example shown by config:dump-reference
---------------------------------------------------------------------------
by stof at 2012-07-06T11:07:50Z
👍
Commits
-------
df66d90 slovenian validators translations updated
Discussion
----------
Slovenian validators translations updated
Updated Slovenian validators translations for master branch.
Commits
-------
d6e1f39 [Form] Fixed FormBuilder to maintain order of its children
Discussion
----------
[Form] Fixed FormBuilder to maintain order of its children
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #4693
Todo: -
Commits
-------
5fe3f39 [Form] Made data mappers completely responsible for dealing with empty values. Removed duplication of code.
9bf6e8b [Form] Compound forms now always need a data mapper. Otherwise an exception is thrown.
Discussion
----------
[Form] Made requirements for data mappers stricter
Bug fix: no
Feature addition: no
Backwards compatibility break: (yes)
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
This cleanup was done while trying to fix#4715, which is not easily fixable right now. It breaks BC for those people who do not extend `FormType` and manually construct `Form` instances, which hopefully nobody does as it is absolutely *not* recommended at this time.
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
-------
819fe34 [Form] refactored variable name to be more consistent with rest of the naming
Discussion
----------
[Form] Refactored config variable naming to be more consistent
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Since `clientTransformer` is renamed to `viewTransformer` everywhere, I think for consistency reasons config variable name should be changed too..
---------------------------------------------------------------------------
by stloyd at 2012-07-06T07:31:39Z
Maybe also rename of `normTransformers` to `modelTransformers` ? As this were changed too.
---------------------------------------------------------------------------
by Inori at 2012-07-06T07:40:53Z
@stloyd makes sense, done
---------------------------------------------------------------------------
by bschussek at 2012-07-06T08:24:43Z
Thanks for the cleanup. Can you squash the commits and prefix the message with "[Form]" please?
---------------------------------------------------------------------------
by Inori at 2012-07-06T08:43:49Z
@bschussek done
---------------------------------------------------------------------------
by bschussek at 2012-07-06T09:31:04Z
Thanks! @fabpot 👍
Commits
-------
6f9eda9 [Form][Validator] Fixed generation of HTML5 pattern attribute based on Assert\Regex to remove delimiters.
Discussion
----------
[Form][Validator] Fixed generation of HTML5 pattern attribute based on Assert\Regex by removing delimiters or using a new option: htmlPattern.
Hopefully, this time is the good one…
* Fixes: [#3766, #4077, #4513, #4520, #4521]
* Bug fix: yes
* Feature addition: yes
* BC break: no
* Symfony2 tests pass: yes
In Issue #3766, it was asked that Assert\Regex generates HTML5 pattern attribute.
It was done in PR #4077, but the generated Regex is in delimited format which is not supported by HTML5.
Hence, `/[a-z]+/` would be converted to `[a-z]+`.
If flags are specified like in `/[a-z]+/i`, it cannot be converted and pattern validation will be disabled client-side. If is however now possible, using a new option, `htmlPattern`, to specify the pattern you want to be used.
Example:
```php
<?php
/**
* @Assert\Regex(pattern="/^[0-9]+[a-z]*$/i", htmlPattern="^[0-9]+[a-zA-Z]*$")
*/
private $civic_number;
```
**Note**: [Documentation](http://symfony.com/doc/current/reference/constraints/Regex.html) should be updated accordingly.
---------------------------------------------------------------------------
by lavoiesl at 2012-06-08T15:45:17Z
God, I just found out you can "add more commits to this pull request by pushing to the master branch on lavoiesl/symfony"…
---------------------------------------------------------------------------
by travisbot at 2012-06-08T15:50:31Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1568634) (merged 2d767b41 into b84b46ba).
---------------------------------------------------------------------------
by petajaros at 2012-07-04T14:23:16Z
Anything new about this issue?
---------------------------------------------------------------------------
by lavoiesl at 2012-07-04T16:25:43Z
Alright, tests are passing using `phpunit -c phpunit.xml.dist --filter 'RegexValidatorTest'`. @travisbot reports errors because he can’t even start the tests due to dependencies, which is not related
---------------------------------------------------------------------------
by vicb at 2012-07-04T16:31:13Z
It should be ready to merge when you have taken the last comments into account. thanks.
---------------------------------------------------------------------------
by lavoiesl at 2012-07-04T16:39:05Z
So it seems this PR will finally pass, thanks a lot.
---------------------------------------------------------------------------
by vicb at 2012-07-04T17:03:35Z
Thank you for this PR and the changes.
---------------------------------------------------------------------------
by fabpot at 2012-07-04T17:10:20Z
@lavoiesl Can you squash your commits before I merge? Thanks.
---------------------------------------------------------------------------
by lavoiesl at 2012-07-04T17:25:18Z
There. I also left trace of some commits I did.
Thanks
[Validator] Added delimiter escaping to Validator\Constraints\Regex::getNonDelimitedPattern
[Form][Validator] Added htmlPattern option for Regex Validation.
[Validator] Fixed Validator\Constraints\Regex::getNonDelimitedPattern variable declarations
[Validator] Fixed tests for Regex htmlPattern option (instead of html_pattern)
[Validation] tweaked generation of pattern to include .* when not anchors are present. Also removed the exception and made getNonDelimitedPattern private
Commits
-------
0555913 [FrameworkBundle] Allow using kernel parameters in routes
Discussion
----------
[FrameworkBundle] Allow using kernel parameters in routes
Kernel parameters can now be used at any position in patterns, defaults and requirements.
Relates to: #3316, #3276
**Differences from 3316:**
- The substitution is now done in the `Router`,
- 3316 uses `$container->getParameterBag()` which is not part of the `ContainerInterface`. The way it been solved in this PR is that some code have been duplicated inside the `Router`, see `resolveString()`.
**BC break:**
Before this PR, nonexistent parameters would have be silently ignored (ie `%idontexist%` would not have been replaced). After this PR, they will throw an exception. However you can escape the value (ie `%%idontexist%%` will be accepted and replaced by `%idontexist%`).
_This behavior is not mandatory and can be reverted if needed. However this keeps the router more consistent with the DI_.
Any feedback ? @helmer @Koc
---------------------------------------------------------------------------
by Seldaek at 2012-07-04T12:40:08Z
👍 for consistency.
---------------------------------------------------------------------------
by helmer at 2012-07-04T13:07:11Z
+1 a much better solution to the problem than mine, closing #3316
---------------------------------------------------------------------------
by Tobion at 2012-07-04T13:21:59Z
How about escaping kernel params with `\%idontexist%` as I suggested it for route params in 4563?
---------------------------------------------------------------------------
by stof at 2012-07-04T13:28:55Z
@Tobion this would not be consistent with the way DI parameters are escaped elsewhere
---------------------------------------------------------------------------
by Koc at 2012-07-04T14:24:25Z
Looks good for me, thanx.
Commits
-------
18f8fc7 [Finder] tests for Yaml class
Discussion
----------
[Finder] tests for Yaml class
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/gajdaw/symfony.png?branch=master)](http://travis-ci.org/gajdaw/symfony)
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: -
Additional tests improving tests coverage.
Commits
-------
3f9e8ff [ClassLoader] made ClassCollectionLoader::load() automatically include class dependencies
6f4d281 [ClassLoader] added missing support for PHP 5.4 traits
Discussion
----------
Classloader optimization
The first commit fixes support for PHP 5.4 trait.
The second one does several things:
* it optimizes the recent merge so that the reflection class instance is only loaded once;
* we use the fact that we now get all class dependencies to automatically add all class dependencies to the map.
---------------------------------------------------------------------------
by fabpot at 2012-07-03T17:26:46Z
I've updated to take into accounts traits.
---------------------------------------------------------------------------
by bamarni at 2012-07-04T11:58:57Z
great job 👍
I can't see it in the diff as this part hasn't changed, but somewhere in the autoReload block there is :
```
if ($meta[1] != $classes) {
$reload = true;
}
```
It should be array_unique($classes), otherwise the file would be perpetually regenerated in autoReload mode when the input contains duplicate, because they're implicitely removed when dumping the files.
---------------------------------------------------------------------------
by fabpot at 2012-07-04T13:20:04Z
@bamarni I've added an `array_unique` call at the top (this bug existed before by the way).
Commits
-------
51b610f [Profiler] fix typehint
Discussion
----------
[Profiler] fix typehint
---------------------------------------------------------------------------
by fabpot at 2012-07-03T10:23:25Z
The profiler only works with Twig anyway.
---------------------------------------------------------------------------
by Tobion at 2012-07-03T10:29:18Z
Right. But why does he have this error: f47b9a6625 (commitcomment-1532164)
And since the class only uses methods of the general interface, I thought this makes it more reliable.
---------------------------------------------------------------------------
by fabpot at 2012-07-03T10:37:26Z
It was unrelated and I fixed that problem already.
Commits
-------
c40a4e5 [HttpFoundation] fix query string normalization
f9ec2ea refactored test method
0880174 [HttpFoundation] added failing tests for query string normalization
Discussion
----------
[HttpFoundation] fix query string normalization
This fixes the query string normalization. There were several problems in it (see test cases that I added).
The main issue, that first catched my eye, was that the query string was urldecoded before it was exploded by `=`. See old code: `explode('=', rawurldecode($segment), 2);`. This means an encoded `=` (`%3D`) would falsely be considered a separator and thus lead to complete different parameters. The fixed test case is at `pa%3Dram=foo%26bar%3Dbaz&test=test`.
---------------------------------------------------------------------------
by Tobion at 2012-07-04T02:21:25Z
cc @simensen considering your PR 4711
Commits
-------
d37003e [HttpFoundation] small fixes in Request
Discussion
----------
[HttpFoundation] small fixes in Request
phpdoc fixes,
making http_build_query explicit
fixing query string of '0', that was ignored.
Unfortunately this '0' problematic is omnipresent because PHP makes it so easy to get wrong (as it is converted to boolean false). I don't know how often I fixed such issue already.
Commits
-------
45219ef [Process] Add default xampp path to the list of possible paths to check
28e1313 [Process] Clean-up/simplify code
56f473a [Process] Add extra dirs argument to the executable finder to allow searching more dirs
Discussion
----------
ExecutableFinder changes
It cleans up stuff a bit and adds a guess for xampp users that wouldn't have php in their path
Commits
-------
eda439f [EventDataCollector] Display a better message when no events have been recorded
6b87981 [TimeDataCollector] Do not throw an exception when no events are recorded
Discussion
----------
[Profiler] Better support for collector in a production env
relates to #3706.
With this PR it is possible to:
- enable only the profiler in a production environment - the wdt being disabled you have to switch to the development environment to inspect the collected data,
- enable both the profiler and the wdt in a production environment (the use case form #3706).
@jmikola would this solve your use case ?
Commits
-------
26a1e0b [ClassLoader] ordered ClassCollectionLoader writing to avoid redeclaration at runtime
Discussion
----------
[HttpKernel] allowed classes to compile to be prepended
I had an issue when registering JMSDIExtraBundle before the frameworkBundle, because the bundle is adding a class to compile wich extends a class to compile added by the frameworkbundle (https://github.com/schmittjoh/JMSDiExtraBundle/blob/master/HttpKernel/ControllerResolver.php#L39).
In my kernel, the bundle was registered before the frameworkbundle, if it's the case, the class is writed before the symfony core class in the cache file, so it will trigger the autoloader to load the symfony core class, then we'll have a fatal error because we're declaring 2 times the same class.
I'm suggesting to add a way to prepend the classes to compile added by an extension, this way we could force classes from the core bundle to pe prepended, and avoid this kind of error with other bundles adding classes extending core classes or implementing core interface. I've also added it to the frameworkbundle.
---------------------------------------------------------------------------
by travisbot at 2012-07-01T12:32:28Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1750090) (merged a989d35c into a1b73887).
---------------------------------------------------------------------------
by schmittjoh at 2012-07-01T12:45:46Z
This doesn't sound really failure proof, better would be to resolve the compiled classes in a way that handles the dependencies gracefully regardless of the order that they are registered in.
---------------------------------------------------------------------------
by bamarni at 2012-07-01T13:03:15Z
yes that would be much cleaner, even if it's not perfect, I don't have any other examples, but I think in most of situations this error happens because of the frameworkbundle late registration?
---------------------------------------------------------------------------
by bamarni at 2012-07-01T23:07:24Z
I've added an automatic reordering as suggested. Now I don't get the error whatever the order of FrameworkBundle and JMSDIExtraBundle is.
---------------------------------------------------------------------------
by vicb at 2012-07-02T10:02:33Z
Does the algo really works ?
Let's say I have the following hierarchy `A < B < C` and the classes added in the following order; `C, B, A`. I believe that the algo would return `B, A, C` instead of `A, B, C`.
An other question: should the dependency resolution be implemented in the `ClassCollectionLoader` rather than in the compiler pass ?
---------------------------------------------------------------------------
by vicb at 2012-07-02T10:23:11Z
@bamarni could you confirm the issue with the algo ? If it is confirmed, do you have time to work on a fix ?
---------------------------------------------------------------------------
by bamarni at 2012-07-02T11:05:53Z
the algo looks correct, there is a loop checking dependencies and appending the parent classes at the begining so it should also work with A > B > C, but I'll check and add a unit test.
You're right about the location it could belong there if we want a general fix, should I put it in the ClassLoader component?
---------------------------------------------------------------------------
by vicb at 2012-07-02T11:28:14Z
Yep please move the code to the ClassLoader.
You could also add some cache mechanism:
- ReflectionClass could be cached (they might be use mulitple times when some dependencies exist and are also used by the class loader)
- May be you could also cache the hierarchy of the classes
---------------------------------------------------------------------------
by bamarni at 2012-07-02T17:25:08Z
@vicb : indeed there were something wrong with the algo when it needed to handle more than 1 level of dependency, it's fixed, I've added a few test case and it passes.
Caching the dependencies found with reflection would be useful if people are dumping several files with some similar classes in the same process, but I don't see why we would need to cache the hierarchy?
---------------------------------------------------------------------------
by vicb at 2012-07-02T18:29:06Z
@bamarni I was referring to a local cache (ie local variable).
- On the first iteration, the code instantiate a reflection class,
- On other iteration you would instantiate again a reflection class for a subset of the classes
- ...
- In the end the former code instantiate again a reflection class for each class in order to get the file name, ...
I was also thinking of an other algo: count the number of parent classes for each classes to include and order classes according to this number. This would be a single loop only, what do you think ?
---------------------------------------------------------------------------
by bamarni at 2012-07-02T19:32:34Z
hum good idea counting the parents is cleaner, even though it looks enough, would it also make sense to treat interfaces and classes separately? I'm thinking about a file with all the interfaces at the begining (ordered by number of parents desc), then the classes would be appended (ordered by number of parents desc too).
---------------------------------------------------------------------------
by vicb at 2012-07-03T07:05:07Z
There is no real benefit in making the interfaces appear first but you can do it if it doesn't make the code more complex.
Please also add the `@throws` annotation to the methods.
---------------------------------------------------------------------------
by bamarni at 2012-07-03T09:37:28Z
@vicb : I've changed it to use the parents count, looks good to me.
---------------------------------------------------------------------------
by bamarni at 2012-07-03T12:23:46Z
changed
---------------------------------------------------------------------------
by bamarni at 2012-07-03T12:42:29Z
fixed
Commits
-------
3466896 [Routing] fix encoding of static static, so UrlGenerator produces valid URLs
Discussion
----------
[Routing] fix encoding of static text
Fixes#4166
As requested by Fabien, I split #4205 into multiple PRs.
---------------------------------------------------------------------------
by stof at 2012-06-09T12:49:32Z
github tells me this PR cannot be merged automatically. Could you rebase it ?
---------------------------------------------------------------------------
by Tobion at 2012-06-09T13:12:55Z
Done
---------------------------------------------------------------------------
by travisbot at 2012-06-09T13:18:18Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1576266) (merged 3466896a into 15b5aa4f).
Commits
-------
d9439ab made the charset overridable (closes#2072)
Discussion
----------
made the charset overridable (closes#2072)
The charset was configurable in a configuration file but it never worked:
framework:
charset: ISO-8859-1
Now, like for the cache and log dirs, you can configure the charset by
overriding the getCharset() method in the app kernel:
public function getCharset()
{
return 'ISO-8859-1';
}
---------------------------------------------------------------------------
by fabpot at 2012-07-03T07:26:04Z
See #2072 for the previous attempts to fix this issue.
The charset was configurable in a configuration file but it never worked:
framework:
charset: ISO-8859-1
Now, like for the cache and log dirs, you can configure the charset by
overriding the getCharset() method in the app kernel:
public function getCharset()
{
return 'ISO-8859-1';
}
Commits
-------
6296a24 Standalone query string normalization
Discussion
----------
[HttpFoundation] Standalone query string normalization
I wanted to leverage query string normalization in a test. I considered copying this code to my own library but I noticed that the only instance data `getQueryString` needed to know from the `Request` was `QUERY_STRING` so I broke the rest out into a standalone static function.
---------------------------------------------------------------------------
by simensen at 2012-07-02T20:05:59Z
I made the requested changes.
---------------------------------------------------------------------------
by fabpot at 2012-07-02T20:10:27Z
Can you squash your commits? Thanks.
---------------------------------------------------------------------------
by simensen at 2012-07-02T20:16:52Z
Sure, no problem. Hopefully I did it correctly.
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
-------
88caf3a [HttpKernel] removed the storage of the current locale in the session
Discussion
----------
[HttpKernel] removed the storage of the current locale in the session
Before this commit, the current locale was stored in the session (if one
was already started). That way, for the next requests, even if the
request locale attribute was not set, the locale was "restored".
But this is a really bad practice as it means that the same URL can have
a different content depending on the previous requests. It would have
been better if the Vary header was set but the locale can be different
from the value coming from the Accept-Language anyway.
This is a BC break but fortunately, you can restore the 2.0 behavior by
creating a simple event listener that contains the logic removed by this
commit.
---------------------------------------------------------------------------
by travisbot at 2012-07-01T06:56:48Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1748659) (merged 009e30f0 into 2e356c1a).
---------------------------------------------------------------------------
by schmittjoh at 2012-07-01T08:15:46Z
How about using a cookie instead? It would remove the BC break, and also be possible to use a vary header?
---------------------------------------------------------------------------
by fabpot at 2012-07-01T09:13:44Z
The goal is to make Symfony as stateless as possible; introducing a cookie would defeat this goal.
---------------------------------------------------------------------------
by drak at 2012-07-01T09:19:37Z
@fabpot - thank you for bringing this to attention. I was meaning to do it a long time ago. The requested language is entirely a per request issue and must always be so. URLs must only ever return one content, and not multiple (e.g. different languages). The correct way to behave is to detect the language based on URL and failing that where a language is not requested, to look at the preferred language from the browser request and if available it can be redirected to that resource (e.g. /fr). This is what we do in Zikula. We have a further session based setting for "preferred language" which if set will override the browser default.
In summary:
1. If the language is specified in the GET request, return that language always. E.g. domain.com/fr/foo should return a French version of foo
2. If no language is specified in the GET request: first check the session for a preferred language, otherwise check the browser string for the preferred language and then if necessary, redirect to that resource. We have a setting which additionally say "always have language in URL, and don't put language code in URL for default language"
This means what in Zikula we only ever have one URL per language version of a page, but it still allows for users to set their preferred language which is taken in to account mainly when they visit the homepage (but in fact any page without a specific language in the request).
---------------------------------------------------------------------------
by drak at 2012-07-01T09:38:06Z
+1 on this PR. Basically the request locale should be in the Request object and calculated according to the applications preferences.
---------------------------------------------------------------------------
by schmittjoh at 2012-07-01T12:38:25Z
I agree that content must be detected based on the request, but I strongly disagree with relying entirely on the URL.
@fabpot, if you think about it using a cookie would still be stateless. There would be no state whatsoever, the detection would be entirely based on the request. Whether the language information is transmitted in the URL or as part of request headers is for the developer to decide eventually, at least IMO. My suggestion would just provide a default which is more BC.
---------------------------------------------------------------------------
by drak at 2012-07-01T20:08:50Z
@schmittjoh it's not entirely from the URL, there are browser preferences and also user defaults ca nalso available but the latter is slightly higher level. IMO it's not really Symfony's job here, it's application level specific. We have a pretty good working example of that in Zikula. Anyone can easily implement your own requirements with a listener.
What is absolutely clear however is it is wrong for one URL to deliver more than one version of any content.
---------------------------------------------------------------------------
by schmittjoh at 2012-07-01T21:16:52Z
I'm 100% for this change. My suggestion would just be more BC while still keeping Symfony2 stateless. Of course, it can be easily implemented in userland if we do not care about BC here.
Regarding different URLs per content, I do not think that this is our decision to make. Generally, developers should be able to make whatever content negotation they see fit. Whether they rely solely on the URL, or also take other request headers into account should not be limited by Symfony2.
---------------------------------------------------------------------------
by fabpot at 2012-07-02T10:37:26Z
I've added a paragraph in the UPGRADE file with a listener example that can be used to keep BC.
Before this commit, the current locale was stored in the session (if one
was already started). That way, for the next requests, even if the
request locale attribute was not set, the locale was "restored".
But this is a really bad practice as it means that the same URL can have
a different content depending on the previous requests. It would have
been better if the Vary header was set but the locale can be different
from the value coming from the Accept-Language anyway.
This is a BC break but fortunately, you can restore the 2.0 behavior by
creating a simple event listener that contains the logic removed by this
commit.
Commits
-------
1472283 fixed CS
bc73487 renamed template to TemplateManager , moved profiler to the deps of manager
5fd6ed6 properties protected
abd0eb7 generating template names moved out from controller to another class
6138e80 [Profiler] relying on config of displayed profile instead of current config.
Discussion
----------
[2.2][Profiler] relying on config of displayed profile instead of current config
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: code of ProfilerController is not covered by any test
Fixes the following tickets: #3372
Todo: ~
This fixes the exception which is raised when viewed profile has other data collectors than in config of currently run profiler.
explained here
https://github.com/symfony/symfony/issues/3372
---------------------------------------------------------------------------
by fabpot at 2012-02-16T06:11:00Z
This should probably be done on the 2.0 branch. Also, I think we need to check if the panel is actually available in the current profiler (if not, we won't be able to display it anyway). So, both checks are important.
---------------------------------------------------------------------------
by wodor at 2012-02-18T10:15:40Z
defects mentioned by Stof are fixed
Commits
-------
b804b94 Fixed style for the abbr tag
147cab7 [WDT] Fix the color of Documentation link to keep concistence.
Discussion
----------
[WDT] Fix the color of Documentation link to keep concistence.
This pull request is to make the Documentation link black as the other links of the WDT
---------------------------------------------------------------------------
by travisbot at 2012-05-11T13:33:24Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1304777) (merged 5a87a098 into 554e0738).
---------------------------------------------------------------------------
by Tobion at 2012-05-11T16:36:39Z
should be done via selector in the css file that is used for the WDT (also refactor the profiler token link like this)
---------------------------------------------------------------------------
by nomack84 at 2012-05-11T17:46:15Z
Done.
---------------------------------------------------------------------------
by travisbot at 2012-05-11T17:48:24Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1307502) (merged eee437c9 into 554e0738).
---------------------------------------------------------------------------
by travisbot at 2012-05-11T18:27:55Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1307838) (merged 3604f131 into dd0da03c).
---------------------------------------------------------------------------
by mvrhov at 2012-05-11T18:40:05Z
While you are at it, the controller text color is also wrong.
---------------------------------------------------------------------------
by travisbot at 2012-05-11T18:43:00Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1308018) (merged 147cab74 into dd0da03c).
---------------------------------------------------------------------------
by nomack84 at 2012-05-11T18:49:47Z
@mvrhov I don't see the difference.
---------------------------------------------------------------------------
by mvrhov at 2012-05-11T19:45:45Z
Set the color for abbr tag on your website to red or something like that. By default abbr color is set to black. My website has is set to #55555 so the controller name its barely visible.
---------------------------------------------------------------------------
by nomack84 at 2012-05-14T12:42:30Z
@mvrhov Done!
---------------------------------------------------------------------------
by travisbot at 2012-05-14T12:43:48Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1326494) (merged b804b942 into dd0da03c).
---------------------------------------------------------------------------
by nomack84 at 2012-05-15T13:09:59Z
Hi @fabpot,
Can you merge this? The only thing it does is add a missed style to the Documentation link and also to the abbr tag, as suggested by @mvrhov.
Greetings!
Commits
-------
4d09fe6 [Finder] '*' and '?' are considered are glob pattern rather than delimiters (fix#4664)
Discussion
----------
[Finder] '*' and '?' are considered are glob pattern rather than delimit...
...ers (fix#4664)
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".
Commits
-------
f09789b [FrameworkBundle] Generate the class cache when warming up the cache
Discussion
----------
[FrameworkBundle] Generate the class cache when warming up the cache
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/jalliot/symfony.png?branch=load-class-cache)](http://travis-ci.org/jalliot/symfony)
Fixes the following tickets: -
Todo: -
With this PR, the commands `cache:clear` (if `--no-warmup` hasn't been specified) and `cache:warmup` generate the class cache. Now the first page load after clearing the cache does not take over one second anymore :)
Of course, if someone does not want to use the class cache for whatever reason, he can always remove the `$kernel->loadClassCache()` in his front controller and the cache will just be ignored...
On a side note, can someone explain why [SensioDistributionBundle does not warmup the cache in the Composer post-install script](https://github.com/sensio/SensioDistributionBundle/blob/master/Composer/ScriptHandler.php#L48)?
---------------------------------------------------------------------------
by travisbot at 2012-06-10T05:18:30Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1579114) (merged baecbaee into 6266b72d).
---------------------------------------------------------------------------
by travisbot at 2012-06-10T05:24:48Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1579154) (merged f09789b1 into 6266b72d).
---------------------------------------------------------------------------
by jalliot at 2012-06-28T23:18:54Z
@fabpot ping
Commits
-------
7464dcd added phpdoc
c413e7b [Routing] remove RequestContextAwareInterface from RequestMatcherInterface
921be34 [Routing] fix phpdoc
Discussion
----------
[Routing] RequestMatcherInterface doesn't need context
Matchers that implement RequestMatcherInterface should match a Request, thus they don't need the request context.
---------------------------------------------------------------------------
by travisbot at 2012-06-14T21:39:48Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1624496) (merged f5ff1fe0 into 7c91ee57).
---------------------------------------------------------------------------
by schmittjoh at 2012-06-15T13:32:59Z
I think it makes sense to remove the RequestContext from the RequestMatcher.
---------------------------------------------------------------------------
by travisbot at 2012-06-15T15:54:28Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1628931) (merged 7464dcd2 into f881d282).
---------------------------------------------------------------------------
by Tobion at 2012-06-26T12:32:06Z
Anything missing?
Commits
-------
c1e4166 moved create of default form label to view layer
Discussion
----------
move create of default form label to view layer
A small optimization if you provide custom labels in the view layer (i.e. `{{ form_label(form.name, 'Your name') }}`
```
Bug fix: no
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: ~
Todo: ~
License of the code: MIT
Documentation PR: ~
```
---------------------------------------------------------------------------
by travisbot at 2012-06-24T14:45:17Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1694310) (merged 37f0b774 into 0d4b02e4).
---------------------------------------------------------------------------
by travisbot at 2012-06-24T15:03:44Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1694418) (merged c1e4166e into 0d4b02e4).
Commits
-------
eb26e89 [FrameworkBundle] Fix built-in server when using query params in paths
Discussion
----------
[FrameworkBundle] Fix built-in server when using query params in paths
$_SERVER['REQUEST_URI'] will contain the query params so is_file will fail.
I propose to use $_SERVER['SCRIPT_FILENAME'] instead which contains the full path and no query params.
---------------------------------------------------------------------------
by ajessu at 2012-06-23T10:17:34Z
I was going to make this comment on your approach in #4484, but I'll make it here, since that issue is already closed.
Your solution won't work on PHP 5.4.0, as `$_SERVER['SCRIPT_FILENAME']` will not be set [see PHP bug #60850](https://bugs.php.net/bug.php?id=60850).
Also PHP 5.4.1 and up, if you don't request a file explicitely, Ex:
http://localhost:8000/app_dev.php
but a location, Ex:
http://localhost:8000/
The value of the `$_SERVER['SCRIPT_FILENAME']` will be the router file, not the script name, which makes relying on `$_SERVER['SCRIPT_FILENAME']` inconsistent. [See this comment on the php bug](https://bugs.php.net/bug.php?id=60850#1331261652)
I'm not sure if (nor how?) the issue of the params should be addressed on this "default" router, to not make it overly complex.
For your use case, and this is just my own early opinion without much thought, in case we can't come up with a general solution, there is always the option of defining your own router and passing it to the `server:run` command with `--router` like so:
php app/console server:run --router=app/config/my_own_router.php
---------------------------------------------------------------------------
by flojon at 2012-06-23T10:31:47Z
So would `$_SERVER['SCRIPT_NAME']` be more reliable? Like this:
if (is_file($_SERVER['DOCUMENT_ROOT'] . DIRECTORY_SEPARATOR . $_SERVER['SCRIPT_NAME'])) {
return false;
}
I did a simple test and `$_SERVER['SCRIPT_NAME']` is set to `/` when accessing the root (using PHP 5.4.3).
---------------------------------------------------------------------------
by flojon at 2012-06-23T10:51:22Z
Browse around the code a bit and `$_SERVER['SCRIPT_NAME']` has been there since PHP 5.4.0:
https://github.com/php/php-src/blob/php-5.4.0/sapi/cli/php_cli_server.c#L598
---------------------------------------------------------------------------
by travisbot at 2012-06-23T11:16:59Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1688361) (merged eb26e896 into 0d4b02e4).
---------------------------------------------------------------------------
by travisbot at 2012-06-24T10:23:52Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1688043) (merged 71855665 into 0d4b02e4).
---------------------------------------------------------------------------
by CHH at 2012-06-29T07:17:32Z
This works fine for me!
👍
Could someone please merge this? This issue makes the `server:run` command currently quite unusable, because it can't load CSS for example which has a `?v=` parameter.
---------------------------------------------------------------------------
by ajessu at 2012-06-29T08:25:14Z
👍 from me also. Works just like `$_SERVER['REQUEST_URI']`, but doesn't include the params.
Tested working on PHP 5.4.0 and 5.4.3.
Commits
-------
911db69 [FrameworkBundle] Typo fix
19eeac8 [HttpFoundation] Removed erroneous reliance on session.auto_start
dcac5d7 [HttpFoundation] Corrected docblocks and properties.
1fd66f3 [FrameworkBundle] Remove 'auto_start' configuration parameter.
Discussion
----------
[HttpFoundation] Remove session start on demand
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
License of the code: MIT
This removes false reliance on ini directive `session.auto_start` to allow a session to start when session bags are accessed before the `SessionStorageInterface` is started.
Sessions must be explicitly started in all circumstances.
---------------------------------------------------------------------------
by stloyd at 2012-06-13T07:22:24Z
@drak Shouldn't you add note about this change in upgrade file ?
---------------------------------------------------------------------------
by drak at 2012-06-13T15:13:37Z
It's a development version change, so not really. But saying that, I have a bunch of documentation to amend when this gets merged and at that time I'll make sure the changelogs and upgrading are up to date as part of that.
---------------------------------------------------------------------------
by dlsniper at 2012-06-13T21:57:28Z
@drak If this change will kick in what does one user of Symfony 2 Standard must do in order to keep compat with this merge? I see that you said you'll update the docs but until that happens some might upgrade their app directly to master :)
Thanks.
---------------------------------------------------------------------------
by drak at 2012-06-14T01:36:04Z
@dlsniper - nothing. This corrects a bug and inconsistency.
---------------------------------------------------------------------------
by travisbot at 2012-06-29T17:48:42Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1739033) (merged 19eeac88 into 62100f1a).
---------------------------------------------------------------------------
by drak at 2012-06-29T17:55:13Z
@fabpot ping. The failing Travis is nothing to do with this PR (see the travis logs).
---------------------------------------------------------------------------
by travisbot at 2012-06-29T20:39:43Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1739805) (merged 911db69b into 62100f1a).