Skip to content

Registry settings are no longer recognized #8310

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

Closed
guyasyou opened this issue Apr 6, 2022 · 8 comments
Closed

Registry settings are no longer recognized #8310

guyasyou opened this issue Apr 6, 2022 · 8 comments

Comments

@guyasyou
Copy link

guyasyou commented Apr 6, 2022

Description

Since PHP 8.1 local settings of a workdir (from registry of windows) are loaded for scripts from another directory (Case D in STR).
Is it bug or new feature of PHP 8.1?

STR:

  • Create directory like C:/Inetpub/vhost/mydomain.tld/httpdocs
  • Create index.php with phpinfo() in the directory
  • Set local config "open_basedir=test" for the directory by registry of windows
  • Create index.php with phpinfo() in another location

Case A (PHP 8.0.14):

cd C:/Inetpub/vhost/mydomain.tld/httpdocs
php -f index.php | grep open_basedir

Actual result: open_basedir = test

Case B (PHP 8.0.14):

cd C:/Inetpub/vhost/mydomain.tld/httpdocs
php -f /another/index.php | grep open_basedir

Actual result: open_basedir = null

Case C (PHP 8.1.4):

cd C:/Inetpub/vhost/mydomain.tld/httpdocs
php -f index.php | grep open_basedir

Actual result: open_basedir = test

Case D (PHP 8.1.4):

cd C:/Inetpub/vhost/mydomain.tld/httpdocs
php -f /another/index.php | grep open_basedir

Actual result: open_basedir = test, but expected NULL (like php 8.0)

Снимок экрана 2022-04-06 в 13 36 18

PHP Version

PHP 8.1.4

Operating System

Windows Server 2019

@damianwadley
Copy link
Member

Sounds like you found an explanation and it wasn't PHP?

@guyasyou
Copy link
Author

guyasyou commented Apr 6, 2022

Sounds like you found an explanation and it wasn't PHP?

I had doubts so I decided to double check
I added the screenshot of real test

@damianwadley
Copy link
Member

What if you run outside of Cygwin? What if you don't run any PHP file but output settings using the -i flag? And

local settings of a workdir (from registry of windows)

exactly what do you mean by that?

@guyasyou
Copy link
Author

guyasyou commented Apr 6, 2022

What if you run outside of Cygwin?

I repeated all cases via CMD of Windows - same result.

What if you don't run any PHP file but output settings using the -i flag?

open_basedir = no-value

exactly what do you mean by that?

image (2)

@damianwadley
Copy link
Member

damianwadley commented Apr 6, 2022

I don't see anything in the changelog that says something was fixed in PHP 8.1.

@guyasyou
Copy link
Author

guyasyou commented Apr 6, 2022

I don't see anything in the changelog that says something was fixed in PHP 8.1.

Me too, but behavior of PHP is changed on 8.1, I think

@cmb69 cmb69 self-assigned this Apr 6, 2022
@cmb69
Copy link
Member

cmb69 commented Apr 6, 2022

This is a pretty serious bug, introduced when the Zend Stream API has been changed to use zend_string* instead of char*, but it apparently has been overlooked to fix the calls to UpdateIniFromRegistry() which still expects a char*. Since there was a cast, the compiler didn't warn about that misuse.

@cmb69 cmb69 changed the title Local directory settings are loaded for scripts from another directory on PHP 8.1.4 (Windows) Registry settings are no longer recognized Apr 6, 2022
cmb69 added a commit to cmb69/php-src that referenced this issue Apr 6, 2022
`zend_file_handle->filename` is a `zend_string*` pointer now, so we
must not cast to `char*` but rather pass the underlying `char*`.
@cmb69 cmb69 linked a pull request Apr 6, 2022 that will close this issue
@guyasyou
Copy link
Author

guyasyou commented Apr 6, 2022

Which versions are affected? 8.1.0-8.1.4?

@cmb69 cmb69 closed this as completed in 1bd9890 Apr 6, 2022
cmb69 added a commit that referenced this issue Apr 6, 2022
* PHP-8.1:
  Fix GH-8310: Registry settings are no longer recognized
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@guyasyou @cmb69 @damianwadley and others