Skip to content

[Process] Fix finding executables independently of open_basedir #58291

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 17, 2024
Merged

[Process] Fix finding executables independently of open_basedir #58291

merged 1 commit into from
Sep 17, 2024

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Issues -
License MIT

This backports #47422 to 5.4, which is a bugfix really.

Instead of #58008 and #57954 /cc @xabbuh @fritzmg

@carsonbot carsonbot added this to the 5.4 milestone Sep 17, 2024
This was referenced Sep 17, 2024
Closed
Closed
@nicolas-grekas
Copy link
Member Author

Thank you @BlackbitDevs.

@nicolas-grekas nicolas-grekas merged commit 796fff2 into symfony:5.4 Sep 17, 2024
11 of 12 checks passed
fritzmg
fritzmg reviewed Sep 17, 2024
View reviewed changes
Copy link
Contributor

@fritzmg fritzmg left a comment •
edited
Loading

Choose a reason for hiding this comment

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

But shouldn't the open_basedir paths be checked additionally? After all, open_basedir could contain other paths with available executables than PATH. This is why I did not consider just backporting the state of Symony 6+ to 5.4.

@nicolas-grekas nicolas-grekas deleted the proc-openbasedir branch September 17, 2024 12:53
@nicolas-grekas
Copy link
Member Author

That'd be incorrect, open_basedir is not meant to replace PATH

@fritzmg
Copy link
Contributor

fritzmg commented Sep 17, 2024

That'd be incorrect, open_basedir is not meant to replace PATH

That was not my point. open_basedir and PATH are two completely different things. My point is that an executable that is available in the system does not necessarily have to be defined in PATH - but its location might be still hinted via the open_basedir setting in some environments.

However, since the usage of open_basedir is discouraged anyway, this likely will not matter that much in the end.

@stof
Copy link
Member

stof commented Sep 17, 2024

@fritzmg open_basedir is not about hinting about the location of executables at all.

@fritzmg
Copy link
Contributor

fritzmg commented Sep 17, 2024 •
edited
Loading

@fritzmg open_basedir is not about hinting about the location of executables at all.

Not directly, no. But if a hosting environment employs open_basedir, it needs to add all paths to exectuables that the PHP environment should have access to to the open_basedir paths. Hence why open_basedir can be used as an additional source to finding executables via the ExecutableFinder.

This was referenced Sep 21, 2024
Merged
Merged
Merged
@richard67 richard67 mentioned this pull request Feb 1, 2025
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fritzmg fritzmg fritzmg left review comments

@stof stof stof approved these changes

Assignees
No one assigned
Labels
Bug Process Status: Reviewed
Projects
None yet
Milestone
5.4
Development

Successfully merging this pull request may close these issues.
5 participants
@nicolas-grekas @fritzmg @stof @carsonbot @BlackbitDevs