Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:74877
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain heroku.com designates 74.125.82.51 as permitted sender)
Content-Type: text/plain; charset=windows-1252
Mime-Version: 1.0 (Mac OS X Mail 6.6 \(1510\))
Date: Thu, 12 Jun 2014 21:47:55 +0200
Cc: Ilia Alshanetsky <ilia@prohost.org>,
 Julien Pauli <jpauli@php.net>
Content-Transfer-Encoding: quoted-printable
Message-ID: <035C7115-6871-4FC6-802E-464FD48070EF@heroku.com>
To: PHP internals <internals@lists.php.net>
Subject: Fixing (and making easier) Apache mod_proxy_fcgi with PHP-FPM
From: dz@heroku.com (David Zuelke)

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=3D65641.

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=85) 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:

1) ProxyPassMatch: skips all .htaccess rewrites etc
2) 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=3D47042 for =
instance), will break stuff:

if (env_path_translated !=3D NULL && env_redirect_url !=3D NULL &&
   env_path_translated !=3D script_path_translated &&
   strcmp(env_path_translated, script_path_translated) !=3D 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 =3D env_path_translated;
   /* we correct SCRIPT_NAME now in case we don't have PATH_INFO */
   env_script_name =3D 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=3D'/index.php'"
env_path_translated=3D'redirect:/index.php/a'"
env_redirect_url=3D'(null)'"
=
script_path_translated=3D'/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=3D'/index.php'"
env_path_translated=3D'redirect:/index.php/a'
env_redirect_url=3D'/a'"
=
script_path_translated=3D'/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=3D'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=85 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=3D65641 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