merged branch ajessu/php54-server-fix (PR symfony#4484)

fabpot · fabpot · commit 3f44bba01e76 · 2012-06-19T10:56:44.000+02:00
Commits ------- d982bac Fix built-in server for PHP > 5.4.1 Discussion ---------- [FrameworkBundle] Fix built-in server command for PHP > 5.4.1 Bug fix: yes Feature addition: no Backwards compatibility break: yes Symfony2 tests pass: yes License of the code: MIT The router isn't routing with PHP > 5.4.1, unless you explicitly include the name of the controller. For the default command: `app/console server:run` localhost:8000/app_dev.php `Works` localhost:8000/ `Doesn't work (it used to work on PHP 5.4.0`) There was a change after PHP 5.4.1 which makes the router from the built-in server command not work, when no resource is specified, as the variable `$_SERVER['SCRIPT_FILENAME']` passes the `isset` check. Changelog: http://php.net/ChangeLog-5.php#5.4.1 - Implemented #60850 (Built in web server does not set $_SERVER['SCRIPT_FILENAME'] when using router) The `router` used to rely on the `$_SERVER['SCRIPT_FILENAME']` being set, to return any asset/file if it existed. This behavior was changed, so that when using PHP's built-in server, the `$_SERVER['SCRIPT_FILENAME']` is now populated with a combination of the document root and the router filename Patch: https://bugs.php.net/patch-display.php?bug_id=60850&patch=add_router_script_file_name_svr_var&revision=latest) --------------------------------------------------------------------------- by travisbot at 2012-06-02T09:06:05Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1506479) (merged b85ff7dd into 1541fe2). --------------------------------------------------------------------------- by ajessu at 2012-06-03T07:16:33Z Thinking a bit more about this, as I find my solution a bit weird. I'll do a bit more testing and report back. --------------------------------------------------------------------------- by fabpot at 2012-06-13T14:30:28Z Any news on this PR? --------------------------------------------------------------------------- by travisbot at 2012-06-18T21:20:17Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1650548) (merged d982bac into 086ff48). --------------------------------------------------------------------------- by ajessu at 2012-06-18T21:35:44Z Updated the solution, and for me, it's ready to be merged now. I talked about this on Symfony Live with @michal-pipa (the original contributor to this command) and we were trying to avoid touching the filesystem for the check of file existance to skip the server, but I don't think there is any other way (and it's dev mode, it really shouldn't matter much either). PHP 5.4.1 and up has changed the behavior of `$_SERVER['SCRIPT_FILENAME']` for the built-in server and it's a bit unreliable/ugly to rely on it now. This fixes the command, it works again for all versions of PHP 5.4.x A very similar solution was also suggested on internals: http://news.php.net/php.internals/53870 @michal-pipa any other ideas? --------------------------------------------------------------------------- by michal-pipa at 2012-06-18T23:14:36Z I'll take a closer look at this tomorrow. But I think that you should revert to original behavior and call production front controller by default to be consistent with other servers. --------------------------------------------------------------------------- by ajessu at 2012-06-19T08:48:17Z > But I think that you should revert to original behavior and call production front controller by default to be consistent with other servers. I disagree. This is a development-only server, and thus, the development controller should be called since IMHO it will be the most common use case for the command, development. If for some reason, someone wants to use their production controller, it's as easy as providing a new router and passing it to the command explicitly. Let me know if you come up with something else.
diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/router.php b/src/Symfony/Bundle/FrameworkBundle/Resources/config/router.php
@@ -18,12 +18,13 @@
  * and pass it as a value to 'router' option of server:run command.
  *
  * @author: Michał Pipa <michal.pipa.xsolve@gmail.com>
+ * @author: Albert Jessurum <ajessu@gmail.com>
  */
 
-if (isset($_SERVER['SCRIPT_FILENAME'])) {
+if (is_file($_SERVER['DOCUMENT_ROOT'] . DIRECTORY_SEPARATOR . $_SERVER['REQUEST_URI'])) {
     return false;
 }
 
-$_SERVER['SCRIPT_FILENAME'] = $_SERVER['DOCUMENT_ROOT'].DIRECTORY_SEPARATOR.'app_dev.php';
+$_SERVER['SCRIPT_FILENAME'] = $_SERVER['DOCUMENT_ROOT'] . DIRECTORY_SEPARATOR . 'app_dev.php';
 
 require 'app_dev.php';

Original file line number	Diff line number	Diff line change
`@@ -18,12 +18,13 @@`
`18`	`18`	`* and pass it as a value to 'router' option of server:run command.`
`19`	`19`	`*`
`20`	`20`	`* @author: Michał Pipa <michal.pipa.xsolve@gmail.com>`
	`21`	`+ * @author: Albert Jessurum <ajessu@gmail.com>`
`21`	`22`	`*/`
`22`	`23`
`23`		`-if (isset($_SERVER['SCRIPT_FILENAME'])) {`
	`24`	`+if (is_file($_SERVER['DOCUMENT_ROOT'] . DIRECTORY_SEPARATOR . $_SERVER['REQUEST_URI'])) {`
`24`	`25`	`return false;`
`25`	`26`	`}`
`26`	`27`
`27`		`-$_SERVER['SCRIPT_FILENAME'] = $_SERVER['DOCUMENT_ROOT'].DIRECTORY_SEPARATOR.'app_dev.php';`
	`28`	`+$_SERVER['SCRIPT_FILENAME'] = $_SERVER['DOCUMENT_ROOT'] . DIRECTORY_SEPARATOR . 'app_dev.php';`
`28`	`29`
`29`	`30`	`require 'app_dev.php';`