Skip to content

external_parameters access in parameters.yml using _SERVER[...], not _ENV[...], superglobal #23348

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ghost opened this issue Jul 1, 2017 · 7 comments

Comments

@ghost
Copy link

ghost commented Jul 1, 2017

Bug report?
no. well, maybe docs/best-practices?

Feature request?
no

BC Break report?
no

RFC?
no

Symfony version
3.3.2

Reading

	How to Set external Parameters in the Service Container
	 http://symfony.com/doc/current/configuration/external_parameters.html

usage of SYMFONY__ vars has been deprecated,

	New in version 3.3: The support of the special SYMFONY__ environment variables was deprecated in Symfony 3.3 and it will be removed in 4.0. Instead of using those variables, define regular environment variables and get their values using the %env(...)% syntax in your config files.

In my SF 3.3 proj, I config to use env vars for DB credentials,

	cat app/config/paramaters.yml
		parameters:
		    database_host: '%env(SF_DB_HOST)%'
		    database_port: '%env(SF_DB_PORT)%'
		    database_name: '%env(SF_DB_NAME)%'
		    database_user: '%env(SF_DB_USER)%'
		    database_PASS: '%env(SF_DB_PASS)%'
		    database_path:   null
		    database_memory: null
		    env(SF_DB_HOST): localhost
		    env(SF_DB_PORT): 3306
		    env(SF_DB_NAME): noop_name
		    env(SF_DB_USER): noop_user
		    env(SF_DB_PASS): noop_PASS
		    ...

In nginx config, I define server vars using php-fpm's fastcgi_param,

	cat sites-enabled/vhost01.conf
		...
		location ~ ^/app\.php(/|$) {
			fastcgi_param SF_DB_HOST "localhost"
			fastcgi_param SF_DB_PORT "3306"
			fastcgi_param SF_DB_NAME "secure_name"
			fastcgi_param SF_DB_USER "secure_user"
			fastcgi_param SF_DB_PASS "secure_pass";
			...

in php.ini, I've ensured

	php -i | egrep -i "variables_order|request_order"
		request_order => EGP => EGP
		variables_order => EGPCS => EGPCS

and in my php-fpm pool conf

	...
	[www]
	clear_env                 = no
	env[SF_DB_PASS] = $_SERVER['SF_DB_PASS']
	env[SF_DB_USER] = $_SERVER['SF_DB_USER']
	env[SF_DB_NAME] = $_SERVER['SF_DB_NAME']
	env[SF_DB_PORT] = $_SERVER['SF_DB_PORT']
	env[SF_DB_HOST] = $_SERVER['SF_DB_HOST']
	...

but on webserver + php-fpm restart,

	phpinfo()
		...
		PHP Variables
			...
			$_SERVER['SF_DB_HOST']	localhost
			$_SERVER['SF_DB_PORT']	3306
			$_SERVER['SF_DB_NAME']	secure_name
			$_SERVER['SF_DB_USER']	secure_user
			$_SERVER['SF_DB_PASS']	secure_pass
			...

are set as expected, but the ENV vars -- needed by SF -- are not

	phpinfo()
		...
		Environment
			...
			SF_DB_HOST 	no value
			SF_DB_PORT 	no value
			SF_DB_NAME 	no value
			SF_DB_USER 	no value
			SF_DB_PASS 	no value
			...

php7's php.ini contains comment that ENV var use is not recommended

	; This directive determines which super global arrays are registered when PHP
	; starts up. G,P,C,E & S are abbreviations for the following respective super
	; globals: GET, POST, COOKIE, ENV and SERVER. There is a performance penalty
	; paid for the registration of these arrays and because ENV is not as commonly
	; used as the others, ENV is not recommended on productions servers.

this thread

	https://stackoverflow.com/questions/8551592/setting-env-fka-http-env-vars-with-nginx-php-fpm

suggests that ENV setting in php-fpm is not reliable (?).

These issues

	SYMFONY__ Env Variables overwriten by parameters.yml #7555
	https://github.com/symfony/symfony/issues/7555

	Symfony2+nginx external ENV vars aren't recognized by doctrine DB actions. fails with 'connection refused'.
	https://github.com/symfony/symfony/issues/14426

appear to still refer to / depend on SYMFONY__ env vars, which are, as above, deprecated.

So the question is how, going forward, to access _SERVER[...] vars in parameters/yml so, for example, doctrine:... cmds can use them?

@nicolas-grekas
Copy link
Member

I don't get this report, it's too long :)
Basically, we use getenv() to ready env vars when $_ENV is disabled.
What's the actual issue you have?

@ghost
Copy link
Author

ghost commented Jul 28, 2017

_SERVER vars are set as expected, but the ENV vars -- needed by SF -- are not

@nicolas-grekas
Copy link
Member

So, fastcgi_param in nginx config only populates $_SERVER, and not getenv()? Too bad. We need to handle that somehow.

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Jul 28, 2017
@ghost
Copy link
Author

ghost commented Jul 28, 2017

only populates $_SERVER, and not getenv()?

as shown in the detail of my 'long' report ;-), it appears so.

seems pretty clear from reading, at least, that use of ENV is not encouraged

@nicolas-grekas
Copy link
Member

@ghost
Copy link
Author

ghost commented Jul 28, 2017

got it. though that report is a little thin on the 'why'. coulda been longer! ;-p

anyway, thanks for queuing this up.

@nicolas-grekas nicolas-grekas self-assigned this Jul 28, 2017
@ghost
Copy link
Author

ghost commented Jul 28, 2017

fyi

Trying to figure out where a 'fix' should best target ... nginx, fcgi/fpm, php, or symfony, I stumbled on this dusty issue:

getenv() not working in HHVM-FastCGI
https://github.com/facebook/hhvm/issues/1650

tho, seems to me that _SERVER vars are the right direction

@nicolas-grekas nicolas-grekas removed their assignment Aug 16, 2017
nicolas-grekas added a commit that referenced this issue Aug 17, 2017
…kas)

This PR was merged into the 3.3 branch.

Discussion
----------

[DI] Fix reading env vars from fastcgi params

| Q             | A
| ------------- | ---
| Branch?       | 3.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #23348
| License       | MIT
| Doc PR        | -

Values in fastcgi_param populate `$_SERVER`, never `$_ENV`.
This PR makes `$container->getEnv()` read from `$_SERVER`, excluding any vars whose name start by `HTTP_` as that would be a security issue (values injection via HTTP headers.)

Embeds a few other fixes found meanwhile.

Commits
-------

adff65a [DI] Fix reading env vars from fastcgi params
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants