8000 [Process] Fix handling empty path found in the PATH env var with ExecutableFinder by nicolas-grekas · Pull Request #58711 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Process] Fix handling empty path found in the PATH env var with ExecutableFinder #58711

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
Oct 31, 2024
Merged

[Process] Fix handling empty path found in the PATH env var with ExecutableFinder #58711

merged 1 commit into from
Oct 31, 2024

Conversation

nicolas-grekas
Copy link
Member
@nicolas-grekas nicolas-grekas commented Oct 29, 2024 •
edited
Loading
Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Issues -
License MIT

@carsonbot carsonbot added this to the 5.4 milestone Oct 29, 2024
@alexandre-daubois
Copy link
Member
alexandre-daubois commented Oct 30, 2024 •
edited
Loading

What about:

diff --git a/src/Symfony/Component/Process/Tests/ExecutableFinderTest.php b/src/Symfony/Component/Process/Tests/ExecutableFinderTest.php
index a1b8d6d54b..5052fc0d6b 100644
--- a/src/Symfony/Component/Process/Tests/ExecutableFinderTest.php
+++ b/src/Symfony/Component/Process/Tests/ExecutableFinderTest.php
@@ -138,6 +138,21 @@ class ExecutableFinderTest extends TestCase
         $this->assertSamePath($target.'.BAT', $result);
     }
 
+    public function testEmptyDirInPath()
+    {
+        putenv(sprintf('PATH=%s:', \dirname(\PHP_BINARY)));
+
+        touch('executable');
+        chmod('executable', 0700);
+
+        $finder = new ExecutableFinder();
+        $result = $finder->find('executable');
+
+        $this->assertSame('./executable', $result);
+
+        unlink('executable');
+    }
+
     private function assertSamePath($expected, $tested)
     {
         if ('\\' === \DIRECTORY_SEPARATOR) {

The result before your fix:

/Users/.../PhpstormProjects/symfony/./executable

After this fix:

./executable

@nicolas-grekas
Copy link
Member Author

Perfect thanks! PR updated.

@nicolas-grekas nicolas-grekas merged commit 44ce9fa into symfony:5.4 Oct 31, 2024
11 of 12 checks passed
@nicolas-grekas nicolas-grekas deleted the process-fix-empty branch October 31, 2024 07:36
@alexandre-daubois alexandre-daubois mentioned this pull request Nov 4, 2024
Merged
nicolas-grekas added a commit that referenced this pull request Nov 4, 2024
@nicolas-grekas
minor #58746 [Process] Improve test cleanup by unlinking in a `finall…
3f16033 
…y` block (alexandre-daubois)

This PR was merged into the 5.4 branch.

Discussion
----------

[Process] Improve test cleanup by unlinking in a `finally` block

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

Follwup for #58711, I realized afterwards that we may unlink in `finally` block to ensure proper test clean up in case something goes wrong.

Commits
-------

5c547c9 [Process] Improve test cleanup by unlinking in a `finally` block
This was referenced Nov 6, 2024
Merged
Merged
Merged
Merged
@TravisCarden TravisCarden mentioned this pull request Nov 18, 2024
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

@alexandre-daubois alexandre-daubois alexandre-daubois 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.
3 participants
@nicolas-grekas @alexandre-daubois @carsonbot
0