Re: Summary: Broken FastCGI with httpd

[Jacob Champion]

On 02/05/2017 05:10 AM, Jim Jagielski wrote:

Not seeing anything fpm_main.c...


Line 1144 in my copy (attached). It's an example of an Apache-specific 
fixup that only runs if apache_was_here is *false*.


--Jacob
if (!apache_was_here && 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   
 
 * we don't want to do this for the new mod_proxy_fcgi approach,
 
 * where redirect_url may also exist but the below will break   
 
 * with rewrites to PATH_INFO, hence the !apache_was_here check 
 
 */ 
 
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; 
 
} 

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

> On Jan 30, 2017, at 4:21 PM, Jacob Champion  wrote:
> 
> On 01/30/2017 01:17 PM, Jim Jagielski wrote:
>> Looking over fpm_main, Apache is detected iff PHP sees the proxy:balancer 
>> and/or
>> proxy:fcgi prefix. Looking at the logic paths related to 'apache_was_here'
> 
> No, apache_was_here is only *new* Apache. There are other fixups that trigger 
> on other variables; search for "apache specific" for another example.
> 

Not seeing anything fpm_main.c...

Re: Summary: Broken FastCGI with httpd

[Jacob Champion]

On 01/30/2017 01:21 PM, Jim Jagielski wrote:

Plus, other than the "additional" way of detecting apache_was_here, that
file has been very stable with no real changes:

https://github.com/php/php-src/commits/master/sapi/fpm/fpm/fpm_main.c

since


https://github.com/php/php-src/commit/e6d93a11ad343efdc42315f7f69ed82515c9f374#diff-624bdd47ab6847d777e15327976a9227


Right. This has been broken for a *long* time; it's why the PHP-FPM 
maintainer went off on a rant in Bugzilla.


--Jacob

Re: Summary: Broken FastCGI with httpd

[Jacob Champion]

On 01/30/2017 01:17 PM, Jim Jagielski wrote:

Looking over fpm_main, Apache is detected iff PHP sees the proxy:balancer and/or
proxy:fcgi prefix. Looking at the logic paths related to 'apache_was_here'


No, apache_was_here is only *new* Apache. There are other fixups that 
trigger on other variables; search for "apache specific" for another 
example.


--Jacob

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

> On Jan 30, 2017, at 4:17 PM, Jim Jagielski  wrote:
> 
>> 
>> On Jan 30, 2017, at 4:04 PM, Jacob Champion  wrote:
>> 
>> On 01/28/2017 07:22 AM, Jim Jagielski wrote:
>>> In your scenario does "old mode" == "old" Apache or non Apache?
>> 
>> "Old" Apache.
>> 
>>> My idea was to send FCGI data such that PHP-FPM doesn't use *any*
>>> Apache-related fixups. In other words, httpd sends "what it should"
>>> and PHP-FPM "uses what it receives"
>> 
>> Right. My point is that without simultaneous modifications to PHP-FPM, I see 
>> no way to do this without breaking deployed use cases, because of how easily 
>> the fixup code is triggered. And therefore IMO we shouldn't push that change 
>> in 2.4.26, but wait (for a reasonable amount of time) for both sides to 
>> agree on the way forward.
>> 
>> I'll try to review Eric's patch later this week, since it would help ease 
>> the migration pain (and give other users a path forward even with broken 
>> FCGI backends).
>> 
> 
> Looking over fpm_main, Apache is detected iff PHP sees the proxy:balancer 
> and/or
> proxy:fcgi prefix. Looking at the logic paths related to 'apache_was_here',
> it looks like it just works around stuff that we *CAN* fix. For example, one
> code path is due to "mod_proxy_fcgi and ProxyPass, apache cannot set 
> PATH_INFO",
> but we can, and do now

Plus, other than the "additional" way of detecting apache_was_here, that
file has been very stable with no real changes:

https://github.com/php/php-src/commits/master/sapi/fpm/fpm/fpm_main.c

since


https://github.com/php/php-src/commit/e6d93a11ad343efdc42315f7f69ed82515c9f374#diff-624bdd47ab6847d777e15327976a9227

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

> On Jan 30, 2017, at 4:04 PM, Jacob Champion  wrote:
> 
> On 01/28/2017 07:22 AM, Jim Jagielski wrote:
>> In your scenario does "old mode" == "old" Apache or non Apache?
> 
> "Old" Apache.
> 
>> My idea was to send FCGI data such that PHP-FPM doesn't use *any*
>> Apache-related fixups. In other words, httpd sends "what it should"
>> and PHP-FPM "uses what it receives"
> 
> Right. My point is that without simultaneous modifications to PHP-FPM, I see 
> no way to do this without breaking deployed use cases, because of how easily 
> the fixup code is triggered. And therefore IMO we shouldn't push that change 
> in 2.4.26, but wait (for a reasonable amount of time) for both sides to agree 
> on the way forward.
> 
> I'll try to review Eric's patch later this week, since it would help ease the 
> migration pain (and give other users a path forward even with broken FCGI 
> backends).
> 

Looking over fpm_main, Apache is detected iff PHP sees the proxy:balancer and/or
proxy:fcgi prefix. Looking at the logic paths related to 'apache_was_here',
it looks like it just works around stuff that we *CAN* fix. For example, one
code path is due to "mod_proxy_fcgi and ProxyPass, apache cannot set PATH_INFO",
but we can, and do now

Re: Summary: Broken FastCGI with httpd

[Jacob Champion]

On 01/28/2017 07:22 AM, Jim Jagielski wrote:

In your scenario does "old mode" == "old" Apache or non Apache?


"Old" Apache.


My idea was to send FCGI data such that PHP-FPM doesn't use *any*
Apache-related fixups. In other words, httpd sends "what it should"
and PHP-FPM "uses what it receives"


Right. My point is that without simultaneous modifications to PHP-FPM, I 
see no way to do this without breaking deployed use cases, because of 
how easily the fixup code is triggered. And therefore IMO we shouldn't 
push that change in 2.4.26, but wait (for a reasonable amount of time) 
for both sides to agree on the way forward.


I'll try to review Eric's patch later this week, since it would help 
ease the migration pain (and give other users a path forward even with 
broken FCGI backends).


--Jacob

Re: Summary: Broken FastCGI with httpd

[Eric Covener]

On Sat, Jan 28, 2017 at 10:22 AM, Jim Jagielski  wrote:
>
>> On Jan 27, 2017, at 1:36 PM, Jacob Champion  wrote:
>>
>> On 01/27/2017 04:56 AM, Jim Jagielski wrote:
>>> Let me relook over fpm-main... from what I saw, there are only 2
>>> logic paths: one if Apache and the other for everybody-else...
>>> and the Apache path only kicks in if it sees the proxy:balancer
>>> or proxy:fcgi prefix.
>>
>> Unfortunately there are at least three: non-Apache, "old" Apache, and "new" 
>> Apache (mod_proxy_fcgi). And the logic of those three paths overlaps -- 
>> they're not completely separate.
>>
>> That's what makes this hard. If we remove the proxy: prefix now, PHP-FPM 
>> "old mode" fixups will kick in (in certain situations involving REDIRECT_URL 
>> etc.) even if there is nothing to fix up. That's what prompted this 
>> conversation.
>>
>
> In your scenario does "old mode" == "old" Apache or non Apache?
>
> My idea was to send FCGI data such that PHP-FPM doesn't use *any*
> Apache-related fixups. In other words, httpd sends "what it should"
> and PHP-FPM "uses what it receives"
>

Worked on this as an Insurance policy against whatever we end up doing
-- so tweaks can occur after ap_add_cgi_vars:

http://people.apache.org/~covener/patches/httpd-trunk-fcgiproxysetenvif.diff

-- 
Eric Covener
cove...@gmail.com

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

> On Jan 27, 2017, at 1:36 PM, Jacob Champion  wrote:
> 
> On 01/27/2017 04:56 AM, Jim Jagielski wrote:
>> Let me relook over fpm-main... from what I saw, there are only 2
>> logic paths: one if Apache and the other for everybody-else...
>> and the Apache path only kicks in if it sees the proxy:balancer
>> or proxy:fcgi prefix.
> 
> Unfortunately there are at least three: non-Apache, "old" Apache, and "new" 
> Apache (mod_proxy_fcgi). And the logic of those three paths overlaps -- 
> they're not completely separate.
> 
> That's what makes this hard. If we remove the proxy: prefix now, PHP-FPM "old 
> mode" fixups will kick in (in certain situations involving REDIRECT_URL etc.) 
> even if there is nothing to fix up. That's what prompted this conversation.
> 

In your scenario does "old mode" == "old" Apache or non Apache?

My idea was to send FCGI data such that PHP-FPM doesn't use *any*
Apache-related fixups. In other words, httpd sends "what it should"
and PHP-FPM "uses what it receives"

Re: Summary: Broken FastCGI with httpd

[Jacob Champion]

On 01/27/2017 04:56 AM, Jim Jagielski wrote:

Let me relook over fpm-main... from what I saw, there are only 2
logic paths: one if Apache and the other for everybody-else...
and the Apache path only kicks in if it sees the proxy:balancer
or proxy:fcgi prefix.


Unfortunately there are at least three: non-Apache, "old" Apache, and 
"new" Apache (mod_proxy_fcgi). And the logic of those three paths 
overlaps -- they're not completely separate.


That's what makes this hard. If we remove the proxy: prefix now, PHP-FPM 
"old mode" fixups will kick in (in certain situations involving 
REDIRECT_URL etc.) even if there is nothing to fix up. That's what 
prompted this conversation.


--Jacob

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

Ahh... thanks! Yeah, it looks like other than removing the
prefix (and host:port), it does nothing more Apache-wise.
> On Jan 27, 2017, at 8:28 AM, Luca Toscano  wrote:
> 
> 
> 
> 2017-01-27 13:56 GMT+01:00 Jim Jagielski :
> 
> > On Jan 26, 2017, at 6:13 PM, Jacob Champion  wrote:
> >
> >
> > +1 (just not for 2.4.26, per my OP in the thread -- there is no way I can 
> > find around the current PHP-FPM "fixups" without compatibility-breaking 
> > behavior).
> >
> 
> Let me relook over fpm-main... from what I saw, there are only 2
> logic paths: one if Apache and the other for everybody-else...
> and the Apache path only kicks in if it sees the proxy:balancer
> or proxy:fcgi prefix.
> 
> My thought was to have mod_proxy_fcgi now start sending "correct"
> values for these envvars requiring NO fixups... ie, PHP-FPM
> will handle those envvars as it does for everybody else.
> 
> Isn't that the better solution? Or is your concern about non PHP-FPM
> fcgi backends and *that* compatibility concern?
> 
> 
> HHVM is another important PHP FCGI backend that might be interesting to take 
> into consideration. It seems that there is a check for mod_proxy_fcgi's 
> prefix in here too:
> 
> https://github.com/facebook/hhvm/blob/HHVM-3.17/hphp/runtime/server/fastcgi/fastcgi-transport.cpp#L243-L244
> 
> Luca

Re: Summary: Broken FastCGI with httpd

[Luca Toscano]

2017-01-27 13:56 GMT+01:00 Jim Jagielski :

>
> > On Jan 26, 2017, at 6:13 PM, Jacob Champion 
> wrote:
> >
> >
> > +1 (just not for 2.4.26, per my OP in the thread -- there is no way I
> can find around the current PHP-FPM "fixups" without compatibility-breaking
> behavior).
> >
>
> Let me relook over fpm-main... from what I saw, there are only 2
> logic paths: one if Apache and the other for everybody-else...
> and the Apache path only kicks in if it sees the proxy:balancer
> or proxy:fcgi prefix.
>
> My thought was to have mod_proxy_fcgi now start sending "correct"
> values for these envvars requiring NO fixups... ie, PHP-FPM
> will handle those envvars as it does for everybody else.
>
> Isn't that the better solution? Or is your concern about non PHP-FPM
> fcgi backends and *that* compatibility concern?
>
>
HHVM is another important PHP FCGI backend that might be interesting to
take into consideration. It seems that there is a check for
mod_proxy_fcgi's prefix in here too:

https://github.com/facebook/hhvm/blob/HHVM-3.17/hphp/runtime/server/fastcgi/fastcgi-transport.cpp#L243-L244

Luca

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

> On Jan 26, 2017, at 6:13 PM, Jacob Champion  wrote:
> 
> 
> +1 (just not for 2.4.26, per my OP in the thread -- there is no way I can 
> find around the current PHP-FPM "fixups" without compatibility-breaking 
> behavior).
> 

Let me relook over fpm-main... from what I saw, there are only 2
logic paths: one if Apache and the other for everybody-else...
and the Apache path only kicks in if it sees the proxy:balancer
or proxy:fcgi prefix.

My thought was to have mod_proxy_fcgi now start sending "correct"
values for these envvars requiring NO fixups... ie, PHP-FPM
will handle those envvars as it does for everybody else.

Isn't that the better solution? Or is your concern about non PHP-FPM
fcgi backends and *that* compatibility concern?

Re: Summary: Broken FastCGI with httpd

[Jacob Champion]

On 01/26/2017 10:59 AM, Jim Jagielski wrote:

We can easily handle generic FCGI expectations via the script that
Eric referred to... what's harder is that PHP does it's own
adjustments of what we send.


Right, there are tests for our CGI compliance in general (PR 51517) and 
then there are integration tests for PHP-FPM in particular. We can do 
both if we want.



Ideally, I'd like for us to stop sending PHP-FPM that whole
proxy:fcgi: stuff (ie, remove that prefix) and focus on just
sending the "right" values for:

 SCRIPT_FILENAME
 SCRIPT_NAME
 PATH_INFO
 PATH_TRANSLATED
 QUERY_STRING


+1 (just not for 2.4.26, per my OP in the thread -- there is no way I 
can find around the current PHP-FPM "fixups" without 
compatibility-breaking behavior).


--Jacob

Re: Summary: Broken FastCGI with httpd

[David Zuelke]

On 26.01.2017, at 18:03, Jim Jagielski  wrote:
> 
> As of HEAD on trunk, configs with the below seem to
> work as expected:
> 
>   AddType application/x-php7-fpm .php
>   Action application/x-php7-fpm /fpm virtual
>   
>SetHandler proxy:fcgi://localhost:9001
>   
> 
> --
> 
>   
> SetHandler "proxy:fcgi://localhost:9001
>   
> 
> What other scenarios should we be looking at?

Maybe add tests for "proxy:unix:/path/to/fpm.sock|fcgi://dummy", and balancers 
as well?

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

> On Jan 26, 2017, at 1:38 PM, Jacob Champion  wrote:
> 
> On 01/26/2017 09:49 AM, Eric Covener wrote:
>> Do you have a scenario that sends a bad var in 2.4 that is corrected
>> by the recent trunk changes?
> 
> I just need to start adding test cases to the suite, I think. That way we're 
> all on the same page as to the things that need to be fixed.
> 

We can easily handle generic FCGI expectations via the script that
Eric referred to... what's harder is that PHP does it's own
adjustments of what we send.

Ideally, I'd like for us to stop sending PHP-FPM that whole
proxy:fcgi: stuff (ie, remove that prefix) and focus on just
sending the "right" values for:

 SCRIPT_FILENAME
 SCRIPT_NAME
 PATH_INFO
 PATH_TRANSLATED
 QUERY_STRING

That is, we send a SCRIPT_FILENAME that PHP-FPM does NOT see
as the trigger that this-is-Apache and does its internal hacking.
If we do *that*, then we can use the perl script as our test center.
That, IMO, would be my pref.

Re: Summary: Broken FastCGI with httpd

[Jacob Champion]

On 01/26/2017 09:49 AM, Eric Covener wrote:

Do you have a scenario that sends a bad var in 2.4 that is corrected
by the recent trunk changes?


I just need to start adding test cases to the suite, I think. That way 
we're all on the same page as to the things that need to be fixed.


--Jacob

Re: Summary: Broken FastCGI with httpd

[Jacob Champion]

On 01/25/2017 09:30 PM, Eric Covener wrote:

But since it's a 2.4.23 regression for fpm, I am looking for the
minimum change, because there are just too many potential fastcgi
servers we could be contacting.


Right, I agree. In fact I'd almost be fine with just reverting my patch 
and the query followup, and shipping in 2.4.26 that *without* a toggle 
(not sure how the rest of you would feel about that given that we've 
been shipping with the new behavior for six months).


My goal with this conversation was to start putting the correct behavior 
in trunk, for an eventual backport of the whole fix to 2.4.x under a 
toggle, and a change to that behavior permanently in 2.next once we 
verify that PHP is on board.


--Jacob

Re: Summary: Broken FastCGI with httpd

[Eric Covener]

On Thu, Jan 26, 2017 at 12:03 PM, Jim Jagielski  wrote:
> As of HEAD on trunk, configs with the below seem to
> work as expected:
>
>AddType application/x-php7-fpm .php
>Action application/x-php7-fpm /fpm virtual
>
> SetHandler proxy:fcgi://localhost:9001
>
>
>  --
>
>gt;
>  SetHandler "proxy:fcgi://localhost:9001
>

For this case, looking at the vars sent,  2.4.x and trunk only differ
by proxy:fcgi://... being restored on trunk for SCRIPT_FILENAME.

This holds in the addl config mod_rewrite perdir is also mapping the
real URL to some actual php file.

IOW PATH_INFO/PATH_TRANSLATED were already right / the requested php
file, even in 2.4, for these cases.

Do you have a scenario that sends a bad var in 2.4 that is corrected
by the recent trunk changes?

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

As of HEAD on trunk, configs with the below seem to
work as expected:

   AddType application/x-php7-fpm .php
   Action application/x-php7-fpm /fpm virtual
   
SetHandler proxy:fcgi://localhost:9001
   

 --

   
 SetHandler "proxy:fcgi://localhost:9001
   

What other scenarios should we be looking at?

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

Yeah... that was something brought up earlier... leveraging
ap_expr with access to things like r->path_info, etc directly.
Right now, we don't have that mechanism, although we could
likely leverage mod_lua maybe to hook in after subprocess_env
has been populated and then adjust as needed.

> On Jan 26, 2017, at 7:56 AM, Eric Covener  wrote:
> 
> On Thu, Jan 26, 2017 at 7:49 AM, Jim Jagielski  wrote:
>> It seems that in many, many cases, we just to "adjust" a handful of
>> the ennvars populated by ap_add_cgi_vars()... Right now, with my
>> patch, we are doing it directly and explicitly in mod_proxy_fcgi.
>> No doubt there are other places that need such special treatment
>> as well.
>> 
>> One possibility would be to continue in that fashion... Another
>> would be to add some sort of hook/callback/fixup to ap_add_cgi_vars()
>> that modules like mod_include, mod_proxy_fcgi,... can register.
>> 
>> The latter seems "cleaner", for some reason, but most likely
>> overkill for the limited cases we are dealing with.
> 
> One thing Jacob brought up on iRC was the way nginx allows/encourages
> you to set these things direclty by splitting/smashing variables
> together.  This might be good as a last resort so when people find
> quirks they can at least resolve them without having every permutation
> accounted for in the code.
> 
> maybe we can peek at FCGI_FOO variables and overlay FOO after we call
> the ap_add_cgi_vars?  And people can use setenv/setenvif?
> 
> Dunno if the evaluation is late enough though. You need to see the
> output of the ap_add_* which to me implied you'd need an
> expression-aware  FCIG directive that is really evaluated right where
> we'd do the override.
> 
> -- 
> Eric Covener
> cove...@gmail.com

Re: Summary: Broken FastCGI with httpd

[Eric Covener]

On Thu, Jan 26, 2017 at 7:56 AM, Eric Covener  wrote:
>
> Dunno if the evaluation is late enough though. You need to see the
> output of the ap_add_* which to me implied you'd need an
> expression-aware  FCIG directive that is really evaluated right where
> we'd do the override.

I've added expression support to a few things, so I feel like i could
probably knock this out if people were into it.

-- 
Eric Covener
cove...@gmail.com

Re: Summary: Broken FastCGI with httpd

[Eric Covener]

On Thu, Jan 26, 2017 at 7:49 AM, Jim Jagielski  wrote:
> It seems that in many, many cases, we just to "adjust" a handful of
> the ennvars populated by ap_add_cgi_vars()... Right now, with my
> patch, we are doing it directly and explicitly in mod_proxy_fcgi.
> No doubt there are other places that need such special treatment
> as well.
>
> One possibility would be to continue in that fashion... Another
> would be to add some sort of hook/callback/fixup to ap_add_cgi_vars()
> that modules like mod_include, mod_proxy_fcgi,... can register.
>
> The latter seems "cleaner", for some reason, but most likely
> overkill for the limited cases we are dealing with.

One thing Jacob brought up on iRC was the way nginx allows/encourages
you to set these things direclty by splitting/smashing variables
together.  This might be good as a last resort so when people find
quirks they can at least resolve them without having every permutation
accounted for in the code.

maybe we can peek at FCGI_FOO variables and overlay FOO after we call
the ap_add_cgi_vars?  And people can use setenv/setenvif?

Dunno if the evaluation is late enough though. You need to see the
output of the ap_add_* which to me implied you'd need an
expression-aware  FCIG directive that is really evaluated right where
we'd do the override.

-- 
Eric Covener
cove...@gmail.com

Re: Summary: Broken FastCGI with httpd

[Eric Covener]

On Thu, Jan 26, 2017 at 4:07 AM, David Zuelke  wrote:
>
> RewriteRule ^ index.php [L], GET /ohai:
>
> $_SERVER['SCRIPT_NAME'] /index.php
> $_SERVER['REQUEST_URI'] /ohai
> $_SERVER['REDIRECT_URL']/ohai
> $_SERVER['SCRIPT_FILENAME'] /app/index.php
>
> Last case is missing PATH_INFO and PATH_TRANSLATED. That's what many PHP 
> frameworks (e.g. Laravel or Symfony) use, and I think they pull the URL 
> straight from REQUEST_URI now.

I think part of the ambiguity here is that w/ mod_fastcgi in front,
we'd force PATH_INFO to show up because mod_fastcgi wanted
SCRIPT_FILENAME to point to the interpreter?  Or at least that somehow
became the convention.

-- 
Eric Covener
cove...@gmail.com

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

It seems that in many, many cases, we just to "adjust" a handful of
the ennvars populated by ap_add_cgi_vars()... Right now, with my
patch, we are doing it directly and explicitly in mod_proxy_fcgi.
No doubt there are other places that need such special treatment
as well.

One possibility would be to continue in that fashion... Another
would be to add some sort of hook/callback/fixup to ap_add_cgi_vars()
that modules like mod_include, mod_proxy_fcgi,... can register.

The latter seems "cleaner", for some reason, but most likely
overkill for the limited cases we are dealing with.

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

The long and short is that if we can get SCRIPT_NAME right,
the rest is cake...

> On Jan 26, 2017, at 4:07 AM, David Zuelke  wrote:
> 
> On 26.01.2017, at 06:16, Eric Covener  wrote:
>> 
>> On Wed, Jan 25, 2017 at 6:12 PM, David Zuelke  wrote:
 AddType application/x-php7-fpm .php
 Action application/x-php7-fpm /php7-fpm virtual
 
   SetHandler proxy:fcgi://localhost:9000
 
 
 setup. This shows what the httpd conf looks like...
 can you provide a barebones fpm setup?
 
 Thx!
>>> 
>>> Just curious; what's the practical difference between that and something 
>>> like:
>>> 
>>> 
>>># make sure the file exists so that if not, 
>>> Apache will show its 404 page and not FPM
>>>   SetHandler proxy:fcgi://…
>>>   
>>> 
>> 
>> I think only the former would pass the original requested path as
>> PATH_INFO, unless there was more config w/ the latter (e.g. sending
>> everything to some single front controller so this snippet always sees
>> index.php)
> 
> Appears to depend on the rewrite (all of these with 2.4.18, SetHandler, and 
> cgi.fix_pathinfo=1):
> 
> GET index.php/ohai:
> 
> $_SERVER['PATH_TRANSLATED']   redirect:/index.php/ohai
> $_SERVER['PATH_INFO'] /ohai
> $_SERVER['SCRIPT_NAME']   /index.php
> $_SERVER['REQUEST_URI']   /index.php/ohai
> $_SERVER['SCRIPT_FILENAME']   /app/index.php
> 
> RewriteRule ^(.+)$ index.php/$1 [L], GET /ohai:
> 
> $_SERVER['PATH_TRANSLATED']   redirect:/index.php/ohai
> $_SERVER['PATH_INFO'] /ohai
> $_SERVER['SCRIPT_NAME']   /index.php
> $_SERVER['REQUEST_URI']   /ohai
> $_SERVER['REDIRECT_URL']  /ohai
> $_SERVER['SCRIPT_FILENAME']   /app/index.php
> 
> RewriteRule ^ index.php [L], GET /ohai:
> 
> $_SERVER['SCRIPT_NAME']   /index.php
> $_SERVER['REQUEST_URI']   /ohai
> $_SERVER['REDIRECT_URL']  /ohai
> $_SERVER['SCRIPT_FILENAME']   /app/index.php
> 
> Last case is missing PATH_INFO and PATH_TRANSLATED. That's what many PHP 
> frameworks (e.g. Laravel or Symfony) use, and I think they pull the URL 
> straight from REQUEST_URI now.
> 
> David
> 

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

Is this via AddType/Action?

> On Jan 25, 2017, at 11:37 PM, Eric Covener  wrote:
> 
> Seems like w/ the update it's still a bogus PATH_INFO that fpm is correcting
> 
>  'PATH_INFO' => '/foo.php/bar/baz/',
> 
> I'm using this as the target from one of the old bug reports.
> 
> #!/usr/bin/perl
> use FCGI;
> use Socket;
> use FCGI::ProcManager;
> use Data::Dumper;
> 
> $num_args = $#ARGV + 1;
> if ($num_args != 1) {
> print "\nUsage: fcgi.pl \n";
> exit 1;
> }
> 
> $proc_manager = FCGI::ProcManager->new( {n_processes => 1} );
> $socket = FCGI::OpenSocket( $ARGV[0], 10 );
> $request = FCGI::Request( \*STDIN, \*STDOUT, \*STDERR, \%req_params,
> $socket, ::FAIL_ACCEPT_ON_INTR );
> $proc_manager->pm_manage();
> if ($request) {
> while ( $request->Accept() >= 0 ) {
>   $proc_manager->pm_pre_dispatch();
>   print("Content-type: text/plain\r\n\r\n");
>   print Dumper(\%req_params);
> }
> }
> FCGI::CloseSocket($socket);
> 
> On Wed, Jan 25, 2017 at 11:31 PM, Jim Jagielski  wrote:
>> OK... I just committed something that in the AddType/Action
>> setup has reasonable values w/ php-fpm70
> 
> 
> 
> -- 
> Eric Covener
> cove...@gmail.com

Re: Summary: Broken FastCGI with httpd

[David Zuelke]

On 26.01.2017, at 06:16, Eric Covener  wrote:
> 
> On Wed, Jan 25, 2017 at 6:12 PM, David Zuelke  wrote:
>>>  AddType application/x-php7-fpm .php
>>>  Action application/x-php7-fpm /php7-fpm virtual
>>>  
>>>SetHandler proxy:fcgi://localhost:9000
>>>  
>>> 
>>> setup. This shows what the httpd conf looks like...
>>> can you provide a barebones fpm setup?
>>> 
>>> Thx!
>> 
>> Just curious; what's the practical difference between that and something 
>> like:
>> 
>> 
>> # make sure the file exists so that if not, 
>> Apache will show its 404 page and not FPM
>>SetHandler proxy:fcgi://…
>>
>> 
> 
> I think only the former would pass the original requested path as
> PATH_INFO, unless there was more config w/ the latter (e.g. sending
> everything to some single front controller so this snippet always sees
> index.php)

Appears to depend on the rewrite (all of these with 2.4.18, SetHandler, and 
cgi.fix_pathinfo=1):

GET index.php/ohai:

$_SERVER['PATH_TRANSLATED'] redirect:/index.php/ohai
$_SERVER['PATH_INFO']   /ohai
$_SERVER['SCRIPT_NAME'] /index.php
$_SERVER['REQUEST_URI'] /index.php/ohai
$_SERVER['SCRIPT_FILENAME'] /app/index.php

RewriteRule ^(.+)$ index.php/$1 [L], GET /ohai:

$_SERVER['PATH_TRANSLATED'] redirect:/index.php/ohai
$_SERVER['PATH_INFO']   /ohai
$_SERVER['SCRIPT_NAME'] /index.php
$_SERVER['REQUEST_URI'] /ohai
$_SERVER['REDIRECT_URL']/ohai
$_SERVER['SCRIPT_FILENAME'] /app/index.php

RewriteRule ^ index.php [L], GET /ohai:

$_SERVER['SCRIPT_NAME'] /index.php
$_SERVER['REQUEST_URI'] /ohai
$_SERVER['REDIRECT_URL']/ohai
$_SERVER['SCRIPT_FILENAME'] /app/index.php

Last case is missing PATH_INFO and PATH_TRANSLATED. That's what many PHP 
frameworks (e.g. Laravel or Symfony) use, and I think they pull the URL 
straight from REQUEST_URI now.

David

Re: Summary: Broken FastCGI with httpd

[Eric Covener]

I'm testing your recent config and the config from
https://github.com/apache/httpd/commit/cab0bfbb2645bb8f689535e5e2834e2dbc23f5a5#commitcomment-20393588
w/ htaccess that makes index.php act like a front controller.  Lately
I look at the output of the fakefpm.pl script I posted above, but was
originally pointing at a simple fpm server.

But since it's a 2.4.23 regression for fpm, I am looking for the
minimum change, because there are just too many potential fastcgi
servers we could be contacting.

IOW why make other tweaks here w/o opt-in if the only report we have
is the guy who is broken by the fcgi:// stripping?  I'm just afraid we
won't see the problems in synthetic tests.



On Wed, Jan 25, 2017 at 11:37 PM, Eric Covener  wrote:
> Seems like w/ the update it's still a bogus PATH_INFO that fpm is correcting
>
>   'PATH_INFO' => '/foo.php/bar/baz/',
>
> I'm using this as the target from one of the old bug reports.
>
> #!/usr/bin/perl
> use FCGI;
> use Socket;
> use FCGI::ProcManager;
> use Data::Dumper;
>
> $num_args = $#ARGV + 1;
> if ($num_args != 1) {
>  print "\nUsage: fcgi.pl \n";
>  exit 1;
> }
>
> $proc_manager = FCGI::ProcManager->new( {n_processes => 1} );
> $socket = FCGI::OpenSocket( $ARGV[0], 10 );
> $request = FCGI::Request( \*STDIN, \*STDOUT, \*STDERR, \%req_params,
> $socket, ::FAIL_ACCEPT_ON_INTR );
> $proc_manager->pm_manage();
> if ($request) {
>  while ( $request->Accept() >= 0 ) {
>$proc_manager->pm_pre_dispatch();
>print("Content-type: text/plain\r\n\r\n");
>print Dumper(\%req_params);
>  }
> }
> FCGI::CloseSocket($socket);
>
> On Wed, Jan 25, 2017 at 11:31 PM, Jim Jagielski  wrote:
>> OK... I just committed something that in the AddType/Action
>> setup has reasonable values w/ php-fpm70
>
>
>
> --
> Eric Covener
> cove...@gmail.com



-- 
Eric Covener
cove...@gmail.com

Re: Summary: Broken FastCGI with httpd

[Eric Covener]

On Wed, Jan 25, 2017 at 6:12 PM, David Zuelke  wrote:
>>   AddType application/x-php7-fpm .php
>>   Action application/x-php7-fpm /php7-fpm virtual
>>   
>> SetHandler proxy:fcgi://localhost:9000
>>   
>>
>> setup. This shows what the httpd conf looks like...
>> can you provide a barebones fpm setup?
>>
>> Thx!
>
> Just curious; what's the practical difference between that and something like:
>
> 
>  # make sure the file exists so that if not, 
> Apache will show its 404 page and not FPM
> SetHandler proxy:fcgi://…
> 
> 

I think only the former would pass the original requested path as
PATH_INFO, unless there was more config w/ the latter (e.g. sending
everything to some single front controller so this snippet always sees
index.php)

Re: Summary: Broken FastCGI with httpd

[Eric Covener]

Seems like w/ the update it's still a bogus PATH_INFO that fpm is correcting

  'PATH_INFO' => '/foo.php/bar/baz/',

I'm using this as the target from one of the old bug reports.

#!/usr/bin/perl
use FCGI;
use Socket;
use FCGI::ProcManager;
use Data::Dumper;

$num_args = $#ARGV + 1;
if ($num_args != 1) {
 print "\nUsage: fcgi.pl \n";
 exit 1;
}

$proc_manager = FCGI::ProcManager->new( {n_processes => 1} );
$socket = FCGI::OpenSocket( $ARGV[0], 10 );
$request = FCGI::Request( \*STDIN, \*STDOUT, \*STDERR, \%req_params,
$socket, ::FAIL_ACCEPT_ON_INTR );
$proc_manager->pm_manage();
if ($request) {
 while ( $request->Accept() >= 0 ) {
   $proc_manager->pm_pre_dispatch();
   print("Content-type: text/plain\r\n\r\n");
   print Dumper(\%req_params);
 }
}
FCGI::CloseSocket($socket);

On Wed, Jan 25, 2017 at 11:31 PM, Jim Jagielski  wrote:
> OK... I just committed something that in the AddType/Action
> setup has reasonable values w/ php-fpm70



-- 
Eric Covener
cove...@gmail.com

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

OK... I just committed something that in the AddType/Action
setup has reasonable values w/ php-fpm70

Re: Summary: Broken FastCGI with httpd

[David Zuelke]

On 25.01.2017, at 20:12, Jim Jagielski  wrote:
> 
> Can you provide to me a pgp-fpm.conf that you use... Basically,
> I want to create an environ that exactly uses the
> 
>   AddType application/x-php7-fpm .php
>   Action application/x-php7-fpm /php7-fpm virtual
>   
> SetHandler proxy:fcgi://localhost:9000
>   
> 
> setup. This shows what the httpd conf looks like...
> can you provide a barebones fpm setup?
> 
> Thx!

Just curious; what's the practical difference between that and something like:


 # make sure the file exists so that if not, 
Apache will show its 404 page and not FPM
SetHandler proxy:fcgi://…



?

David

Re: Summary: Broken FastCGI with httpd

[Jacob Champion]

On 01/25/2017 12:21 PM, Jim Jagielski wrote:

OK, got it working and just saw that in these cases, proxy_fcgi_canon()
isn't even called... which throws that whole idea out of
the water.


I argue that this is a bug. It's made other PHP fixes difficult. See 
[1], about halfway through the message.


(Hopefully, my bold declaration that this is a bug will bring someone 
out of the woodwork who can explain how it is not a bug, and why it is 
this way.)


--Jacob

[1] 
https://lists.apache.org/thread.html/87fd8d1dc208b7d74fadf7b3929e71cfd7279c5c5b9b2566386cb684@%3Cdev.httpd.apache.org%3E

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

OK, got it working and just saw that in these cases, proxy_fcgi_canon()
isn't even called... which throws that whole idea out of
the water.

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

Can you provide to me a pgp-fpm.conf that you use... Basically,
I want to create an environ that exactly uses the

   AddType application/x-php7-fpm .php
   Action application/x-php7-fpm /php7-fpm virtual
   
 SetHandler proxy:fcgi://localhost:9000
   

setup. This shows what the httpd conf looks like...
can you provide a barebones fpm setup?

Thx!

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

The idea was to key in on the extension specified in the AddType line.

