You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jim Riggs <ap...@riggs.me> on 2011/09/19 14:37:51 UTC

mod_proxy_fcgi + mod_proxy_balancer vs. php-fpm and query strings

I am having a couple of problems WRT using mod_proxy_fcgi inside a balancer proxied to php-fpm. There are lots of variables in this scenario, but I think I have narrowed the issues down.  The setup looks like this:

httpd -> balancer -> fcgi balancer members -> php-fpm

Issue 1: PHP-FPM does not handle the "proxy:balancer" prefix in SCRIPT_FILENAME. It does handle "proxy:fcgi" as a special case (see https://bugs.php.net/bug.php?id=54152 fix by jim). So, it seems we need to also add a "proxy:balancer" exception there unless a balanced mod_proxy_fcgi member should actually be using "proxy:fcgi" instead. What are people's thoughts on the prefix that should be sent by httpd in this case? To address this for now, I have modified PHP (fpm_main.c alongside jim's existing changes).

Issue 2: Once I got Issue 1 addressed, everything started working except in the case of a query string. I spent considerable time tracing and trying to figure out where the issue is occurring, but I am hoping one of you who is much more familiar with the code than I will be able to say, "Oh, look right here." The problem is that the query string is getting appended to SCRIPT_FILENAME if proxied through a balancer. FPM does not like this. It does not seem to happen in the case of proxying directly to "fcgi://...", but once I change this to "balancer://...", the query string gets added to SCRIPT_FILENAME. I believe this happened with both ProxyPass* and mod_rewrite [P]. In mod_rewrite, this should get handled in splitout_queryargs(), but somehow it is getting added back (probably in proxy_balancer_canon() which adds the query string back to r->filename?). For right now, I have done a brute-force "fix" for this by adding the code below to the beginning of send_environment() in mod_proxy_fcgi.c, before the calls to ap_add_common_vars() and ap_add_cgi_vars(). I am guessing that this isn't the ultimate fix for this issue, so I am interested in others' thoughts.

+    /* Remove query string from r->filename (r->args is already set and passed via QUERY_STRING) */
+    q = ap_strchr_c(r->filename, '?');
+    if (q != NULL) {
+        *q = '\0';
+    }


Re: mod_proxy_fcgi + mod_proxy_balancer vs. php-fpm and query strings

Posted by Jim Jagielski <ji...@jaguNET.com>.
On Sep 19, 2011, at 12:54 PM, Jim Jagielski wrote:

> I'll look at all this when I have some time in a few days…
> 
>> 
>> This sounds like it is related to https://issues.apache.org/bugzilla/show_bug.cgi?id=51077 as well.  Probably a new patch is needed to consistently and properly fix all of the cases (regular, mod_proxy_{f,s}cgi, mod_proxy_{f,s}cgi + balancer).
>> 

In the BUGZ attachment:

-            if (r->args != NULL) {
+	    if ((r->args != NULL)
+                && ((r->proxyreq == PROXYREQ_PROXY)
+                    || (rulestatus == ACTION_NOESCAPE))) {
+                /* see proxy_http:proxy_http_canon() */

breaks tests...


Re: mod_proxy_fcgi + mod_proxy_balancer vs. php-fpm and query strings

Posted by Jim Jagielski <ji...@jaguNET.com>.
I'll look at all this when I have some time in a few days…

On Sep 19, 2011, at 10:32 AM, Mark Montague wrote:

> On September 19, 2011 8:37 , Jim Riggs <ap...@riggs.me> wrote:
>> httpd ->  balancer ->  fcgi balancer members ->  php-fpm
>> 
>> Issue 1: PHP-FPM does not handle the "proxy:balancer" prefix in SCRIPT_FILENAME. It does handle "proxy:fcgi" as a special case (see https://bugs.php.net/bug.php?id=54152 fix by jim). So, it seems we need to also add a "proxy:balancer" exception there unless a balanced mod_proxy_fcgi member should actually be using "proxy:fcgi" instead. What are people's thoughts on the prefix that should be sent by httpd in this case? To address this for now, I have modified PHP (fpm_main.c alongside jim's existing changes).
> 
> As the person who wrote the changes that Jim later modified and committed, this seems reasonable to me, assuming it is correct (I say "assuming" only because I have never used mod_proxy_fcgi in a balancer configuration).
> 
> 
>> Issue 2: Once I got Issue 1 addressed, everything started working except in the case of a query string. I spent considerable time tracing and trying to figure out where the issue is occurring, but I am hoping one of you who is much more familiar with the code than I will be able to say, "Oh, look right here." The problem is that the query string is getting appended to SCRIPT_FILENAME if proxied through a balancer. FPM does not like this. It does not seem to happen in the case of proxying directly to "fcgi://...", but once I change this to "balancer://...", the query string gets added to SCRIPT_FILENAME. I believe this happened with both ProxyPass* and mod_rewrite [P]. In mod_rewrite, this should get handled in splitout_queryargs(), but somehow it is getting added back (probably in proxy_balancer_canon() which adds the query string back to r->filename?). For right now, I have done a brute-force "fix" for this by adding the code below to the beginning of send_environment() in mod_proxy_fcgi.c, before the calls to ap_add_common_vars() and ap_add_cgi_vars(). I am guessing that this isn't the ultimate fix for this issue, so I am interested in others' thoughts.
>> 
>> +    /* Remove query string from r->filename (r->args is already set and passed via QUERY_STRING) */
>> +    q = ap_strchr_c(r->filename, '?');
>> +    if (q != NULL) {
>> +        *q = '\0';
>> +    }
>> 
> 
> This sounds like it is related to https://issues.apache.org/bugzilla/show_bug.cgi?id=51077 as well.  Probably a new patch is needed to consistently and properly fix all of the cases (regular, mod_proxy_{f,s}cgi, mod_proxy_{f,s}cgi + balancer).
> 
> --
>  Mark Montague
>  mark@catseye.org
> 
> 


Re: mod_proxy_fcgi + mod_proxy_balancer vs. php-fpm and query strings

Posted by Mark Montague <ma...@catseye.org>.
On September 19, 2011 8:37 , Jim Riggs <ap...@riggs.me> wrote:
> httpd ->  balancer ->  fcgi balancer members ->  php-fpm
>
> Issue 1: PHP-FPM does not handle the "proxy:balancer" prefix in SCRIPT_FILENAME. It does handle "proxy:fcgi" as a special case (see https://bugs.php.net/bug.php?id=54152 fix by jim). So, it seems we need to also add a "proxy:balancer" exception there unless a balanced mod_proxy_fcgi member should actually be using "proxy:fcgi" instead. What are people's thoughts on the prefix that should be sent by httpd in this case? To address this for now, I have modified PHP (fpm_main.c alongside jim's existing changes).

As the person who wrote the changes that Jim later modified and 
committed, this seems reasonable to me, assuming it is correct (I say 
"assuming" only because I have never used mod_proxy_fcgi in a balancer 
configuration).


> Issue 2: Once I got Issue 1 addressed, everything started working except in the case of a query string. I spent considerable time tracing and trying to figure out where the issue is occurring, but I am hoping one of you who is much more familiar with the code than I will be able to say, "Oh, look right here." The problem is that the query string is getting appended to SCRIPT_FILENAME if proxied through a balancer. FPM does not like this. It does not seem to happen in the case of proxying directly to "fcgi://...", but once I change this to "balancer://...", the query string gets added to SCRIPT_FILENAME. I believe this happened with both ProxyPass* and mod_rewrite [P]. In mod_rewrite, this should get handled in splitout_queryargs(), but somehow it is getting added back (probably in proxy_balancer_canon() which adds the query string back to r->filename?). For right now, I have done a brute-force "fix" for this by adding the code below to the beginning of send_environment() in mod_proxy_fcgi.c, before the calls to ap_add_common_vars() and ap_add_cgi_vars(). I am guessing that this isn't the ultimate fix for this issue, so I am interested in others' thoughts.
>
> +    /* Remove query string from r->filename (r->args is already set and passed via QUERY_STRING) */
> +    q = ap_strchr_c(r->filename, '?');
> +    if (q != NULL) {
> +        *q = '\0';
> +    }
>

This sounds like it is related to 
https://issues.apache.org/bugzilla/show_bug.cgi?id=51077 as well.  
Probably a new patch is needed to consistently and properly fix all of 
the cases (regular, mod_proxy_{f,s}cgi, mod_proxy_{f,s}cgi + balancer).

--
   Mark Montague
   mark@catseye.org