This PR was submitted for the 2.1 branch but it was merged into the master branch instead (closes#5885).
Commits
-------
efc22a8 [HttpKernel] Added ability to set controller result in the kernel.view event
Discussion
----------
[HttpKernel] Added ability to set controller result in the kernel.view event
Added the method `GetResponseForControllerResultEvent:setControllerResult(array $controllerResult)` to allow kernel.view events to inject data into the result.
---------------------------------------------------------------------------
by stof at 2012-11-01T13:48:51Z
new features canot be added in the 2.1 branch, which is a maintenance branch and so receives only bugfixes.
---------------------------------------------------------------------------
by fabpot at 2012-11-02T07:13:56Z
I'm not opposed to the change but can you share your use case with us?
---------------------------------------------------------------------------
by ltouroumov at 2012-11-02T07:55:55Z
I have custom annotations that allow controller methods to be invoked before the actions is run (`@BeforeFilter`) and after the action is run (`@ViewFilter`) and after the response is sent (`@ResponseFilter`).
The view filter allows to alter the controller result for example to inject parent entities into the template.
If you need more details I suppose I could post the code but I would have to ask the boss first.
---------------------------------------------------------------------------
by ltouroumov at 2012-11-02T07:57:53Z
Also I fixed the missing dollar sign in the [`6ffea1dc1cfa23844c6d90a1bfa9a282d1807a61`](6ffea1dc1c) commit. But I can't seem to attach it to the pull request.
---------------------------------------------------------------------------
by henrikbjorn at 2012-11-02T08:12:20Z
Needs tests.
---------------------------------------------------------------------------
by ltouroumov at 2012-11-02T08:38:03Z
Is it really necessary to write a test for a simple setter ? I'm not opposed to testing, just looking for a justification. And should I create a new test case or is there one ?
---------------------------------------------------------------------------
by henrikbjorn at 2012-11-02T08:42:49Z
Since your code had a clear syntax error i think that a test is in order ;)
---------------------------------------------------------------------------
by ltouroumov at 2012-11-02T08:45:03Z
Can't argue with that !
Would putting it in `Symfony\Component\HttpKernel\Tests\EventListener\ControllerResponseTest` do the trick ?
---------------------------------------------------------------------------
by henrikbjorn at 2012-11-02T09:16:55Z
Sounds fair enough.
---------------------------------------------------------------------------
by ltouroumov at 2012-11-02T09:31:09Z
This tests covers my use case(s), but there might be others I didn't think of.
---------------------------------------------------------------------------
by ludekstepan at 2012-11-20T17:15:45Z
+1
Please merge into master, I have a use case for it, now I'm relying on a Reflection to set the private property, but that's really a dirty solution. The setter would be perfect!
This PR was submitted for the master branch but it was merged into the 2.1 branch instead (closes#6073).
Commits
-------
c1c822b Filesystem Component mirror symlinked directory fix
Discussion
----------
Filesystem Component mirror symlinked directory fix
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets:#6070
Todo:
License of the code: MIT
Documentation PR:
Because of the order in which file types (i.e. dir, link, file) are checked in the Filesystem component, symlinked directories get treated as directories instead of symlinks. As a result symlinked directories are not truly mirrored when performing a php app/console assets:install. This PR checks that a file is a symlink BEFORE checking that its a directory and properly symlinks as necessary.
This PR was merged into the 2.1 branch.
Commits
-------
915dd30 info about session namespace
4b21d18 fix upgrade info about locale
Discussion
----------
fix upgrade info about locale
it duplicated the header and had an irrelevant point inbetween
I also added an entry about session namespace.
This PR was merged into the 2.1 branch.
Commits
-------
0f4d8af [Process][Tests] fixed chainedCommandsOutput tests
Discussion
----------
[Process][Tests] fixed chainedCommandsOutput tests
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes (previously no)
Fixes the following tickets: -
Todo: -
License of the code: MIT
Currently, if you run the Process Component tests on an OS where echo does not support the -n option (like MacOS X), they will fail.
This PR fix that by using only a simple echo which is universal.
This PR was merged into the 2.0 branch.
Commits
-------
64b54dc Use better default ports in urlRedirectAction
64216f2 Add tests for urlRedirectAction
Discussion
----------
Default to current port in urlRedirectAction
I was a bit surprised when I used urlRedirectAction from a non-standard port (8000) it redirected me to port 80. I would argue that the default should be to use the current port instead. This is a simple patch to change that. This should only break in the case someone is relying on the current default to redirect from a non-standard port to the standard port, which should be a really rare case...
---------------------------------------------------------------------------
by Tobion at 2012-11-11T20:29:54Z
The idea is right but the implementation not. Seems this patch is not as "simple" as you said.
When you're on HTTPS and want to redirect to $scheme = HTTP, then it still uses the current HTTPS port which is wrong.
---------------------------------------------------------------------------
by flojon at 2012-11-11T20:36:47Z
Ah, I see the problem. So I guess the correct behavior would be to use the current port if staying with the same scheme or go to standard port if switching scheme. Unless the user has specified a port which will always override...
---------------------------------------------------------------------------
by Tobion at 2012-11-11T20:42:18Z
That would be the best solution that is currently possible but not the best solution that should be possible.
Because if you switch scheme but the other scheme does not use the standard port, it still doesn't work.
Ideally the Request class had an option that allows to define the ports symfony should use for HTTP and HTTPS.
This logic is in RequestContext, but it's not used here.
---------------------------------------------------------------------------
by flojon at 2012-11-11T21:32:55Z
Bummer, I forgot to check if the current port is a standard port...
---------------------------------------------------------------------------
by Tobion at 2012-11-11T21:35:13Z
add some tests
---------------------------------------------------------------------------
by flojon at 2012-11-11T23:28:18Z
Added tests and fixed my previous error
---------------------------------------------------------------------------
by flojon at 2012-11-15T18:25:12Z
@Tobion is there anything else I needed for this?
---------------------------------------------------------------------------
by fabpot at 2012-11-19T12:56:04Z
To be consistent with how we manage HTTP ports elsewhere, I'd rather use the values of the `request_listener.http_port` and `request_listener.https_port`:
```php
if (null === $httpPort) {
$httpPort = $this->container->getParameter('request_listener.http_port');
}
if (null === $httpsPort) {
$httpsPort = $this->container->getParameter('request_listener.https_port');
}
```
This is done in the `security.authentication.retry_entry_point` service and for the `router_listener` listener.
The parameter name is probably not the best one, but that could be changed then in master.
---------------------------------------------------------------------------
by flojon at 2012-11-19T13:49:18Z
@fabpot But then you would need to set that parameter manually right? It wouldn't automatically redirect you to the same port, which was what I wanted to achieve...
Could this be the right order of preference:
If a value was specified in the route use that.
Otherwise use the current port
unless switching scheme then use the parameter value
---------------------------------------------------------------------------
by fabpot at 2012-11-19T13:52:17Z
Your order of preference looks good to me.
---------------------------------------------------------------------------
by flojon at 2012-11-19T19:13:19Z
Man this was more involved than I thought... :)
Changed the logic to use the parameters when not using the current port. Also tried clean up the tests a little bit... Enjoy!
This PR was merged into the master branch.
Commits
-------
e39b709 [Routing] initialize the Route properties
Discussion
----------
[Routing] initialize the Route properties
They should normally be initialized anyway in the constructor. But when extending the Route (like in CMF) and using an ORM/ODM to persist them in the DB, the constructor is not called. Then a new property that is not saved like hostnamePattern stays null which in turn makes the RouteCompiler fails as it expects '' instead of null.
This PR was squashed before being merged into the master branch (closes#6063).
Commits
-------
d3d8ff4 [travis-ci] Zend Garbage Collection only for PHP5.4
Discussion
----------
[travis-ci] Zend Garbage Collection only for PHP5.4
They should normally be initialized anyway in the constructor. But when extending the Route (like in CMF) and using an ORM/ODM to persist them in the DB, the constructor is not called. Then a new property that is not saved like hostnamePattern stays null which in turn makes the RouteCompiler fails as it expects '' instead of null.
This PR was merged into the 2.0 branch.
Commits
-------
f2cbea3 [Security] remove escape charters from username provided by Digest DigestAuthenticationListener
80f6992 [Security] added test extra for digest authentication
d66b03c fixed CS
694697d [Security] Fixed digest authentication
c067586 [Security] Fixed digest authentication
Discussion
----------
Fix digest authentication
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets:
Todo: -
License of the code: MIT
Documentation PR: -
Replaces: #5485
This adds the missing fixes.
My only concerns is the ```\"``` removing.
```\"``` is only needed for the HTTP transport, but keeping them would require to also store the username with the escapes as well.
---------------------------------------------------------------------------
by fabpot at 2012-10-30T11:25:28Z
The digest authentication mechanism is not that widespread due to its limitation. And the transport is not HTTP, I think we are talking about very few cases.
---------------------------------------------------------------------------
by sstok at 2012-10-30T12:49:14Z
Apache seems to remove (ignore) escape characters.
```c
if (auth_line[0] == '=') {
auth_line++;
while (apr_isspace(auth_line[0])) {
auth_line++;
}
vv = 0;
if (auth_line[0] == '\"') { /* quoted string */
auth_line++;
while (auth_line[0] != '\"' && auth_line[0] != '\0') {
if (auth_line[0] == '\\' && auth_line[1] != '\0') {
auth_line++; /* escaped char */
}
value[vv++] = *auth_line++;
}
if (auth_line[0] != '\0') {
auth_line++;
}
}
else { /* token */
while (auth_line[0] != ',' && auth_line[0] != '\0'
&& !apr_isspace(auth_line[0])) {
value[vv++] = *auth_line++;
}
}
value[vv] = '\0';
}
```
But would this change be a BC break for people already using quotes but without a comma and thus they never hit this bug?
The change it self is minimum, just calling ```str_replace('\\\\', '\\', str_replace('\\"', '"', $value))``` when getting the username.
---------------------------------------------------------------------------
by fabpot at 2012-11-13T13:00:12Z
@sstok Doing the same as Apache seems the best option here (just document the BC break).
---------------------------------------------------------------------------
by sstok at 2012-11-15T16:05:00Z
Hopefully I did this correct, but the needed escapes seem correctly removed.
`\"` is changed to `"` `\\` is changed to `\`
`\'` it kept as it is, as this needs no correcting.
@Vincent-Simonin Can you verify please.
---------------------------------------------------------------------------
by Vincent-Simonin at 2012-11-19T09:28:18Z
Authentication didn't work with this configuration :
```
providers:
in_memory:
name: in_memory
users:
te"st: { password: test, roles: [ 'ROLE_USER' ] }
```
`te"st` was set in authentication form's user field.
(Must we also escape `"` in configuration file ?)
Tests were performed with nginx.
---------------------------------------------------------------------------
by sstok at 2012-11-19T09:33:34Z
Yes. YAML escapes using an duplicate quote, like SQL.
```yaml
providers:
in_memory:
name: in_memory
users:
"te""st": { password: test, roles: [ 'ROLE_USER' ] }
```
This PR was squashed before being merged into the master branch (closes#5888).
Commits
-------
2379d86 CS Fixes - Replaced "array of type" by "Type[]" in PHPDoc block
Discussion
----------
CS Fixes - Replaced "array of type" by "Type[]" in PHPDoc block
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: no (but tests doesn't pass on master too). See Travis.
License of the code: MIT
Documentation PR: Not Applicable
Status: Finished
To improve support of the eclipse PDT pluggin (for autocompletion), I propose to change the array notation in PHPDoc blocks to match the phpDocumentor notation for "array of type".
Modifications are made for the following components:
- BrowserKit
- ClassLoader
- Config
- Console
- CssSelector
- DependencyInjection
- DomCrawler
- EventDispatcher (no changes)
- Filesystem (no changes)
- Finder
- Form
- HttpFoundation
- HttpKernel
- Locale
- OptionResolver (no changes)
- Process (no changes)
- Routing (no changes)
- Serializer (no changes)
- Templating
- Translation
- Validator
- Yaml (no changes)
- Security
- Stopwatch (no changes)
See Proposal https://github.com/symfony/symfony/pull/5852
---------------------------------------------------------------------------
by pborreli at 2012-11-01T15:19:27Z
will you make a PR for each component ? why not only one PR with one commit for each component instead ?
---------------------------------------------------------------------------
by raziel057 at 2012-11-01T15:32:39Z
Ok, I'm going try to do it.
---------------------------------------------------------------------------
by raziel057 at 2012-11-01T16:12:56Z
I would like to rename my branch from COMPONENT_Form to changes-phpdoc (as all modifications would be commited in only one branch), so I tried to execute the following command but I have an error.
git remote rename COMPONENT_Form changes-phpdoc
error: Could not rename config section 'remote.COMPONENT_Form' to 'remote.changes-phpdoc'
Do you know how to do it?
---------------------------------------------------------------------------
by pborreli at 2012-11-01T16:14:26Z
don't rename it, you will have to close and make another PR which is useless here, just edit the title.
---------------------------------------------------------------------------
by stof at 2012-11-01T16:16:17Z
and ``git remote rename`` is about renaming a remote repo, not a branch
---------------------------------------------------------------------------
by raziel057 at 2012-11-03T11:36:02Z
Is it normal that all my commit are duplicated? I would like just update my master and merge with my branch.
---------------------------------------------------------------------------
by fabpot at 2012-11-06T10:22:55Z
@raziel057 Can you rebase on master? That should fix your problem.
---------------------------------------------------------------------------
by fabpot at 2012-11-09T13:28:53Z
@raziel057 Can you finish this PR?
---------------------------------------------------------------------------
by Tobion at 2012-11-09T13:34:45Z
I'll do it for the routing component this evening because I know it by heart. ^^
---------------------------------------------------------------------------
by raziel057 at 2012-11-09T15:06:26Z
@Tobion ok Thanks!
@fabpot Yes, I will try to finish it this week end.
---------------------------------------------------------------------------
by raziel057 at 2012-11-11T13:04:07Z
@Tobion Did you already change PHPDoc in the Routing component?
---------------------------------------------------------------------------
by Tobion at 2012-11-11T15:21:18Z
@raziel057 Yes I'm working on it.
---------------------------------------------------------------------------
by Tobion at 2012-11-12T15:16:31Z
@raziel057 Done. See #5994
This PR was merged into the 2.0 branch.
Commits
-------
32dc31e [SecurityBundle] Convert Http method to uppercase in the config
Discussion
----------
[SecurityBundle] Convert Http method to uppercase in the config
This is not striclty required as method names would be converted to uppercase by the matcher after #5988.
However I think it is better to always use uppercase for http method names.
The config UT has also been improved as part of this PR.
This is good to propagate to 2.1 & 2.2 also.
This PR was submitted for the master branch but it was merged into the 2.0 branch instead (closes#6015).
Commits
-------
f61c019 Update src/Symfony/Component/DomCrawler/Tests/FormTest.php
9b3aaf2 Update src/Symfony/Component/DomCrawler/Form.php
Discussion
----------
FIX: Malformed field path ""
In case we have the name attribute empty.
---------------------------------------------------------------------------
by fabpot at 2012-11-15T06:12:35Z
Can you add a unit test for that case?
---------------------------------------------------------------------------
by bierdok at 2012-11-15T09:21:01Z
Voila.
This PR was merged into the master branch.
Commits
-------
824a0f3 [Routing] compatibility with older PCRE (pre 8)
Discussion
----------
[Routing] compatibility with older PCRE (pre 8)
#6062 for master
This PR was merged into the 2.1 branch.
Commits
-------
1daefa5 [Routing] made it compatible with older PCRE version (pre 8)
Discussion
----------
[Routing] compatibility with older PCRE version (pre 8)
fixes#4093
Ok I changed my mind about this issue.
1. I figured more people are affected than I thought and CentOS is stubborn.
2. Symfony still uses the old regex style `?P<param>` in several other components. So also doing so in the routing makes it more consistent.
3. Even if it's definitely not good to use an over 6 year old PCRE version with a recent PHP version, we can still try to provide the best experience. It doesn't mean we support outdated software stacks of custom PHP compilations as we won't and cannot specifically test against it.
@fabpot: I will do a seperate PR on master when you merged this because the code changed alot in master so it cannot easily be merged I guess. I will also convert the symfony requirement for PCRE in the requirements check to a recommendation.
This PR was merged into the master branch.
Commits
-------
acbb393 Renamed variable for consistency
Discussion
----------
[SecurityBundle] Renamed variable for consistency
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: -
This PR was merged into the 2.1 branch.
Commits
-------
ea2bb09 tiny refactoring for consistency
Discussion
----------
tiny refactoring for consistency
no need to use the iterator within the class. not done anywhere else.
This PR was merged into the master branch.
Commits
-------
966e7d6 [DI] removed unneeded is_object() calls
Discussion
----------
[DI] removed unneeded is_object() calls
I searched through all of symfony for occurences of the coding style `(is_object($value) && $value instanceof Object)` with a regex like `is_object\(\$[a-zA-z0-9]+\) && \$[a-zA-z0-9]+ instanceof `.
The `is_object` calls are not needed in this case. Only the DI component made such duplicate checks.
This PR was merged into the master branch.
Commits
-------
acf8a70 [Routing] fix Route recompilation when hostname changed
Discussion
----------
[Routing] fix Route recompilation when hostname changed
This PR was squashed before being merged into the master branch (closes#6030).
Commits
-------
749dac1 Improve docBlock
Discussion
----------
Improve docBlock
This is just a minor change documenting the return type of `SerializerInterface::deserialize()`.
This PR was merged into the master branch.
Commits
-------
644de74 Fix docblock in Doctrine Bridge
Discussion
----------
Fix docblocks in Doctrine Bridge
