Hi everyone (already discussed this a bit off-list with Julien),
Ilia and I sat down last week at International PHP Conference to discuss https://bugs.php.net/bug.php?id=65641.
This is a pretty critical issue with Apache 2.4, since it leads to an incorrect SCRIPT_NAME (and thus also PHP_SELF) value if PATH_INFO is given, and frameworks such as CodeIgniter (I know…) break under these circumstances. Ilia has remarked that the emalloc() that never gets free()d might not be ideal with debug on, but it's hard to do in this case, so not sure how critical the patch is. It may need some comments though explaining that if SCRIPT_NAME contains PATH_INFO, the latter is stripped from it again.
More importantly:
We also looked at a related thing together: support for a new way of proxying requests to PHP-FPM in Apache trunk (and the feature will now be backported to Apache 2.4.x, so it'll be in the next release).
This is very big news for PHP-FPM, because the current two options for using mod_proxy_fcgi (which is the future-proof way of connecting Apache and FPM) are both far from ideal:
- ProxyPassMatch: skips all .htaccess rewrites etc
- Default mod_rewrite rules on VirtualHost level: will always have to take precedence over certain user-land rewrites, so there's always conflicts, redirect loops and so forth (I've tried, it's messy).
The new approach, which can also be built as a standalone module for older versions of Apache (https://gist.github.com/progandy/6ed4eeea60f6277c3e39), works like this:
<FilesMatch .php$>
SetHandler proxy:fcgi://localhost:9000
</FilesMatch>
It's very simple, and very similar to how mod_php is used in Apache, so that'll make it very easy and convenient for users to use FPM with Apache 2.4.10+
It works very well, except in one situation: when rewriting to PATH INFO like this (common case with many frameworks):
RewriteCond %{REQUEST_FILENAME} !-f
RewriteRule ^(.+)$ index.php/$1 [L]
In this situation this really old code, apparently copied from the CGI handling into FPM (see https://bugs.php.net/bug.php?id=47042 for instance), will break stuff:
if (env_path_translated != NULL
&& env_redirect_url != NULL
&&
env_path_translated != script_path_translated &&
strcmp(env_path_translated, script_path_translated) != 0) {
/*
* pretty much apache specific. If we have a redirect_url
* then our script_filename and script_name point to the
* php executable
/
script_path_translated = env_path_translated;
/ we correct SCRIPT_NAME now in case we don't have PATH_INFO */
env_script_name = env_redirect_url;
}
The PHP executable obviously never will be in SCRIPT_FILENAME because we're dealing with FCGI here, so the entire bit is redundant.
It doesn't trigger with /index.php/a:
env_script_name='/index.php'"
env_path_translated='redirect:/index.php/a'"
env_redirect_url='(null)'"
script_path_translated='/Users/dzuelke/Code/heroku/php-test/hello_heroku_php2/index.php'"
But breaks stuff when using the above rewrite and requesting /a, because in this situation:
env_script_name='/index.php'"
env_path_translated='redirect:/index.php/a'
env_redirect_url='/a'"
script_path_translated='/Users/dzuelke/Code/heroku/php-test/hello_heroku_php2/index.php'
Here, the if will trigger, and env_script_name will be changed from the correct '/index.php' to the incorrect '/a', so FPM will report file not found.
With cgi.fix_pathinfo 0, it doesn't work either; script_filename='redirect:/index.php/a' at the end.
Just removing the segment above will lead to a missing PATH_INFO though. There is a related segment further down in the /* make sure path_info/translated are empty */ bit (which should be reworded "remember original values in the request"). Remember, it clears PATH_INFO because with Apache and pure CGI, SCRIPT_FILENAME is the path to the PHP executable, and PATH_INFO has the .php file that was requested:
if (env_redirect_url) {
if (orig_path_info) {
_sapi_cgibin_putenv("ORIG_PATH_INFO", orig_path_info TSRMLS_CC);
_sapi_cgibin_putenv("PATH_INFO", NULL
TSRMLS_CC);
}
if (orig_path_translated) {
_sapi_cgibin_putenv("ORIG_PATH_TRANSLATED", orig_path_translated TSRMLS_CC);
_sapi_cgibin_putenv("PATH_TRANSLATED", NULL
TSRMLS_CC);
}
}
Once that bit is removed as well, everything works as expected.
To reproduce, use any Apache 2.4+ (Ubuntu 14.04 has 2.4.7) and build and configure the module from the gist.
Could we still squeeze this fix into 5.2.14? I know you just rolled RC1 (while I was fiddling with this stuff), but it would be so, so huge to have this in… or at least in 5.2.15.
I should mention that when using the new SetHandler approach, the https://bugs.php.net/bug.php?id=65641 ticket isn't strictly necessary as the error condition covered there doesn't occur anymore, but for people using the (very inferior) ProxyPassMatch or mod_rewrite solutions, it'd still solve a lot of problems.
Pull request here: https://github.com/php/php-src/pull/694
Thanks,
David
Could we still squeeze this fix into 5.2.14? I know you just rolled RC1
(while I was fiddling with this stuff), but it would be so, so huge to have
this in… or at least in 5.2.15.
just to prevent confusion, you meant 5.5.14 and 5.5.15 here.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Could we still squeeze this fix into 5.2.14? I know you just rolled RC1
(while I was fiddling with this stuff), but it would be so, so huge to have
this in… or at least in 5.2.15.just to prevent confusion, you meant 5.5.14 and 5.5.15 here.
Obviously 5.5.X yes.
I myself answered that I dont see any criticity to roll out an RC2 for
5.5.14, just for this specific bug.
I think it can go to 5.5.15 or any version as far as the patch is
tested and merged up.
Julien.Pauli
Could we still squeeze this fix into 5.2.14? I know you just rolled RC1
(while I was fiddling with this stuff), but it would be so, so huge to have
this in… or at least in 5.2.15.just to prevent confusion, you meant 5.5.14 and 5.5.15 here.
Obviously 5.5.X yes.
I myself answered that I dont see any criticity to roll out an RC2 for
5.5.14, just for this specific bug.
I think it can go to 5.5.15 or any version as far as the patch is
tested and merged up.
Given the security issues fixes waiting to be released, I would not
delay the releases any longer.
Cheers,
Pierre
@pierrejoye | http://www.libgd.org
Hi!
I'm a bit confused here - which patch is proposed for merge here, the
one in bug 65641 or the one in pull 694 or both? Because they seem to be
different and the original mail mentions both. In any case, neither of
them seems critical enough that it couldn't wait for the next scheduled
version. We could merge into the dev repo anytime that we agree it's
working and people that need it urgently can use the snapshots.
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
Hi!
I'm a bit confused here - which patch is proposed for merge here, the
one in bug 65641 or the one in pull 694 or both? Because they seem to be
different and the original mail mentions both. In any case, neither of
them seems critical enough that it couldn't wait for the next scheduled
version. We could merge into the dev repo anytime that we agree it's
working and people that need it urgently can use the snapshots.
Agreed.
Ideally, both make it in, but the pull request is a bit more important IMO.
David
Could we still squeeze this fix into 5.2.14? I know you just rolled RC1
(while I was fiddling with this stuff), but it would be so, so huge to have
this in… or at least in 5.2.15.just to prevent confusion, you meant 5.5.14 and 5.5.15 here.
Obviously 5.5.X yes.
I myself answered that I dont see any criticity to roll out an RC2 for
5.5.14, just for this specific bug.
I think it can go to 5.5.15 or any version as far as the patch is
tested and merged up.Given the security issues fixes waiting to be released, I would not
delay the releases any longer.
So, who wants to merge the PR? :)
And if someone could review and possibly apply any of the https://bugs.php.net/bug.php?id=65641 attachments that'd be great too, although I suspect that it's possible to fix the problem way further up in the code so that this stripping of stuff isn't even necessary at such a "late" stage.
With those two out of the way, PHP-FPM and Apache will be a lot, lot easier to set up together. One bit of config, done.
David
P.S. would be great for 5.4 too I think, not just 5.5+!
Bump?
We've been running http://github.com/php/php-src/pull/694 in production at Heroku for a while now, with no issues.
I really think this should be in 5.6, too. It'll make setting up PHP-FPM so much less painful on Apache once their new release is out.
And if someone could also please review and commit https://bugs.php.net/bug.php?id=65641 (or find a better way; I'll be glad to help with anything like setting up Apache accordingly etc), then even with current versions of Apache, PHP will finally work fine. Currently, quite a few frameworks etc that use (or rewrite to) PATH_INFO are plainly broken.
Thanks,
David
Could we still squeeze this fix into 5.2.14? I know you just rolled RC1
(while I was fiddling with this stuff), but it would be so, so huge to have
this in… or at least in 5.2.15.just to prevent confusion, you meant 5.5.14 and 5.5.15 here.
Obviously 5.5.X yes.
I myself answered that I dont see any criticity to roll out an RC2 for
5.5.14, just for this specific bug.
I think it can go to 5.5.15 or any version as far as the patch is
tested and merged up.Given the security issues fixes waiting to be released, I would not
delay the releases any longer.So, who wants to merge the PR? :)
And if someone could review and possibly apply any of the https://bugs.php.net/bug.php?id=65641 attachments that'd be great too, although I suspect that it's possible to fix the problem way further up in the code so that this stripping of stuff isn't even necessary at such a "late" stage.
With those two out of the way, PHP-FPM and Apache will be a lot, lot easier to set up together. One bit of config, done.
David
P.S. would be great for 5.4 too I think, not just 5.5+!
The change is now in Apache 2.4.x (http://svn.apache.org/viewvc?view=revision&revision=1604378) and they're planning a release soon: http://thread.gmane.org/gmane.comp.apache.devel/53822
Can someone please at least merge the pull request? Would be a shame IMO if this didn't make it into 5.6.0.
I'll gladly provide docs for this, too.
David
Bump?
We've been running http://github.com/php/php-src/pull/694 in production at Heroku for a while now, with no issues.
I really think this should be in 5.6, too. It'll make setting up PHP-FPM so much less painful on Apache once their new release is out.
And if someone could also please review and commit https://bugs.php.net/bug.php?id=65641 (or find a better way; I'll be glad to help with anything like setting up Apache accordingly etc), then even with current versions of Apache, PHP will finally work fine. Currently, quite a few frameworks etc that use (or rewrite to) PATH_INFO are plainly broken.
Thanks,
David
Could we still squeeze this fix into 5.2.14? I know you just rolled RC1
(while I was fiddling with this stuff), but it would be so, so huge to have
this in… or at least in 5.2.15.just to prevent confusion, you meant 5.5.14 and 5.5.15 here.
Obviously 5.5.X yes.
I myself answered that I dont see any criticity to roll out an RC2 for
5.5.14, just for this specific bug.
I think it can go to 5.5.15 or any version as far as the patch is
tested and merged up.Given the security issues fixes waiting to be released, I would not
delay the releases any longer.So, who wants to merge the PR? :)
And if someone could review and possibly apply any of the https://bugs.php.net/bug.php?id=65641 attachments that'd be great too, although I suspect that it's possible to fix the problem way further up in the code so that this stripping of stuff isn't even necessary at such a "late" stage.
With those two out of the way, PHP-FPM and Apache will be a lot, lot easier to set up together. One bit of config, done.
David
P.S. would be great for 5.4 too I think, not just 5.5+!
Julien, do you have any problem merging this PR to PHP-5.5(and merging
upwards) so it can be released with 5.5.15 (and with 5.6.0RC2)?
The change is now in Apache 2.4.x (
http://svn.apache.org/viewvc?view=revision&revision=1604378) and they're
planning a release soon:
http://thread.gmane.org/gmane.comp.apache.devel/53822Can someone please at least merge the pull request? Would be a shame IMO
if this didn't make it into 5.6.0.I'll gladly provide docs for this, too.
David
Bump?
We've been running http://github.com/php/php-src/pull/694 in production
at Heroku for a while now, with no issues.I really think this should be in 5.6, too. It'll make setting up PHP-FPM
so much less painful on Apache once their new release is out.And if someone could also please review and commit
https://bugs.php.net/bug.php?id=65641 (or find a better way; I'll be glad
to help with anything like setting up Apache accordingly etc), then even
with current versions of Apache, PHP will finally work fine. Currently,
quite a few frameworks etc that use (or rewrite to) PATH_INFO are plainly
broken.Thanks,
David
On Thu, Jun 12, 2014 at 10:11 PM, Ferenc Kovacs tyra3l@gmail.com
wrote:Could we still squeeze this fix into 5.2.14? I know you just rolled
RC1
(while I was fiddling with this stuff), but it would be so, so huge
to have
this in… or at least in 5.2.15.just to prevent confusion, you meant 5.5.14 and 5.5.15 here.
Obviously 5.5.X yes.
I myself answered that I dont see any criticity to roll out an RC2 for
5.5.14, just for this specific bug.
I think it can go to 5.5.15 or any version as far as the patch is
tested and merged up.Given the security issues fixes waiting to be released, I would not
delay the releases any longer.So, who wants to merge the PR? :)
And if someone could review and possibly apply any of the
https://bugs.php.net/bug.php?id=65641 attachments that'd be great too,
although I suspect that it's possible to fix the problem way further up in
the code so that this stripping of stuff isn't even necessary at such a
"late" stage.With those two out of the way, PHP-FPM and Apache will be a lot, lot
easier to set up together. One bit of config, done.David
P.S. would be great for 5.4 too I think, not just 5.5+!
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Julien, do you have any problem merging this PR to PHP-5.5(and merging
upwards) so it can be released with 5.5.15 (and with 5.6.0RC2)?
Don't we take 5.4 as basis ?
Julien
The change is now in Apache 2.4.x
(http://svn.apache.org/viewvc?view=revision&revision=1604378) and they're
planning a release soon:
http://thread.gmane.org/gmane.comp.apache.devel/53822Can someone please at least merge the pull request? Would be a shame IMO
if this didn't make it into 5.6.0.I'll gladly provide docs for this, too.
David
Bump?
We've been running http://github.com/php/php-src/pull/694 in production
at Heroku for a while now, with no issues.I really think this should be in 5.6, too. It'll make setting up PHP-FPM
so much less painful on Apache once their new release is out.And if someone could also please review and commit
https://bugs.php.net/bug.php?id=65641 (or find a better way; I'll be glad to
help with anything like setting up Apache accordingly etc), then even with
current versions of Apache, PHP will finally work fine. Currently, quite a
few frameworks etc that use (or rewrite to) PATH_INFO are plainly broken.Thanks,
David
On Thu, Jun 12, 2014 at 10:11 PM, Ferenc Kovacs tyra3l@gmail.com
wrote:Could we still squeeze this fix into 5.2.14? I know you just rolled
RC1
(while I was fiddling with this stuff), but it would be so, so huge
to have
this in… or at least in 5.2.15.just to prevent confusion, you meant 5.5.14 and 5.5.15 here.
Obviously 5.5.X yes.
I myself answered that I dont see any criticity to roll out an RC2
for
5.5.14, just for this specific bug.
I think it can go to 5.5.15 or any version as far as the patch is
tested and merged up.Given the security issues fixes waiting to be released, I would not
delay the releases any longer.So, who wants to merge the PR? :)
And if someone could review and possibly apply any of the
https://bugs.php.net/bug.php?id=65641 attachments that'd be great too,
although I suspect that it's possible to fix the problem way further up in
the code so that this stripping of stuff isn't even necessary at such a
"late" stage.With those two out of the way, PHP-FPM and Apache will be a lot, lot
easier to set up together. One bit of config, done.David
P.S. would be great for 5.4 too I think, not just 5.5+!
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
2014.06.25. 11:13, "Julien Pauli" jpauli@php.net ezt írta:
Julien, do you have any problem merging this PR to PHP-5.5(and merging
upwards) so it can be released with 5.5.15 (and with 5.6.0RC2)?Don't we take 5.4 as basis ?
Julien
ops, sorry I have missinterpreted your previous mail.
having it in 5.4 would of course better.
Stas are you ok with merging this to PHP-5.4 now?
Hi!
ops, sorry I have missinterpreted your previous mail.
having it in 5.4 would of course better.Stas are you ok with merging this to PHP-5.4 now?
I'm ok with it. Did you ping Jerome Loyet or Antony? They are listed as
maintainers, haven't heard from them on that.
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
Hi!
ops, sorry I have missinterpreted your previous mail.
having it in 5.4 would of course better.Stas are you ok with merging this to PHP-5.4 now?
I'm ok with it. Did you ping Jerome Loyet or Antony? They are listed as
maintainers, haven't heard from them on that.
As Pierre said : are we absolutely sure about this patch ?
History shows that we once failed at patching FPM and introduced problems.
Just to say.
Julien.Pauli
On Thu, Jun 26, 2014 at 7:49 PM, Stas Malyshev smalyshev@sugarcrm.com
wrote:Hi!
ops, sorry I have missinterpreted your previous mail.
having it in 5.4 would of course better.Stas are you ok with merging this to PHP-5.4 now?
I'm ok with it. Did you ping Jerome Loyet or Antony? They are listed as
maintainers, haven't heard from them on that.As Pierre said : are we absolutely sure about this patch ?
History shows that we once failed at patching FPM and introduced problems.
More exactly:
Tested only in heroku environment. I would better get it in 5.6.0, see how
things will go and then merge to other branches.
Hi!
ops, sorry I have missinterpreted your previous mail.
having it in 5.4 would of course better.Stas are you ok with merging this to PHP-5.4 now?
I'm ok with it. Did you ping Jerome Loyet or Antony? They are listed as
maintainers, haven't heard from them on that.As Pierre said : are we absolutely sure about this patch ?
History shows that we once failed at patching FPM and introduced problems.
That's what RCs are for, right? :)
It's a relatively straightforward change, and you can see from a look at http://github.com/php/php-src/pull/694 and https://github.com/dzuelke/php-src/blob/apache_fcgi_sethandler/sapi/cgi/cgi_main.c#L1205 that the code that the change removes was simply a verbatim copy from the old CGI code that didn't get cleaned up during the "translation" to FPM.
In a FastCGI environment, SCRIPT_FILENAME and SCRIPT_NAME cannot contain the name of the PHP executable, as that's not known or invoked by the web server, so the code is completely redundant.
CCing Antony so he can confirm/approve.
David
P.S. Just to emphasize this again: we're running this in production for all PHP customers now with zero issues, running a standard FPM config and the usual Apache config; tested with ProxyPassMatch, mod_rewrite and the new (Apache 2.4.10) SetHandler way of connecting the two. The first two suffer from https://bugs.php.net/bug.php?id=65641, so that should be addressed too, eventually.
That's what RCs are for, right? :)
In an ideal world with ideal users yes.
But we are not in that world :)
That's what RCs are for, right? :)
In an ideal world with ideal users yes.
But we are not in that world :)
I know :)
For e.g. https://bugs.php.net/bug.php?id=65641 I realize it's difficult to judge the potential impact; I've hit many different variations of (F)CGI env vars, and it's one big mess where you pull on one end and it collapses on the other.
The PR I'm suggesting however is not in the complicated fix_pathinfo/apache_was_here nightmare section of the code and thus straightforward enough to evaluate.
Plus, it very obviously doesn't belong there in the first place. It fixes an issue that can't even occur with FastCGI, and as a result (because the if() is very "generic" and "aggressive" in how it tries to "detect" a certain situation), it breaks other config combinations, such as the new SetHandler based Apache FastCGI proxying.
David
That's what RCs are for, right? :)
In an ideal world with ideal users yes.
But we are not in that world :)
I know :)
So what is wrong in putting us on the safe? Waiting 2 months max is not
that bad :)
That's what RCs are for, right? :)
In an ideal world with ideal users yes.
But we are not in that world :)
I know :)
So what is wrong in putting us on the safe? Waiting 2 months max is not that bad :)
Sure, but then it would be good to have this in the next 5.6 RC, that'll probably get more testing exposure, too.
Either way, someone should please commit it into some branch soon ;)
David
That's what RCs are for, right? :)
In an ideal world with ideal users yes.
But we are not in that world :)
I know :)
So what is wrong in putting us on the safe? Waiting 2 months max is not
that bad :)Sure, but then it would be good to have this in the next 5.6 RC, that'll
probably get more testing exposure, too.Either way, someone should please commit it into some branch soon ;)
David
Was waiting for Stas to reply, I will merge it into 5.6 this weekend if he
doesn't reply that he wants to merge it into 5.4, then we can test it with
the 5.6RCs and merge back to 5.4/5.5.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
So what is wrong in putting us on the safe? Waiting 2 months max is not that bad :)
Sure, but then it would be good to have this in the next 5.6 RC, that'll probably get more testing exposure, too.
Either way, someone should please commit it into some branch soon ;)
Was waiting for Stas to reply, I will merge it into 5.6 this weekend if he doesn't reply that he wants to merge it into 5.4, then we can test it with the 5.6RCs and merge back to 5.4/5.5.
Thanks!
Stas, any thoughts?
Hi,
Thanks for the merge, but the just-merged PR addresses a different issue than bug 65641. That one is still unaddressed.
The NEWS entry for the just-merged PR should be something like "Support Apache 2.4.10+ SetHandler method for mod_proxy_fcgi and PHP-FPM", and bug 65641 should be re-opened.
I'll see if I can look at the patches for bug 65641 again the next few days; my gut feeling is that there should be a much easier way of fixing this. After all, the patches all strip out parts of a string that shouldn't be there in the first place.
David
That's what RCs are for, right? :)
In an ideal world with ideal users yes.
But we are not in that world :)
I know :)
So what is wrong in putting us on the safe? Waiting 2 months max is not that bad :)
Sure, but then it would be good to have this in the next 5.6 RC, that'll probably get more testing exposure, too.
Either way, someone should please commit it into some branch soon ;)
David
Was waiting for Stas to reply, I will merge it into 5.6 this weekend if he doesn't reply that he wants to merge it into 5.4, then we can test it with the 5.6RCs and merge back to 5.4/5.5.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
(to clarify: when using 2.4.10+ with SetHandler and the now merged PR, the bug doesn't occur, but if people are using mod_rewrite, or ProxyPass(Match), either because they don't know about the new method or are stuck on older versions of Apache, bug 65641 is still relevant)
Hi,
Thanks for the merge, but the just-merged PR addresses a different issue than bug 65641. That one is still unaddressed.
The NEWS entry for the just-merged PR should be something like "Support Apache 2.4.10+ SetHandler method for mod_proxy_fcgi and PHP-FPM", and bug 65641 should be re-opened.
I'll see if I can look at the patches for bug 65641 again the next few days; my gut feeling is that there should be a much easier way of fixing this. After all, the patches all strip out parts of a string that shouldn't be there in the first place.
David
That's what RCs are for, right? :)
In an ideal world with ideal users yes.
But we are not in that world :)
I know :)
So what is wrong in putting us on the safe? Waiting 2 months max is not that bad :)
Sure, but then it would be good to have this in the next 5.6 RC, that'll probably get more testing exposure, too.
Either way, someone should please commit it into some branch soon ;)
David
Was waiting for Stas to reply, I will merge it into 5.6 this weekend if he doesn't reply that he wants to merge it into 5.4, then we can test it with the 5.6RCs and merge back to 5.4/5.5.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Hi!
Was waiting for Stas to reply, I will merge it into 5.6 this weekend if
he doesn't reply that he wants to merge it into 5.4, then we can test it
with the 5.6RCs and merge back to 5.4/5.5.
Maybe we can have people try it out in 5.6, if it works fine then we can
backport it into 5.4 if people need it. I don't have anything against
having it in 5.4, just not sure how much risk is in it since I don't
know that part of code that well.
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
2014.06.30. 9:02, "Stas Malyshev" smalyshev@sugarcrm.com ezt írta:
Hi!
Was waiting for Stas to reply, I will merge it into 5.6 this weekend if
he doesn't reply that he wants to merge it into 5.4, then we can test it
with the 5.6RCs and merge back to 5.4/5.5.Maybe we can have people try it out in 5.6, if it works fine then we can
backport it into 5.4 if people need it. I don't have anything against
having it in 5.4, just not sure how much risk is in it since I don't
know that part of code that well.
this is what Pierre also suggested and what I ended up doing.
Hi everyone (already discussed this a bit off-list with Julien),
Ilia and I sat down last week at International PHP Conference to discuss https://bugs.php.net/bug.php?id=65641.
This is a pretty critical issue with Apache 2.4, since it leads to an incorrect SCRIPT_NAME (and thus also PHP_SELF) value if PATH_INFO is given, and frameworks such as CodeIgniter (I know…) break under these circumstances. Ilia has remarked that the emalloc() that never gets free()d might not be ideal with debug on, but it's hard to do in this case, so not sure how critical the patch is. It may need some comments though explaining that if SCRIPT_NAME contains PATH_INFO, the latter is stripped from it again.
More importantly:
We also looked at a related thing together: support for a new way of proxying requests to PHP-FPM in Apache trunk (and the feature will now be backported to Apache 2.4.x, so it'll be in the next release).
This is very big news for PHP-FPM, because the current two options for using mod_proxy_fcgi (which is the future-proof way of connecting Apache and FPM) are both far from ideal:
- ProxyPassMatch: skips all .htaccess rewrites etc
- Default mod_rewrite rules on VirtualHost level: will always have to take precedence over certain user-land rewrites, so there's always conflicts, redirect loops and so forth (I've tried, it's messy).
The new approach, which can also be built as a standalone module for older versions of Apache (https://gist.github.com/progandy/6ed4eeea60f6277c3e39), works like this:
<FilesMatch .php$>
SetHandler proxy:fcgi://localhost:9000
</FilesMatch>It's very simple, and very similar to how mod_php is used in Apache, so that'll make it very easy and convenient for users to use FPM with Apache 2.4.10+
This sounds great! And I see in the comments on github that it works with unix sockets too! I just wasted several hours trying to get my user-land rewrites working... I had to swap to using a RewriteRule instead of ProxyPassMatch, but I could not get it working with a unix socket. In the end I had to reconfigure FPM to listen to a tcp socket instead. Very annoying, but I’m very happy to see a much simpler and cleaner solution just around the corner! Hopefully I’ll actually be able to start using 5.5 in production soon.
Cheers,
David
The new approach, which can also be built as a standalone module for older versions of Apache (https://gist.github.com/progandy/6ed4eeea60f6277c3e39), works like this:
<FilesMatch .php$>
SetHandler proxy:fcgi://localhost:9000
</FilesMatch>It's very simple, and very similar to how mod_php is used in Apache, so that'll make it very easy and convenient for users to use FPM with Apache 2.4.10+
This sounds great! And I see in the comments on github that it works with unix sockets too! I just wasted several hours trying to get my user-land rewrites working... I had to swap to using a RewriteRule instead of ProxyPassMatch, but I could not get it working with a unix socket. In the end I had to reconfigure FPM to listen to a tcp socket instead. Very annoying, but I’m very happy to see a much simpler and cleaner solution just around the corner! Hopefully I’ll actually be able to start using 5.5 in production soon.
You can use UDS too; you just have to trick Apache into creating proxy instances first:
<Proxy "unix:/tmp/php-fpm.sock|fcgi://php-fpm">
# we must declare a parameter in here (doesn't matter which) or it'll not register the proxy ahead of time
ProxySet disablereuse=off
</Proxy>
<FilesMatch .php$>
SetHandler proxy:fcgi://php-fpm
</FilesMatch>
This basically creates an alias to the UDS under whatever name you give it after "fcgi://", and you can reference that in SetHandler, rewrites or ProxyPass(Match). Apache will pass "proxy:fcgi://php-fpm:8000" (no idea why it picks that port) as the prefix to the backend in this case.
The better solution of course would be if FPM supported the unix domain socket syntax in SCRIPT_FILENAME et al (which will then look like proxy:unix:/tmp/php-fpm.sock|fcgi://php-fpm, so it'll need to "regex-strip" ^proxy:unix:[^|]+|fcgi:// instead of the static proxy:fcgi:// prefix that it strips now.
Anyone want to take a stab at fixing that? Line ~1100 in fpm_main.c :)
David