> On Jan 24, 2017, at 8:43 AM, David Zuelke  wrote:
> 
> On 24.01.2017, at 01:24, Jim Jagielski  wrote:
>> 
>> 
>>> On Jan 23, 2017, at 4:35 PM, Jacob Champion  wrote:
>>> 
>>> 
>>> What situations lead to CONTENT_TYPE being set to a PATH_INFO delimiter? 
>>> I'm not sure what this is supposed to do.
>>> 
>> 
>> The *idea* was to look for ".php" in the actual URL for example :)
> 
> It doesn't have to be ".php" for PHP to be able to run it. You can execute 
> any file extension (that sometimes requires fiddling with settings, e.g. with 
> PHP-FPM, but still).
> 
> David
> 
> 
> 
> 

Re: Summary: Broken FastCGI with httpd

[David Zuelke]

On 24.01.2017, at 01:24, Jim Jagielski  wrote:
> 
> 
>> On Jan 23, 2017, at 4:35 PM, Jacob Champion  wrote:
>> 
>> 
>> What situations lead to CONTENT_TYPE being set to a PATH_INFO delimiter? I'm 
>> not sure what this is supposed to do.
>> 
> 
> The *idea* was to look for ".php" in the actual URL for example :)

It doesn't have to be ".php" for PHP to be able to run it. You can execute any 
file extension (that sometimes requires fiddling with settings, e.g. with 
PHP-FPM, but still).

David

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

> On Jan 23, 2017, at 4:35 PM, Jacob Champion  wrote:
> 
> 
> What situations lead to CONTENT_TYPE being set to a PATH_INFO delimiter? I'm 
> not sure what this is supposed to do.
> 

The *idea* was to look for ".php" in the actual URL for example :)

Re: Summary: Broken FastCGI with httpd

[Jacob Champion]

On 01/23/2017 09:05 AM, Jim Jagielski wrote:

OK, I was thinking something like this, which tries to "compartmentalize"
it to where we actually create the CGI vars...

Anyone able to test?


This patch doesn't seem to change anything in my limited tests so far. 
Comments inline:



diff --git a/modules/mappers/mod_actions.c b/modules/mappers/mod_actions.c
index 2a67a2742..efe22f814 100644
--- a/modules/mappers/mod_actions.c
+++ b/modules/mappers/mod_actions.c
@@ -186,7 +186,8 @@ static int action_handler(request_rec *r)
 ap_field_noparam(r->pool, r->content_type);

 if (action && (t = apr_table_get(conf->action_types, action))) {
-if (*t++ == '0' && r->finfo.filetype == APR_NOFILE) {
+int virtual = (*t++ == '0' ? 0 : 1);
+if (!virtual && r->finfo.filetype == APR_NOFILE) {
 ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00652)
   "File does not exist: %s", r->filename);
 return HTTP_NOT_FOUND;
@@ -197,6 +198,9 @@ static int action_handler(request_rec *r)
  * (will be REDIRECT_HANDLER there)
  */
 apr_table_setn(r->subprocess_env, "HANDLER", action);
+if (virtual) {
+apr_table_setn(r->subprocess_env, "virtual_script", "1");
+}
 }

 if (script == NULL)
diff --git a/modules/proxy/mod_proxy_fcgi.c b/modules/proxy/mod_proxy_fcgi.c
index 6f7bda009..ff7f58b90 100644
--- a/modules/proxy/mod_proxy_fcgi.c
+++ b/modules/proxy/mod_proxy_fcgi.c
@@ -141,9 +141,12 @@ static int proxy_fcgi_canon(request_rec *r, char *url)
 if (!strcasecmp(pathinfo_type, "unescape")) {
 ap_unescape_url_keep2f(r->path_info, 0);
 }
+}
+if (r->path_info && *r->path_info) {
 ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01061)
-"set r->path_info to %s", r->path_info);
+  "set r->path_info to %s", r->path_info);
 }
+
 }

 return OK;
diff --git a/server/util_script.c b/server/util_script.c
index 0f12bacc2..0d6ec9313 100644
--- a/server/util_script.c
+++ b/server/util_script.c
@@ -381,6 +381,7 @@ AP_DECLARE(void) ap_add_cgi_vars(request_rec *r)
 core_dir_config *conf =
 (core_dir_config *)ap_get_core_module_config(r->per_dir_config);
 int request_uri_from_original = 1;
+int path_info_exists = r->path_info && r->path_info[0];
 const char *request_uri_rule;

 apr_table_setn(e, "GATEWAY_INTERFACE", "CGI/1.1");
@@ -406,13 +407,43 @@ AP_DECLARE(void) ap_add_cgi_vars(request_rec *r)

 if (!strcmp(r->protocol, "INCLUDED")) {
 apr_table_setn(e, "SCRIPT_NAME", r->uri);
-if (r->path_info && *r->path_info) {
+if (path_info_exists) {
 apr_table_setn(e, "PATH_INFO", r->path_info);
 }
 }
-else if (!r->path_info || !*r->path_info) {
+else if (!path_info_exists) {
 apr_table_setn(e, "SCRIPT_NAME", r->uri);
 }
+/*
+ * Deal with special cases where the scripts are virtual
+ * (external) and we have r->filename of the form:
+ * proxy:fcgi://127.0.0.1:9000/path/to/test.php/foo/bar/
+ * proxy:balancer://127.0.0.1:9000/path/to/test.php/foo1/bar2/
+ */
+
+else if (apr_table_get(e, "virtual_script")) {


By the time we get here for my Action tests, "virtual_script" has 
already been renamed to "REDIRECT_virtual_script".



+char *path;
+char *script;
+char *scan;
+script = apr_pstrdup(r->pool, r->path_info);
+scan = (char *)apr_table_get(e, "CONTENT_TYPE");


What situations lead to CONTENT_TYPE being set to a PATH_INFO delimiter? 
I'm not sure what this is supposed to do.



+if (!scan) {
+scan = ".";
+}
+path = ap_strstr(script, scan);
+if (path) {
+while (*path && *path != '/') {
+path++;
+}
+if (*path) {
+*path = '\0';
+path++;
+}
+}
+apr_table_setn(e, "SCRIPT_NAME", script);
+apr_table_setn(e, "PATH_INFO", apr_pstrcat(r->pool, "/",
+   ((path && *path) ? path : ""), NULL));


This appears to give PATH_INFO a leading slash even if there was no 
slash after the script in the URI.



+}
 else {
 int path_info_start = ap_find_path_info(r->uri, r->path_info);

@@ -422,7 +453,7 @@ AP_DECLARE(void) ap_add_cgi_vars(request_rec *r)
 apr_table_setn(e, "PATH_INFO", r->path_info);
 }

-if (r->path_info && r->path_info[0]) {
+if (path_info_exists) {
 /*
  * To get PATH_TRANSLATED, treat PATH_INFO as a URI path.
  * Need to re-escape it for this, since the entire URI was

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

OK, I was thinking something like this, which tries to "compartmentalize"
it to where we actually create the CGI vars...

Anyone able to test?

diff --git a/modules/mappers/mod_actions.c b/modules/mappers/mod_actions.c
index 2a67a2742..efe22f814 100644
--- a/modules/mappers/mod_actions.c
+++ b/modules/mappers/mod_actions.c
@@ -186,7 +186,8 @@ static int action_handler(request_rec *r)
 ap_field_noparam(r->pool, r->content_type);
 
 if (action && (t = apr_table_get(conf->action_types, action))) {
-if (*t++ == '0' && r->finfo.filetype == APR_NOFILE) {
+int virtual = (*t++ == '0' ? 0 : 1);
+if (!virtual && r->finfo.filetype == APR_NOFILE) {
 ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00652)
   "File does not exist: %s", r->filename);
 return HTTP_NOT_FOUND;
@@ -197,6 +198,9 @@ static int action_handler(request_rec *r)
  * (will be REDIRECT_HANDLER there)
  */
 apr_table_setn(r->subprocess_env, "HANDLER", action);
+if (virtual) {
+apr_table_setn(r->subprocess_env, "virtual_script", "1");
+}
 }
 
 if (script == NULL)
diff --git a/modules/proxy/mod_proxy_fcgi.c b/modules/proxy/mod_proxy_fcgi.c
index 6f7bda009..ff7f58b90 100644
--- a/modules/proxy/mod_proxy_fcgi.c
+++ b/modules/proxy/mod_proxy_fcgi.c
@@ -141,9 +141,12 @@ static int proxy_fcgi_canon(request_rec *r, char *url)
 if (!strcasecmp(pathinfo_type, "unescape")) { 
 ap_unescape_url_keep2f(r->path_info, 0);
 }
+}
+if (r->path_info && *r->path_info) {
 ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01061)
-"set r->path_info to %s", r->path_info);
+  "set r->path_info to %s", r->path_info);
 }
+
 }
 
 return OK;
diff --git a/server/util_script.c b/server/util_script.c
index 0f12bacc2..0d6ec9313 100644
--- a/server/util_script.c
+++ b/server/util_script.c
@@ -381,6 +381,7 @@ AP_DECLARE(void) ap_add_cgi_vars(request_rec *r)
 core_dir_config *conf =
 (core_dir_config *)ap_get_core_module_config(r->per_dir_config);
 int request_uri_from_original = 1;
