Skip to content

[DI] Fix reading env vars from fastcgi params #23899

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

Merged
merged 1 commit into from
Aug 17, 2017

Conversation

nicolas-grekas
Copy link
Member

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.

@nicolas-grekas nicolas-grekas force-pushed the di-fix branch 2 times, most recently from d829791 to 8477c91 Compare August 16, 2017 12:45
@nicolas-grekas
Copy link
Member Author

Merging to unlock other PRs.
But review still welcomed.

@nicolas-grekas nicolas-grekas merged commit adff65a into symfony:3.3 Aug 17, 2017
nicolas-grekas added a commit that referenced this pull request 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
@nicolas-grekas nicolas-grekas deleted the di-fix branch August 17, 2017 12:10
@fabpot fabpot mentioned this pull request Aug 28, 2017
fabpot added a commit that referenced this pull request Sep 7, 2017
This PR was merged into the 3.4 branch.

Discussion
----------

[DI] Allow processing env vars

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | see description
| License       | MIT
| Doc PR        | -

This PR is an updated version of #20276 ~~(it embeds #23899 for now.)~~

It superscedes/closes:
- [DI] Add support for secrets #23621 ping @dunglas
- Runtime container parameter not found event filter #23669 ping @marfillaster
- [DependencyInjection] [DX] Support for JSON string environment variables #23823 ping @Pierstoval
- add support for composite environment variables #17689 ping @greg0ire
- [DI] ENV parameters at runtime with PHP 7 strict types not working properly #20434 ping @sandrokeil
- Issue when using a SQLite database and the DATABASE_URL env var #23527 ping @javiereguiluz

#22151 is another story, so not fixed here.

The way it works is via `%env(foo:BAR)%` prefixes, where "foo" can be bound to any services you'd like.
By default, the following prefixes are supported:
- `bool`, `int`, `float`, `string`, `base64`
- `const` (for referencing PHP constants)
- `json` (supporting only json **arrays** for type predictability)
- `file` (eg for access to secrets stored in files.)
- `resolve` (for processing parameters inside env vars.)

New prefixes can be added by implementing the new `EnvProviderInterface`, and tagging with `container.env_provider` (see `Rot13EnvProvider` in tests.)

Prefixes can be combined to chain processing, eg.
`%env(json:base64:file:FOO)%` will be roughly equivalent to
`json_decode(base64_decode(file_get_content(getenv('FOO'))))`.

Commits
-------

1f92e45 [DI] Allow processing env vars
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants