Skip to content
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

[Process] Fix ignoring of bad env var names #21776

Merged
merged 1 commit into from
Feb 27, 2017

Conversation

nicolas-grekas
Copy link
Member

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

Patch backported from master, which is free from the linked issue.

@hboomsma
Copy link

👍

$this->env = array();
foreach ($env as $key => $value) {
$this->env[$key] = (string) $value;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that logic removed or is it somewhere else?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any place where it is moved, meaning it is a BC break (btw, I remember having commented on it for the refactoring in 3.3 too already)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reverted

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it mean that this should be reverted on 3.3 as well?

Copy link
Member Author

@nicolas-grekas nicolas-grekas Feb 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only the array_filter part - the string cast should not happen here because it removes the possibility to unset an env var which is a feature on master

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, I did it in this PR: keep only the array_filter part.
That will conflict with master so we'll have a reminder.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, you still cast to string later in the process, after having checked for env removals?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes - see concat when calling putenv - and proc_open does it also (just confirmed)

@fabpot
Copy link
Member

fabpot commented Feb 27, 2017

Thank you @nicolas-grekas.

@fabpot fabpot merged commit 406bb09 into symfony:3.2 Feb 27, 2017
fabpot added a commit that referenced this pull request Feb 27, 2017
This PR was merged into the 3.2 branch.

Discussion
----------

[Process] Fix ignoring of bad env var names

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

Patch backported from master, which is free from the linked issue.

Commits
-------

406bb09 [Process] Fix ignoring of bad env var names
@nicolas-grekas nicolas-grekas deleted the proc-fix-env branch February 28, 2017 08:48
@fabpot fabpot mentioned this pull request Mar 9, 2017
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.

5 participants