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

Set $_SERVER['SCRIPT_NAME'] within proxy command #11562

Merged
merged 1 commit into from
Aug 2, 2023

Conversation

fredden
Copy link
Contributor

@fredden fredden commented Jul 21, 2023

Fixes: #11555
Fixes: sebastianbergmann/phpunit#5447

This produces something like the following. This specific example has been taken from the test suite.

if (__FILE__ === realpath($_SERVER['SCRIPT_NAME'])) {
    $_SERVER['SCRIPT_NAME'] = realpath(__DIR__ . '/..'.'/vendor/foo/bar/binary');
}

@@ -359,6 +359,11 @@ public function url_stat(\$path, \$flags)

$globalsCode
$streamProxyCode

if (__FILE__ === realpath(\$_SERVER['SCRIPT_NAME'])) {
\$_SERVER['SCRIPT_NAME'] = $binPathExported;
Copy link
Member

Choose a reason for hiding this comment

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

We should do a realpath on $binPathExported I think otherwise you end up with a relative path which would not be the full path, but maybe that's fine too I guess SCRIPT_NAME isn't guaranteed to be canonical?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wondered about this while writing the code for this. From what I can tell, SCRIPT_NAME holds the same path that was used on the command line.

Screenshot_2023-07-23_20-53-10

The documentation for $_SERVER says that SCRIPT_NAME contains a "path" whereas __FILE__ contains a "full path". I don't know if difference in terms there is intentional or significant.

Contains the current script's path. This is useful for pages which need to point to themselves. The __FILE__ constant contains the full path and filename of the current (i.e. included) file.

I've updated the pull request to use realpath() as requested.

@Seldaek Seldaek added this to the 2.6 milestone Jul 23, 2023
@Seldaek Seldaek merged commit 9c25633 into composer:main Aug 2, 2023
19 checks passed
@Seldaek
Copy link
Member

Seldaek commented Aug 2, 2023

Thanks

@fredden fredden deleted the proxy-server-script-name branch August 3, 2023 09:31
@Wulfheart
Copy link

@Seldaek out of curiosity: I guess there are plans to make a new release where this change is also included. Any ETA for that release? Thank you in advance.

@Seldaek
Copy link
Member

Seldaek commented Aug 29, 2023

@Wulfheart yes, I'll try asap, life is a bit hectic at the moment tho, but definitely my next target is a 2.6.0 release :)

@Wulfheart
Copy link

@Seldaek thanks for your response. Glad that it is on the table and not in the far future.
Take care of yourself!

@Seldaek
Copy link
Member

Seldaek commented Sep 3, 2023

I had to revert this due to #11617 - it was a hacky attempt at fixing it and well it backfired as these things often do.

So I think the best is to just call the phar directly as per sebastianbergmann/phpunit#5447 (comment) - or someone sends a PR detecting the Composer bin proxy and also allowing that for execution of the phar. This is detectable by checking for presence of the global _composer_bin_dir variable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Phar file not executable from vendor/bin Unable to execute phar in vendor/bin
3 participants