Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[Process] Support usingProcess::findExecutable() independently ofopen_basedir#47422

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

Conversation

@BlackbitDevs
Copy link
Contributor

QA
Bug fix?no
New feature?yes
Deprecations?no
TicketsFix#41006
LicenseMIT

With this PRConsole::findExecutable() will find executables also if the their path is not allowed inopen_basedir config similar to

$command ='\\' === \DIRECTORY_SEPARATOR ?'where' :'command -v';
if ($php =strtok(exec($command.''.escapeshellarg($php)), \PHP_EOL)) {
if (!is_executable($php)) {
returnfalse;
}
}else {

Console::findExecutable()'s responsibility is to find an executable which can be called with a Symfony Process or by PHP's functions likeexec,system etc.

The goal of PHP'sopen_basedir config is to restrict reading / writing files within PHP processes. Imho this is completely independent of finding an executable.

If PHP's intention was to restrict executing applications which are not present inopen_basedir's paths, it would have been implemented there.

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

LGTM. There's a related failure on Windows, see appveyor.
Please add a test case also.

@BlackbitDevs
Copy link
ContributorAuthor

BlackbitDevs commentedSep 13, 2022•
edited
Loading

@nicolas-grekas

There's a related failure on Windows, see appveyor.
Please add a test case also.

Can you point me where the error occurs? Have bit of a problem interprertinghttps://ci.appveyor.com/project/fabpot/symfony/builds/44759158 - I only seeCommand exited with code 1 at the end but not where there is a failing test.

@stof
Copy link
Member

@BlackbitNeueMedien This is because this URL was showing only the end of the logs, as explained in the banner on top.

Looking at the full logs, I see an issue:https://ci.appveyor.com/project/fabpot/symfony/builds/44759158?fullLog=true#L1706 shows that your new code leaks to stderr

However, I'm still not sure what changes the exit code to a failure one.

@nicolas-grekasnicolas-grekas modified the milestones:6.2,6.3Nov 5, 2022
@nicolas-grekasnicolas-grekas modified the milestones:6.3,6.4May 23, 2023
@nicolas-grekasnicolas-grekas changed the title[Process] Support finding executables independent of open_basedir config[Process] Support finding executables independently of open_basedir configAug 1, 2023
@nicolas-grekasnicolas-grekas changed the title[Process] Support finding executables independently of open_basedir config[Process] Support usingProcess::findExecutable() independently ofopen_basedirAug 1, 2023
@nicolas-grekasnicolas-grekasforce-pushed thebugfix/find-executables-outside-open_basedir branch from1e0428c to5af7d53CompareAugust 1, 2023 14:58
@fabpot
Copy link
Member

Thank you@BlackbitDevs.

@fabpotfabpot merged commite66a68c intosymfony:6.4Aug 1, 2023
@nicolas-grekasnicolas-grekas mentioned this pull requestAug 7, 2023
nicolas-grekas added a commit that referenced this pull requestAug 7, 2023
This PR was merged into the 6.4 branch.Discussion----------[Process] fix tests| Q             | A| ------------- | ---| Branch?       | 6.4| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -- `testFindProcessInOpenBasedir` is a duplicate of `testFindWithOpenBaseDir`- `testFindWithOpenBaseDir` currently expects that we search open_basedir instead of PATH when the setting is set, but this doesn't really make sense, and#47422 removed this behavior- `PhpSubprocessTest::testSubprocess` expects a php that defaults to memory_limit=-1, which is not the case currently for the sigchild-enabled binaryCommits-------4ca4417 [Process] fix tests
This was referencedOct 21, 2023
Luc45 added a commit to Luc45/symfony that referenced this pull requestMar 4, 2024
nicolas-grekas added a commit that referenced this pull requestSep 17, 2024
…sedir (BlackbitDevs)This PR was merged into the 5.4 branch.Discussion----------[Process] Fix finding executables independently of open_basedir| Q             | A| ------------- | ---| Branch?       | 5.4| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Issues        | -| License       | MITThis backports#47422 to 5.4, which is a bugfix really.Instead of#58008 and#57954 /cc `@xabbuh` `@fritzmg`Commits-------4424763 [Process] Fix finding executables independently of open_basedir
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@stofstofstof left review comments

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

6.4

Development

Successfully merging this pull request may close these issues.

ExecutableFinder attempts access outside ofopen_basedir directive, producing unnecessary errors/logs

5 participants

@BlackbitDevs@stof@fabpot@nicolas-grekas@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp