[FrameworkBundle] Enable possibility to run PHP bultin server in production env #7507

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
4 participants
@canni
Contributor

canni commented Mar 28, 2013

See the referenced ticket for discussion;

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #7430
License MIT
@stof

View changes

src/Symfony/Bundle/FrameworkBundle/Command/ServerRunCommand.php
+
+ if ('prod' === $env) {
+ $output->writeln(
+ '<error>Running PHP bultin server in production environment is NOT secure!</error>'

This comment has been minimized.

Show comment Hide comment
@stof

stof Mar 28, 2013

Member

typo here

@stof

stof Mar 28, 2013

Member

typo here

@canni

This comment has been minimized.

Show comment Hide comment
@canni

canni Mar 28, 2013

Contributor

@stof corrected thx

Contributor

canni commented Mar 28, 2013

@stof corrected thx

@fabpot

View changes

src/Symfony/Bundle/FrameworkBundle/Command/ServerRunCommand.php
+
+ if ('prod' === $env) {
+ $output->writeln(
+ '<error>Running PHP built-in server in production environment is NOT secure!</error>'

This comment has been minimized.

Show comment Hide comment
@fabpot

fabpot Apr 1, 2013

Owner

Can you put everything on the same line? Also, it's not just about it being not secure, so I think something like "Running the PHP built-in server in a production environment is NOT recommended" would be better.

@fabpot

fabpot Apr 1, 2013

Owner

Can you put everything on the same line? Also, it's not just about it being not secure, so I think something like "Running the PHP built-in server in a production environment is NOT recommended" would be better.

$router = $input->getOption('router') ?: $this
->getContainer()
->get('kernel')
- ->locateResource('@FrameworkBundle/Resources/config/router.php')
+ ->locateResource(sprintf(

This comment has been minimized.

Show comment Hide comment
@fabpot

fabpot Apr 1, 2013

Owner

Same as above, the whole method call should be on the same line.

@fabpot

fabpot Apr 1, 2013

Owner

Same as above, the whole method call should be on the same line.

This comment has been minimized.

Show comment Hide comment
@merk

merk Apr 1, 2013

Contributor

Should there be error handling if someone tries to run this command in another environment and the router doesnt exist?

@merk

merk Apr 1, 2013

Contributor

Should there be error handling if someone tries to run this command in another environment and the router doesnt exist?

This comment has been minimized.

Show comment Hide comment
@canni

canni Apr 1, 2013

Contributor

Someone will see nice error about failed resource lock-up, not some "require" fatal :)

@canni

canni Apr 1, 2013

Contributor

Someone will see nice error about failed resource lock-up, not some "require" fatal :)

This comment has been minimized.

Show comment Hide comment
@stof

stof Apr 4, 2013

Member

Thus, you can probably create it in app/Resources/FrameworkBundle/config/router_my_env.php for other environments. So letting the resource locator handle missing files is easier.

@stof

stof Apr 4, 2013

Member

Thus, you can probably create it in app/Resources/FrameworkBundle/config/router_my_env.php for other environments. So letting the resource locator handle missing files is easier.

@fabpot

View changes

src/Symfony/Bundle/FrameworkBundle/Resources/config/router_prod.php
+ * @author: Albert Jessurum <ajessu@gmail.com>
+ */
+
+if (is_file($_SERVER['DOCUMENT_ROOT'] . DIRECTORY_SEPARATOR . $_SERVER['SCRIPT_NAME'])) {

This comment has been minimized.

Show comment Hide comment
@fabpot

fabpot Apr 1, 2013

Owner

we don't put whitespace before and after a dot in Symfony.

@fabpot

fabpot Apr 1, 2013

Owner

we don't put whitespace before and after a dot in Symfony.

@canni

This comment has been minimized.

Show comment Hide comment
@canni

canni Apr 1, 2013

Contributor

@fabpot corrected

Contributor

canni commented Apr 1, 2013

@fabpot corrected

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment