Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[DI] allow "." and "-" in env processor lines#35029
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| throw new InvalidArgumentException(sprintf('Invalid %s name: only "word" characters are allowed.', $name)); | ||
| } | ||
| if ($this->has($name) && null !== ($defaultValue = parent::get($name)) && !\is_string($defaultValue)) { | ||
| throw new RuntimeException(sprintf('The default value of an env() parameter must be a string or null, but "%s" given to "%s".', \gettype($defaultValue), $name)); | ||
| } | ||
| $uniqueName = md5($name.'_'.self::$counter++); | ||
| $placeholder = sprintf('%s_%s_%s', $this->getEnvPlaceholderUniquePrefix(),str_replace(':', '_', $env), $uniqueName); | ||
| $placeholder = sprintf('%s_%s_%s', $this->getEnvPlaceholderUniquePrefix(),strtr($env, ':-.', '___'), $uniqueName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
replacing the 3 chars with the same replacement will be an issue: we can have multiple env lines producing the same placeholder, which will go wrong when reverting them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
There is an md5 on the line before that will prevent any collisions from happening.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
ah indeed. I missed that.
Thank you@nicolas-grekas. |
…-grekas)This PR was merged into the 5.1-dev branch.Discussion----------[DI] allow "." and "-" in env processor lines| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| Deprecations? | no| Tickets |Fix#34864| License | MIT| Doc PR | -As explained in the linked issue, this is especially usefull with the `key` processor.Commits-------231c505 [DI] allow "." and "-" in env processor lines
This PR was merged into the 4.4 branch.Discussion----------[DI] fix dumping env vars| Q | A| ------------- | ---| Branch? | 4.4| Bug fix? | yes| New feature? | no| Deprecations? | no| Tickets | -| License | MIT| Doc PR | -Forgotten in#35029For 4.4 to reduce merge conflicts (it doesn't allow the new chars in 4.4 anyway,#35029 is still needed)Commits-------746a8d1 [DI] fix dumping env vars
As explained in the linked issue, this is especially usefull with the
keyprocessor.