Commit Graph

10369 Commits

Author SHA1 Message Date
Fabien Potencier
c4a33713a6 merged branch Tobion/request (PR #4737)
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.
2012-07-04 07:18:32 +02:00
Fabien Potencier
8d84e75a75 merged branch Tobion/prevsession (PR #4736)
Commits
-------

4d0ae1f [HttpFoundation] refactored hasPreviousSession

Discussion
----------

[HttpFoundation] refactored hasPreviousSession
2012-07-04 07:16:31 +02:00
Tobias Schultze
d37003ec56 [HttpFoundation] small fixes in Request 2012-07-04 04:13:21 +02:00
Tobias Schultze
4d0ae1fb0c [HttpFoundation] refactored hasPreviousSession 2012-07-04 03:41:37 +02:00
Tobias Schultze
c40a4e50a9 [HttpFoundation] fix query string normalization 2012-07-04 03:10:48 +02:00
Tobias Schultze
f9ec2ea3be refactored test method 2012-07-04 03:05:07 +02:00
Tobias Schultze
0880174a54 [HttpFoundation] added failing tests for query string normalization 2012-07-04 02:42:19 +02:00
Fabien Potencier
af57fe0f3a merged branch Seldaek/processfinder (PR #4731)
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
2012-07-03 21:44:11 +02:00
Fabien Potencier
037b4d8096 [ClassLoader] fixed typo 2012-07-03 21:28:10 +02:00
Fabien Potencier
391ee67d68 merged branch vicb/profiler (PR #4727)
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 ?
2012-07-03 19:54:59 +02:00
Fabien Potencier
a5f257210b merged branch bschussek/issue4480 (PR #4732)
Commits
-------

45d967e [Form] Fixed: Empty forms can be compound too

Discussion
----------

[Form] Fixed: Empty forms can be compound too

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #4480
Todo: -
2012-07-03 19:47:53 +02:00
Bernhard Schussek
45d967e95e [Form] Fixed: Empty forms can be compound too 2012-07-03 19:10:17 +02:00
Fabien Potencier
6f4d28181a [ClassLoader] added missing support for PHP 5.4 traits 2012-07-03 19:09:11 +02:00
Fabien Potencier
85977649a4 fixed unit tests 2012-07-03 19:06:57 +02:00
Jordi Boggiano
45219ef80f [Process] Add default xampp path to the list of possible paths to check 2012-07-03 18:58:37 +02:00
Jordi Boggiano
28e1313e5d [Process] Clean-up/simplify code 2012-07-03 18:58:27 +02:00
Fabien Potencier
3cfe916e65 [FrameworkBundle] fixed some unit tests 2012-07-03 18:55:00 +02:00
Jordi Boggiano
56f473a073 [Process] Add extra dirs argument to the executable finder to allow searching more dirs 2012-07-03 18:19:03 +02:00
Victor Berchet
eda439ffe5 [EventDataCollector] Display a better message when no events have been recorded 2012-07-03 18:15:27 +02:00
Victor Berchet
6b87981641 [TimeDataCollector] Do not throw an exception when no events are recorded 2012-07-03 18:15:20 +02:00
Fabien Potencier
23f41a21ca merged branch vicb/templates (PR #4723)
Commits
-------

aef7663 [FrameworkBundle] Create a dedicated template filename parser

Discussion
----------

[FrameworkBundle] Create a dedicated template filename parser

Bug fix: no
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes

related to #3116
2012-07-03 17:47:16 +02:00
Fabien Potencier
4bb1431720 merged branch jmikola/patch-2 (PR #4728)
Commits
-------

0fe56e6 [Config] Fix comment typo

Discussion
----------

[Config] Fix comment typo
2012-07-03 17:43:58 +02:00
Jeremy Mikola
0fe56e6012 [Config] Fix comment typo 2012-07-03 12:34:31 -03:00
Fabien Potencier
480323c4e9 merged branch mageekguy/master (PR #4725)
Commits
-------

6155583 Remove bug in locale management in listener.

Discussion
----------

Remove bug in locale listener
2012-07-03 16:27:02 +02:00
Frédéric Hardy
615558394a Remove bug in locale management in listener. 2012-07-03 16:15:48 +02:00
Fabien Potencier
3ee1b383af merged branch vicb/auto_start (PR #4724)
Commits
-------

c5470b0 [Session] Removes references to the deprecated 'auto_start' setting

Discussion
----------

[Session] Removes references to the deprecated 'auto_start' setting

fix #4721
2012-07-03 16:13:04 +02:00
Victor Berchet
c5470b06bb [Session] Removes references to the deprecated 'auto_start' setting 2012-07-03 15:44:06 +02:00
Victor Berchet
aef7663676 [FrameworkBundle] Create a dedicated template filename parser 2012-07-03 14:58:30 +02:00
Fabien Potencier
2335dd0d60 merged branch bamarni/compile-classes (PR #4694)
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
2012-07-03 14:53:35 +02:00
Bilal Amarni
26a1e0be5e [ClassLoader] ordered ClassCollectionLoader writing to avoid redeclaration at runtime 2012-07-03 14:40:59 +02:00
Fabien Potencier
2f66ae3a0c merged branch Tobion/encode-rel-path (PR #4722)
Commits
-------

5c6f848 [Routing] use faster approach for encoding rel segments
25d326b [Routing] fix encoding of path segments '.' and '..'

Discussion
----------

[Routing] fix encoding of path segments '.' and '..'

#4559 for master
2012-07-03 12:59:10 +02:00
Tobias Schultze
5c6f848a7d [Routing] use faster approach for encoding rel segments
replaced the preg_replace code that was 1.4 times slower than the new code (verified with benchmark
2012-07-03 12:52:06 +02:00
Tobias Schultze
25d326b55e [Routing] fix encoding of path segments '.' and '..' 2012-07-03 12:52:05 +02:00
Fabien Potencier
e9477820f2 merged branch vicb/framework/config (PR #4721)
Commits
-------

816539b [FrameworkBundle] Display an error message when 'session.auto_start' is used (deprecated)

Discussion
----------

[FrameworkBundle] Display an error message when 'session.auto_start' is ...

...used (deprecated)
2012-07-03 12:24:04 +02:00
Tobias Schultze
51b610f8ba [Profiler] fix typehint 2012-07-03 13:04:37 +03:00
Victor Berchet
816539b110 [FrameworkBundle] Display an error message when 'session.auto_start' is used (deprecated) 2012-07-03 11:35:02 +02:00
Fabien Potencier
11e8a33c7c merged branch Tobion/4166 (PR #4530)
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).
2012-07-03 11:09:19 +02:00
Fabien Potencier
bf59b8677c merged branch fabpot/charset-fix (PR #4716)
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.
2012-07-03 10:43:15 +02:00
Fabien Potencier
d9439aba71 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';
    }
2012-07-03 10:28:30 +02:00
Fabien Potencier
f47b9a6625 [WebProfilerBundle] inlined a service (closes #4717) 2012-07-03 10:02:51 +02:00
Fabien Potencier
736aa210a2 merged branch simensen/normalize-querystring (PR #4711)
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.
2012-07-02 22:18:33 +02:00
Beau Simensen
6296a241a8 Standalone query string normalization 2012-07-02 13:11:17 -07:00
Fabien Potencier
67a69ea357 [Security] updated CHANGELOG 2012-07-02 19:29:27 +02:00
Fabien Potencier
637aaacccb merged branch uwej711/security_target_path_master (PR #4409)
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?
2012-07-02 19:27:21 +02:00
Fabien Potencier
b26cd4c080 merged branch avorobiev/fix1 (PR #4708)
Commits
-------

036c15e [DependencyInjection] Unescape class arguments part 2
19bdae1 [DependencyInjection] Fixed unescaping of class arguments

Discussion
----------

[DependencyInjection] Fixed unescaping of class arguments

https://github.com/symfony/symfony/pull/4707
2012-07-02 18:58:19 +02:00
avorobiev
036c15ecfa [DependencyInjection] Unescape class arguments part 2 2012-07-02 18:17:30 +04:00
avorobiev
19bdae1b90 [DependencyInjection] Fixed unescaping of class arguments 2012-07-02 18:10:38 +04:00
Fabien Potencier
81fe2ff8e2 merged branch fabpot/locale-listener (PR #4692)
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.
2012-07-02 12:37:57 +02:00
Fabien Potencier
88caf3a370 [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.
2012-07-02 12:36:25 +02:00
Fabien Potencier
7407773234 [WebProfilerBundle] tweaked previous merge 2012-07-02 11:12:51 +02:00