Skip to content

FPM fix and test for setting php_admin_value doc_root #7673

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
wants to merge 2 commits into from

Conversation

bukka
Copy link
Member

@bukka bukka commented Nov 20, 2021

This PR adds a test for #7660 fix and in that way also adds a coverage for the doc_root INI usage with FPM.

The test has been verified and failing without a fix.

jlbprof and others added 2 commits November 20, 2021 22:41
While testing the cPanel usage of PHP-FPM, we stumbled on this bug.
Without the fix, the zend_string is corrupted and getting odd filenames

When using FPM we kept getting "No input file specified".

I work for cPanel and we use PHP extensively.
@bukka
Copy link
Member Author

bukka commented Nov 20, 2021

@jlbprof @nikic I have been testing just against 8.1. Is this the only version impacted? I haven't checked any lower branches as I assume it would have been already reported...

Copy link
Member

@nikic nikic left a comment

Choose a reason for hiding this comment

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

LG. And yes, this is an 8.1 only issue.

* @param string|null $successMessage
* @param string|null $errorMessage
* @param bool $connKeepAlive
* @param string|null $scritpFilename
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @param string|null $scritpFilename
* @param string|null $scriptFilename

...and below.

@nikic nikic closed this in 435a5ac Nov 23, 2021
@nikic
Copy link
Member

nikic commented Nov 23, 2021

Merged this myself so it makes it into 8.1.0.

@jlbprof
Copy link
Contributor

jlbprof commented Nov 23, 2021

Thanx all for tending to this. :)

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

Successfully merging this pull request may close these issues.

3 participants