+int path_info_exists = r->path_info && r->path_info[0];
 const char *request_uri_rule;
 
 apr_table_setn(e, "GATEWAY_INTERFACE", "CGI/1.1");
@@ -406,13 +407,43 @@ AP_DECLARE(void) ap_add_cgi_vars(request_rec *r)
 
 if (!strcmp(r->protocol, "INCLUDED")) {
 apr_table_setn(e, "SCRIPT_NAME", r->uri);
-if (r->path_info && *r->path_info) {
+if (path_info_exists) {
 apr_table_setn(e, "PATH_INFO", r->path_info);
 }
 }
-else if (!r->path_info || !*r->path_info) {
+else if (!path_info_exists) {
 apr_table_setn(e, "SCRIPT_NAME", r->uri);
 }
+/*
+ * Deal with special cases where the scripts are virtual
+ * (external) and we have r->filename of the form:
+ * proxy:fcgi://127.0.0.1:9000/path/to/test.php/foo/bar/
+ * proxy:balancer://127.0.0.1:9000/path/to/test.php/foo1/bar2/
+ */
+
+else if (apr_table_get(e, "virtual_script")) {
+char *path;
+char *script;
+char *scan;
+script = apr_pstrdup(r->pool, r->path_info);
+scan = (char *)apr_table_get(e, "CONTENT_TYPE");
+if (!scan) {
+scan = ".";
+}
+path = ap_strstr(script, scan);
+if (path) {
+while (*path && *path != '/') {
+path++;
+}
+if (*path) {
+*path = '\0';
+path++;
+}
+}
+apr_table_setn(e, "SCRIPT_NAME", script);
+apr_table_setn(e, "PATH_INFO", apr_pstrcat(r->pool, "/",
+   ((path && *path) ? path : ""), NULL));
+}
 else {
 int path_info_start = ap_find_path_info(r->uri, r->path_info);
 
@@ -422,7 +453,7 @@ AP_DECLARE(void) ap_add_cgi_vars(request_rec *r)
 apr_table_setn(e, "PATH_INFO", r->path_info);
 }
 
-if (r->path_info && r->path_info[0]) {
+if (path_info_exists) {
 /*
  * To get PATH_TRANSLATED, treat PATH_INFO as a URI path.
  * Need to re-escape it for this, since the entire URI was

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

> On Jan 19, 2017, at 1:08 PM, David Zuelke  wrote:
> 
> Yeah, not much interest yet, sorry :( Jim, I think you have a php.net 
> account; are you also on that mailing list and can chime in?
> 

It's been awhile... certainly before the Github switch.

Don't *think* I'm on the php-internals list, but will double check
and join if not 

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

Let me mull this over... basically, we want/need
to be able to read into the AddType directive the
MIME type and the suffix and then look for the 1st occurance
of a "file" with that suffix in the URL path and *assume*
that is the actual SCRIPT_NAME. Being aware that this
is virtual via the Action directive may actually make
that *easier*.

It's been awhile since I look at those code paths, and
not sure where they all roam and wind, but let me dig
in a little.

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

> On Jan 19, 2017, at 1:00 PM, Jacob Champion  wrote:
> 
>> Would it make sense, mostly from a PHP point-of-view, to send
>> something like SCRIPT_FILENAME_RAW (or even
>> APACHE_SCRIPT_FILENAME)... Or does that simply complicate an already
>> fuzzy and nebulous situation?
> 
> I think it probably makes things worse. It's just one more non-standard 
> variable for us to maintain and backends to manipulate.
> 

The idea is that they would get the "raw" URL... the whole thing.
Complete and unfiltered.

Yeah, maybe not :)

Re: Summary: Broken FastCGI with httpd

[David Zuelke]

On 19.01.2017, at 19:00, Jacob Champion  wrote:
> 
> On 01/18/2017 01:00 PM, Jim Jagielski wrote:
>> After all, it's easier for the FCGI server to know the SCRIPT_NAME
>> than httpd to "guess"...
> 
> I think the recent breakage calls this assumption into question. The server 
> admin knows exactly where the scripts are in the use cases we're currently 
> discussing; why should the backend have to "know" anything in those 
> situations?
> 
> It's worth noting that letting PHP try to split apart SCRIPT_FILENAME on its 
> own has led to security issues in the past [1,2]. They were mitigated by 
> other means on the PHP side, I believe, but we can avoid recurrences in 
> future FCGI implementations.
> 
>>> On Jan 18, 2017, at 2:01 PM, Jacob Champion  wrote:
>>> We only get QUERY_STRING right. (Well, I won't say SCRIPT_FILENAME is 
>>> "wrong", since there is no standard definition, but it's not helpful.) 
>>> SCRIPT_NAME is supposed to point to a possible script-path (see RFC 3875 
>>> for definitions), '/hello.php' in this instance. PATH_INFO and 
>>> PATH_TRANSLATED should not include 'hello.php' in their values.
>> 
>> That is because we have no idea what SCRIPT_NAME is...
> 
> One way to approach unknowns is to have httpd and/or the backend guess at 
> things; another way (that we can't start doing until a version-bump release) 
> is to refuse to send back a FastCGI request unless we know exactly what these 
> variables should be. That lets the user fix the issue and gain a better 
> understanding of what is going on, instead of relying on us and the backend 
> to perform magic together during the correct phase of the moon.
> 
>> With the "guess" that
>> it is /php7-fpm, then PATH_INFO kind of makes sense, since it is
>> the portion of the URI following the script. And considering
>> that we use
>> 
>>   Action application/x-php7-fpm /php7-fpm virtual
>> 
>> the 2nd parameter is specifically noted as *being the cgi-script* and
>> so of course httpd assumes that /php7-fpm is SCRIPT_NAME, because
>> we explicitly called it that :)
> 
> Fine by me. So then, in my opinion, it seems like one part of moving forward 
> should be to deprecate the use of Action to invoke script multiplexers like 
> PHP-FPM, and document accordingly. I.e. if you're not redirecting to *the 
> script*, as in standard CGI, no Action for you.
> 
> (That explicitly deprecates the mod_fastcgi model for use with PHP-FPM, which 
> would also be fine by me. I don't like the way it couples the public 
> URI-space to an implementation detail.)
> 
> Are there similar arguments for the variable values discussed in PR 51517? It 
> used ProxyPassMatch, not Action, for its implementation.

ProxyPassMatch has its own set of problems in combination with rewrites in e.g. 
.htaccess. The only method that I've found works reliably with any .htaccess 
rule that also works with e.g. mod_php is through SetHandler in 2.4.10+.


>> Would it make sense, mostly from a PHP point-of-view, to send
>> something like SCRIPT_FILENAME_RAW (or even
>> APACHE_SCRIPT_FILENAME)... Or does that simply complicate an already
>> fuzzy and nebulous situation?
> 
> I think it probably makes things worse. It's just one more non-standard 
> variable for us to maintain and backends to manipulate.

Agreed


>> Is [1] being used as the canonical place for this discussion w/ the
>> PHP Group?
>> 
>> 1. https://github.com/php/php-src/pull/694#issuecomment-271963956
> 
> I'm not sure if there is a canonical place yet...
> 
>http://news.php.net/php.internals/97810
> 
> is another possibility, if the conversation is picked up.

Yeah, not much interest yet, sorry :( Jim, I think you have a php.net account; 
are you also on that mailing list and can chime in?


> --Jacob
> 
> [1] https://forum.nginx.org/read.php?2,88845,88845#msg-88845
> [2] https://bugs.php.net/bug.php?id=55181
>(can't find a CVE right now, sorry)
> 

Re: Summary: Broken FastCGI with httpd

[Jacob Champion]

On 01/18/2017 01:00 PM, Jim Jagielski wrote:

After all, it's easier for the FCGI server to know the SCRIPT_NAME
than httpd to "guess"...


I think the recent breakage calls this assumption into question. The 
server admin knows exactly where the scripts are in the use cases we're 
currently discussing; why should the backend have to "know" anything in 
those situations?


It's worth noting that letting PHP try to split apart SCRIPT_FILENAME on 
its own has led to security issues in the past [1,2]. They were 
mitigated by other means on the PHP side, I believe, but we can avoid 
recurrences in future FCGI implementations.



On Jan 18, 2017, at 2:01 PM, Jacob Champion  wrote:
We only get QUERY_STRING right. (Well, I won't say SCRIPT_FILENAME is "wrong", 
since there is no standard definition, but it's not helpful.) SCRIPT_NAME is supposed to 
point to a possible script-path (see RFC 3875 for definitions), '/hello.php' in this 
instance. PATH_INFO and PATH_TRANSLATED should not include 'hello.php' in their values.


That is because we have no idea what SCRIPT_NAME is...


One way to approach unknowns is to have httpd and/or the backend guess 
at things; another way (that we can't start doing until a version-bump 
release) is to refuse to send back a FastCGI request unless we know 
exactly what these variables should be. That lets the user fix the issue 
and gain a better understanding of what is going on, instead of relying 
on us and the backend to perform magic together during the correct phase 
of the moon.



With the "guess" that
it is /php7-fpm, then PATH_INFO kind of makes sense, since it is
the portion of the URI following the script. And considering
that we use

   Action application/x-php7-fpm /php7-fpm virtual

the 2nd parameter is specifically noted as *being the cgi-script* and
so of course httpd assumes that /php7-fpm is SCRIPT_NAME, because
we explicitly called it that :)


Fine by me. So then, in my opinion, it seems like one part of moving 
forward should be to deprecate the use of Action to invoke script 
multiplexers like PHP-FPM, and document accordingly. I.e. if you're not 
redirecting to *the script*, as in standard CGI, no Action for you.


(That explicitly deprecates the mod_fastcgi model for use with PHP-FPM, 
which would also be fine by me. I don't like the way it couples the 
public URI-space to an implementation detail.)


Are there similar arguments for the variable values discussed in PR 
51517? It used ProxyPassMatch, not Action, for its implementation.



Would it make sense, mostly from a PHP point-of-view, to send
something like SCRIPT_FILENAME_RAW (or even
APACHE_SCRIPT_FILENAME)... Or does that simply complicate an already
fuzzy and nebulous situation?


I think it probably makes things worse. It's just one more non-standard 
variable for us to maintain and backends to manipulate.



Is [1] being used as the canonical place for this discussion w/ the
PHP Group?

1. https://github.com/php/php-src/pull/694#issuecomment-271963956


I'm not sure if there is a canonical place yet...

http://news.php.net/php.internals/97810

is another possibility, if the conversation is picked up.

--Jacob

[1] https://forum.nginx.org/read.php?2,88845,88845#msg-88845
[2] https://bugs.php.net/bug.php?id=55181
(can't find a CVE right now, sorry)

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

Would it make sense, mostly from a PHP point-of-view, to
send something like SCRIPT_FILENAME_RAW (or even
APACHE_SCRIPT_FILENAME)... Or does that simply
complicate an already fuzzy and nebulous situation?

Is [1] being used as the canonical place for this
discussion w/ the PHP Group?

1. https://github.com/php/php-src/pull/694#issuecomment-271963956

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

The thing is that when we are proxying back, some of those
values can't make sense, since, for example, there is no
"real" path on the httpd server that maps to the actual
request file

I think the original idea was to simply send the full URL to
the FCGI server, to let it parse things out for itself...
After all, it's easier for the FCGI server to know the SCRIPT_NAME
than httpd to "guess"... This was, iirc, the initial
concept related to how to leverage SCRIPT_FILENAME on
the PHP side.

What we need is some specific understanding between, in this
case, PHP-FPM and httpd on what it wants/expects/needs regarding
these env-vars. Yeah, SCRIPT_FILENAME seems core to this, I
think.

> On Jan 18, 2017, at 2:01 PM, Jacob Champion  wrote:
> 
> On 01/18/2017 06:43 AM, Jim Jagielski wrote:
>> Also, the fact that different methods of invoking FCGI result
>> in different vars, at 1st blush, doesn't seem "incorrect"
>> assuming that each difference makes some sense, in a way.
> 
> They don't make sense. For example, mod_proxy_fcgi can be set up in a way 
> that mirrors the mod_fastcgi preferred configuration, using an Action:
> 
>  AddType application/x-php7-fpm .php
>  Action application/x-php7-fpm /php7-fpm virtual
>  
>SetHandler proxy:fcgi://localhost:9000
>  
> 
> This results in the following variables when I access the URL 
> '/hello.php/path/info?query':
> 
>  SCRIPT_FILENAME: /var/www/php7-fpm
>  SCRIPT_NAME: /php7-fpm
>  PATH_INFO:   /hello.php/path/info
>  PATH_TRANSLATED: /var/www/hello.php/path/info
>  QUERY_STRING:query
> 
> We only get QUERY_STRING right. (Well, I won't say SCRIPT_FILENAME is 
> "wrong", since there is no standard definition, but it's not helpful.) 
> SCRIPT_NAME is supposed to point to a possible script-path (see RFC 3875 for 
> definitions), '/hello.php' in this instance. PATH_INFO and PATH_TRANSLATED 
> should not include 'hello.php' in their values.

That is because we have no idea what SCRIPT_NAME is... With the "guess" that
it is /php7-fpm, then PATH_INFO kind of makes sense, since it is
the portion of the URI following the script. And considering
that we use

   Action application/x-php7-fpm /php7-fpm virtual

the 2nd parameter is specifically noted as *being the cgi-script* and
so of course httpd assumes that /php7-fpm is SCRIPT_NAME, because
we explicitly called it that :)


> 
> It's not just Action. PR 51517 contains examples using ProxyPassMatch.
> 
> --Jacob

Re: Summary: Broken FastCGI with httpd

[Jacob Champion]

On 01/18/2017 06:47 AM, David Zuelke wrote:

Thanks for picking this up!


And thank you for driving this on the PHP side!

--Jacob

Re: Summary: Broken FastCGI with httpd

[Jacob Champion]

On 01/17/2017 11:46 PM, Stefan Eissing wrote:

Would it make sense to narrow down the setups to a few blessed ones that become 
part of our tests?


I think so. Problem is that I don't, myself, know what is actually in 
use out there. I keep learning about new ways to invoke CGI every month, 
it seems.


If anyone has a test matrix they're using for themselves, or even just a 
good list of "ways to configure (Fast)CGI", that'd be really useful.


--Jacob

Re: Summary: Broken FastCGI with httpd

[Jacob Champion]

On 01/18/2017 06:56 AM, Eric Covener wrote:

Unfortunately we might need our own directives for the overrides here
to get them to run after the normal vars are calculated and only when
we're in the proxy_fcgi  handler.


See https://bz.apache.org/bugzilla/show_bug.cgi?id=28903 for an older 
conversation about this.


--Jacob

Re: Summary: Broken FastCGI with httpd

[Jacob Champion]

On 01/18/2017 06:43 AM, Jim Jagielski wrote:

Also, the fact that different methods of invoking FCGI result
in different vars, at 1st blush, doesn't seem "incorrect"
assuming that each difference makes some sense, in a way.


They don't make sense. For example, mod_proxy_fcgi can be set up in a 
way that mirrors the mod_fastcgi preferred configuration, using an Action:


  AddType application/x-php7-fpm .php
  Action application/x-php7-fpm /php7-fpm virtual
  
SetHandler proxy:fcgi://localhost:9000
  

This results in the following variables when I access the URL 
'/hello.php/path/info?query':


  SCRIPT_FILENAME: /var/www/php7-fpm
  SCRIPT_NAME: /php7-fpm
  PATH_INFO:   /hello.php/path/info
  PATH_TRANSLATED: /var/www/hello.php/path/info
  QUERY_STRING:query

We only get QUERY_STRING right. (Well, I won't say SCRIPT_FILENAME is 
"wrong", since there is no standard definition, but it's not helpful.) 
SCRIPT_NAME is supposed to point to a possible script-path (see RFC 3875 
for definitions), '/hello.php' in this instance. PATH_INFO and 
PATH_TRANSLATED should not include 'hello.php' in their values.


It's not just Action. PR 51517 contains examples using ProxyPassMatch.

--Jacob

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

Just some additional info (the perl script described might be useful,
esp if we fold it into the test framework):

https://bugs.php.net/bug.php?id=54152


> On Jan 18, 2017, at 9:47 AM, David Zuelke  wrote:
> 
>> On 17.01.2017, at 23:16, Jacob Champion  wrote:
>> 
>> (This conversation is currently spread over Bugzilla, IRC, GitHub, and 
>> php-internals. Here's my attempt at summarizing it for all of you. If you 
>> have no interest in CGI or FastCGI, stop reading now.)
> 
> Thanks for picking this up!
> 
> 
>> 2) Define what SCRIPT_FILENAME means.
>> 
>> SCRIPT_FILENAME isn't actually a CGI 1.1 standard variable. We appear to 
>> have defined it as "whatever r->filename contains", so we've effectively 
>> coupled our implementation details to external clients. PHP-FPM and 
>> fcgiwrap, for example, assume that SCRIPT_FILENAME should point to the 
>> script that should be executed to handle the request. We need to standardize 
>> it.
> 
> There's one more caveat around SCRIPT_FILENAME, I think: it might not be the 
> same for httpd and the FCGI backend if they're running on separate machines!
> 
> David
> 

Re: Summary: Broken FastCGI with httpd

[Eric Covener]

On Wed, Jan 18, 2017 at 9:47 AM, David Zuelke  wrote:
> There's one more caveat around SCRIPT_FILENAME, I think: it might not be the 
> same for httpd and the FCGI backend if they're running on separate machines!

I think this goes to the notion of overriding these variables w/
config rather than trying to coax the server to try to construct them
correctly by rewriting.

Unfortunately we might need our own directives for the overrides here
to get them to run after the normal vars are calculated and only when
we're in the proxy_fcgi  handler.

-- 
Eric Covener
cove...@gmail.com

Re: Summary: Broken FastCGI with httpd

[David Zuelke]

> On 17.01.2017, at 23:16, Jacob Champion  wrote:
> 
> (This conversation is currently spread over Bugzilla, IRC, GitHub, and 
> php-internals. Here's my attempt at summarizing it for all of you. If you 
> have no interest in CGI or FastCGI, stop reading now.)

Thanks for picking this up!


> 2) Define what SCRIPT_FILENAME means.
> 
> SCRIPT_FILENAME isn't actually a CGI 1.1 standard variable. We appear to have 
> defined it as "whatever r->filename contains", so we've effectively coupled 
> our implementation details to external clients. PHP-FPM and fcgiwrap, for 
> example, assume that SCRIPT_FILENAME should point to the script that should 
> be executed to handle the request. We need to standardize it.

There's one more caveat around SCRIPT_FILENAME, I think: it might not be the 
same for httpd and the FCGI backend if they're running on separate machines!

David

Re: Summary: Broken FastCGI with httpd

[Jim Jagielski]

It seems to me that SCRIPT_FILENAME is the key w/ PHP-FPM, but
I could be wrong.

Also, the fact that different methods of invoking FCGI result
in different vars, at 1st blush, doesn't seem "incorrect"
assuming that each difference makes some sense, in a way.

Finally, I think that instead of trying to address these in
the various proxy modules/submodules, etc, we should do so
in util_script.c instead. One central place where we "canonically"
set these params, and handle the different ways r->filename may
be mangled ;)

Re: Summary: Broken FastCGI with httpd

[Stefan Eissing]

Thanks for the nice summary, Jacob. 

I find the topic of cgi in our server surprisingly complex. Sometimes mod_http2 
stumbles on "details" like conn->id to identify requests that Seemed a Good 
Idea at the time. 

Would it make sense to narrow down the setups to a few blessed ones that become 
part of our tests?

Stefan

> Am 17.01.2017 um 23:16 schrieb Jacob Champion :
> 
> (This conversation is currently spread over Bugzilla, IRC, GitHub, and 
> php-internals. Here's my attempt at summarizing it for all of you. If you 
> have no interest in CGI or FastCGI, stop reading now.)
> 
> == Backstory ==
> 
> Back in May I was testing some FCGI backends with mod_proxy_fcgi, and I found 
> a backend called fcgiwrap that didn't work. That conversation is in [1]. 
> SCRIPT_FILENAME was being set to a value beginning with 
> "proxy:fcgi://host:port", and that didn't make any sense. We patched the 
> problem in 2.4.21 by stripping the weird prefix and called it good.
> 
> Fast forward to find that in *some* cases, SCRIPT_FILENAME could still 
> contain weird values that didn't make any sense (in this case, a query 
> string), because proxy canonicalization doesn't run correctly in 
> per-directory rewrites. The conversation is in [2]. We stripped the query 
> string and called it good.
> 
> Fast forward to find that in *some* cases involving mod_rewrite, PHP-FPM now 
> refuses to operate correctly with mod_proxy_fcgi. That conversation is in 
> [3]. There is no patch, and we can't call this good anymore...
> 
> == Problem ==
> 
> PHP-FPM does a lot of gymnastics to get around old CGI incompatibilities with 
> mod_fastcgi, et al. It was using our accidental "proxy:fcgi//" prefix as a 
> marker to say "this is a new Apache server with mod_proxy_fcgi; don't do some 
> of the weird fixups". Without the marker, in specific cases PHP-FPM will 
> assume it is talking to an older incompatible FCGI module and "fix" things 
> incorrectly.
> 
> I looked to see if there was a way we could keep the current behavior and 
> trick current PHP-FPM deployments into not enabling their "quirks mode" 
> fixups. I don't see any, not without breaking other valid use cases. For now, 
> I've filed a Showstopper against 2.4.x with the intention of reverting the 
> change entirely for 2.4.26.
> 
> == Moving Forward ==
> 
> I think the best way to make both us and our users happy is to talk to the 
> PHP devs and fix both sides at once. To fix our side, I think we have to do 
> two things:
> 
> 1) Fix the CGI 1.1 incompatibilities in all of our first-party FCGI modules 
> so backends don't have to perform any fixup.
> 
> Those quirks have their own (in)famous bug:
> 
>https://bz.apache.org/bugzilla/show_bug.cgi?id=51517
> 
> in which the reporter melts down after three years of inaction.
> 
> The difficulty here is that there are a bunch of supported ways to invoke 
> FCGI (Action, SetHandler, ProxyPass, RewriteRule), all of which seem to 
> define CGI variables to different values in different situations.
> 
> 2) Define what SCRIPT_FILENAME means.
> 
> SCRIPT_FILENAME isn't actually a CGI 1.1 standard variable. We appear to have 
> defined it as "whatever r->filename contains", so we've effectively coupled 
> our implementation details to external clients. PHP-FPM and fcgiwrap, for 
> example, assume that SCRIPT_FILENAME should point to the script that should 
> be executed to handle the request. We need to standardize it.
> 
> These fixes probably can't be enabled by default until we go through a 
> compatibility version bump. But I've started chatting with people on the PHP 
> side [4] and we can hopefully fix these sorts of things once and for all.
> 
> --Jacob
> 
> [1] https://bz.apache.org/bugzilla/show_bug.cgi?id=59618
> [2] 
> https://lists.apache.org/thread.html/87fd8d1dc208b7d74fadf7b3929e71cfd7279c5c5b9b2566386cb684@%3Cdev.httpd.apache.org%3E
> [3] https://github.com/apache/httpd/commit/cab0bfbb#commitcomment-20393588
> [4] https://github.com/php/php-src/pull/694#issuecomment-271963956

Summary: Broken FastCGI with httpd

[Jacob Champion]

(This conversation is currently spread over Bugzilla, IRC, GitHub, and 
php-internals. Here's my attempt at summarizing it for all of you. If 
you have no interest in CGI or FastCGI, stop reading now.)


== Backstory ==

Back in May I was testing some FCGI backends with mod_proxy_fcgi, and I 
found a backend called fcgiwrap that didn't work. That conversation is 
in [1]. SCRIPT_FILENAME was being set to a value beginning with 
"proxy:fcgi://host:port", and that didn't make any sense. We patched the 
problem in 2.4.21 by stripping the weird prefix and called it good.


Fast forward to find that in *some* cases, SCRIPT_FILENAME could still 
contain weird values that didn't make any sense (in this case, a query 
string), because proxy canonicalization doesn't run correctly in 
per-directory rewrites. The conversation is in [2]. We stripped the 
query string and called it good.


Fast forward to find that in *some* cases involving mod_rewrite, PHP-FPM 
now refuses to operate correctly with mod_proxy_fcgi. That conversation 
is in [3]. There is no patch, and we can't call this good anymore...


== Problem ==

PHP-FPM does a lot of gymnastics to get around old CGI incompatibilities 
with mod_fastcgi, et al. It was using our accidental "proxy:fcgi//" 
prefix as a marker to say "this is a new Apache server with 
mod_proxy_fcgi; don't do some of the weird fixups". Without the marker, 
in specific cases PHP-FPM will assume it is talking to an older 
incompatible FCGI module and "fix" things incorrectly.


I looked to see if there was a way we could keep the current behavior 
and trick current PHP-FPM deployments into not enabling their "quirks 
mode" fixups. I don't see any, not without breaking other valid use 
cases. For now, I've filed a Showstopper against 2.4.x with the 
intention of reverting the change entirely for 2.4.26.


== Moving Forward ==

I think the best way to make both us and our users happy is to talk to 
the PHP devs and fix both sides at once. To fix our side, I think we 
have to do two things:


1) Fix the CGI 1.1 incompatibilities in all of our first-party FCGI 
modules so backends don't have to perform any fixup.


Those quirks have their own (in)famous bug:

https://bz.apache.org/bugzilla/show_bug.cgi?id=51517

in which the reporter melts down after three years of inaction.

The difficulty here is that there are a bunch of supported ways to 
invoke FCGI (Action, SetHandler, ProxyPass, RewriteRule), all of which 
seem to define CGI variables to different values in different situations.


2) Define what SCRIPT_FILENAME means.

SCRIPT_FILENAME isn't actually a CGI 1.1 standard variable. We appear to 
have defined it as "whatever r->filename contains", so we've effectively 
coupled our implementation details to external clients. PHP-FPM and 
fcgiwrap, for example, assume that SCRIPT_FILENAME should point to the 
script that should be executed to handle the request. We need to 
standardize it.


These fixes probably can't be enabled by default until we go through a 
compatibility version bump. But I've started chatting with people on the 
PHP side [4] and we can hopefully fix these sorts of things once and for 
all.


--Jacob

[1] https://bz.apache.org/bugzilla/show_bug.cgi?id=59618
[2] 
https://lists.apache.org/thread.html/87fd8d1dc208b7d74fadf7b3929e71cfd7279c5c5b9b2566386cb684@%3Cdev.httpd.apache.org%3E

[3] https://github.com/apache/httpd/commit/cab0bfbb#commitcomment-20393588
[4] https://github.com/php/php-src/pull/694#issuecomment-271963956