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

Fix env fallback to an unresolved variable #29409

Merged
merged 1 commit into from Dec 17, 2018

Conversation

Projects
None yet
3 participants
@jderusse
Copy link
Contributor

jderusse commented Dec 1, 2018

Q A
Branch? 4.2
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

This PR fixes a tricky case where an env processor require to resolve another parameter.

For example, allowing a user to either pass a PEM key or the path to a PEM key, I could use this config

parameters:
  private_key: '%env(default:raw_key:file:PRIVATE_KEY)%'
  raw_key: '%env(PRIVATE_KEY)%'

here, the default processor, performs a $container->getParameter('raw_key') which fail because raw_key has to be resolved to.

I'm not sure, if this is the right way to do it, please review it carefully.

Here is my reproduct case

<?php


require __DIR__.'/vendor/autoload.php';

use Symfony\Component\DependencyInjection\ContainerBuilder;

$containerBuilder = new ContainerBuilder();
$containerBuilder->setParameter('env(PRIVATE_KEY)', 'FOO');
$containerBuilder->setParameter('raw_key', '%env(PRIVATE_KEY)%');
$containerBuilder->setParameter('private_key', '%env(default:raw_key:file:PRIVATE_KEY)%');
$containerBuilder->compile(true);

var_dump($containerBuilder->getParameter('private_key'));

@jderusse jderusse force-pushed the jderusse:fallback-to-env branch from d7908f9 to 6682b44 Dec 1, 2018

@nicolas-grekas nicolas-grekas added this to the 4.2 milestone Dec 1, 2018

@jderusse jderusse force-pushed the jderusse:fallback-to-env branch from 6682b44 to a7456b9 Dec 3, 2018

@nicolas-grekas
Copy link
Member

nicolas-grekas left a comment

LGTM

@nicolas-grekas nicolas-grekas changed the base branch from master to 4.2 Dec 17, 2018

@nicolas-grekas nicolas-grekas force-pushed the jderusse:fallback-to-env branch from a7456b9 to ad6df01 Dec 17, 2018

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Dec 17, 2018

Thank you @jderusse.

@nicolas-grekas nicolas-grekas merged commit ad6df01 into symfony:4.2 Dec 17, 2018

0 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build cancelled
Details
continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
fabbot.io Some changes should be done to comply with our standards.
Details

nicolas-grekas added a commit that referenced this pull request Dec 17, 2018

bug #29409 Fix env fallback to an unresolved variable (jderusse)
This PR was submitted for the master branch but it was merged into the 4.2 branch instead (closes #29409).

Discussion
----------

Fix env fallback to an unresolved variable

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

This PR fixes a tricky case where an env processor require to resolve another parameter.

For example, allowing a user to either pass a PEM key or the path to a PEM key, I could use this config
```
parameters:
  private_key: '%env(default:raw_key:file:PRIVATE_KEY)%'
  raw_key: '%env(PRIVATE_KEY)%'
```

here, the default processor, performs a `$container->getParameter('raw_key')` which fail because `raw_key` has to be resolved to.

I'm not sure, if this is the right way to do it, please review it carefully.

Here is my reproduct case
```
<?php

require __DIR__.'/vendor/autoload.php';

use Symfony\Component\DependencyInjection\ContainerBuilder;

$containerBuilder = new ContainerBuilder();
$containerBuilder->setParameter('env(PRIVATE_KEY)', 'FOO');
$containerBuilder->setParameter('raw_key', '%env(PRIVATE_KEY)%');
$containerBuilder->setParameter('private_key', '%env(default:raw_key:file:PRIVATE_KEY)%');
$containerBuilder->compile(true);

var_dump($containerBuilder->getParameter('private_key'));
```

Commits
-------

ad6df01 Fix env fallback to an unresolved variable

nicolas-grekas added a commit that referenced this pull request Dec 17, 2018

Revert "bug #29409 Fix env fallback to an unresolved variable (jderus…
…se)"

This reverts commit ab95ae3, reversing
changes made to 8a60907.

nicolas-grekas added a commit that referenced this pull request Dec 17, 2018

Merge branch '4.2'
* 4.2:
  Revert "bug #29409 Fix env fallback to an unresolved variable (jderusse)"

@fabpot fabpot referenced this pull request Jan 6, 2019

Merged

Release v4.2.2 #29803

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