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: mod_http2 and canceled requests logs status 500 internal server error code

[Stefan Eissing]

Hi, will do that tomorrow.

> Am 26.01.2017 um 16:35 schrieb Stefan Priebe - Profihost AG 
> :
> 
> Hi Stefan,
> 
> did you already had the time to look at the 500 status code in case of
> canceled requests?
> 
> Stefan
> 
> Am 25.01.2017 um 15:55 schrieb Stefan Priebe - Profihost AG:
>> 
>> Am 25.01.2017 um 15:50 schrieb Stefan Eissing:
>>> 
 Am 25.01.2017 um 15:31 schrieb Stefan Priebe - Profihost AG 
 :
 
 Chrome docs says:
 
 "cancelled / net::ERR_ABORTED is intended to only be generated when a
 user action causes a load to be interrupted. This can happen when a new
 navigation interrupts an existing one, or when the user clicks the STOP
 button"
 
 This matches my click assumption. I think it's just wrong that mod_http2
 generates a 500 in the logs.
>>> 
>>> I agree. Might be a new one related to the 304 fix in 1.8.10.
>> 
>> Would be great if you can have a look. It confuses the users while
>> reading their logs.
>> 
>> I also saw some more entries with 0 byte size in logs if there is no
>> body. http/1.1 logs the header size as well.
>> 
>> Greets,
>> Stefan
>> 
 
 Stefan
 
 Am 25.01.2017 um 15:27 schrieb Stefan Priebe - Profihost AG:
> Am 25.01.2017 um 14:57 schrieb Yann Ylavic:
>> Hi,
>> 
>> On Wed, Jan 25, 2017 at 2:46 PM, Stefan Priebe - Profihost AG
>>  wrote:
>>> 
>>> Is this a bug or a feature? Is it correct that it logs a 500 code?
>> 
>> ErrorLog's file should tell more about the error.
> 
> No error log does not have an entry with the same or nearly the same
> timestamp.
> 
> Stefan
> 
>> 
>> Regards,
>> Yann.
>> 
>>> 
>>> Stefan Eissing
>>> 
>>> bytes GmbH
>>> Hafenstrasse 16
>>> 48155 Münster
>>> www.greenbytes.de
>>> 

Stefan Eissing

bytes GmbH
Hafenstrasse 16
48155 Münster
www.greenbytes.de

Re: mod_http2 and canceled requests logs status 500 internal server error code

[Stefan Priebe - Profihost AG]

Hi Stefan,

did you already had the time to look at the 500 status code in case of
canceled requests?

Stefan

Am 25.01.2017 um 15:55 schrieb Stefan Priebe - Profihost AG:
> 
> Am 25.01.2017 um 15:50 schrieb Stefan Eissing:
>>
>>> Am 25.01.2017 um 15:31 schrieb Stefan Priebe - Profihost AG 
>>> :
>>>
>>> Chrome docs says:
>>>
>>> "cancelled / net::ERR_ABORTED is intended to only be generated when a
>>> user action causes a load to be interrupted. This can happen when a new
>>> navigation interrupts an existing one, or when the user clicks the STOP
>>> button"
>>>
>>> This matches my click assumption. I think it's just wrong that mod_http2
>>> generates a 500 in the logs.
>>
>> I agree. Might be a new one related to the 304 fix in 1.8.10.
> 
> Would be great if you can have a look. It confuses the users while
> reading their logs.
> 
> I also saw some more entries with 0 byte size in logs if there is no
> body. http/1.1 logs the header size as well.
> 
> Greets,
> Stefan
> 
>>>
>>> Stefan
>>>
>>> Am 25.01.2017 um 15:27 schrieb Stefan Priebe - Profihost AG:
 Am 25.01.2017 um 14:57 schrieb Yann Ylavic:
> Hi,
>
> On Wed, Jan 25, 2017 at 2:46 PM, Stefan Priebe - Profihost AG
>  wrote:
>>
>> Is this a bug or a feature? Is it correct that it logs a 500 code?
>
> ErrorLog's file should tell more about the error.

 No error log does not have an entry with the same or nearly the same
 timestamp.

 Stefan

>
> Regards,
> Yann.
>
>>
>> Stefan Eissing
>>
>> bytes GmbH
>> Hafenstrasse 16
>> 48155 Münster
>> www.greenbytes.de
>>

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