You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by jc...@apache.org on 2017/06/29 17:43:48 UTC
svn commit: r1800306 - in /httpd/httpd/trunk: CHANGES
modules/mappers/mod_actions.c modules/proxy/mod_proxy_fcgi.c
Author: jchampion
Date: Thu Jun 29 17:43:48 2017
New Revision: 1800306
URL: http://svn.apache.org/viewvc?rev=1800306&view=rev
Log:
proxy_fcgi: remove FPM-specific logic
Reverts r1780328, r1780329, and their associated followups, which
incorrectly manipulated SCRIPT_NAME by default. All proxy_fcgi.t
regression tests now pass.
PR: 61202
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/modules/mappers/mod_actions.c
httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1800306&r1=1800305&r2=1800306&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Thu Jun 29 17:43:48 2017
@@ -1,6 +1,10 @@
-*- coding: utf-8 -*-
Changes with Apache 2.5.0
+ *) mod_proxy_fcgi: Revert to 2.4.20 FCGI behavior for the default
+ ProxyFCGIBackendType, fixing a regression with PHP-FPM. PR 61202.
+ [Jacob Champion]
+
*) core: Avoid duplicate HEAD in Allow header.
This is a regression in 2.4.24 (unreleased), 2.4.25 and 2.4.26.
PR 61207. [Christophe Jaillet]
Modified: httpd/httpd/trunk/modules/mappers/mod_actions.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_actions.c?rev=1800306&r1=1800305&r2=1800306&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/mappers/mod_actions.c (original)
+++ httpd/httpd/trunk/modules/mappers/mod_actions.c Thu Jun 29 17:43:48 2017
@@ -186,8 +186,7 @@ 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))) {
- int virtual = (*t++ == '0' ? 0 : 1);
- if (!virtual && r->finfo.filetype == APR_NOFILE) {
+ if (*t++ == '0' && 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;
@@ -198,9 +197,6 @@ 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->notes, "virtual_script", "1");
- }
}
if (script == NULL)
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c?rev=1800306&r1=1800305&r2=1800306&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c Thu Jun 29 17:43:48 2017
@@ -321,7 +321,6 @@ static apr_status_t send_environment(pro
apr_status_t rv;
apr_size_t avail_len, len, required_len;
int next_elem, starting_elem;
- int fpm = 0;
fcgi_req_config_t *rconf = ap_get_module_config(r->request_config, &proxy_fcgi_module);
fcgi_dirconf_t *dconf = ap_get_module_config(r->per_dir_config, &proxy_fcgi_module);
@@ -354,8 +353,6 @@ static apr_status_t send_environment(pro
*qs = '\0';
}
}
- } else {
- fpm = 1;
}
if (newfname) {
@@ -364,38 +361,9 @@ static apr_status_t send_environment(pro
}
}
-#if 0
- ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
- "r->filename: %s", (r->filename ? r->filename : "nil"));
- ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
- "r->uri: %s", (r->uri ? r->uri : "nil"));
- ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
- "r->path_info: %s", (r->path_info ? r->path_info : "nil"));
-#endif
-
ap_add_common_vars(r);
ap_add_cgi_vars(r);
- if (fpm || apr_table_get(r->notes, "virtual_script")) {
- /*
- * Adjust SCRIPT_NAME, PATH_INFO and PATH_TRANSLATED for PHP-FPM
- * TODO: Right now, PATH_INFO and PATH_TRANSLATED look OK...
- */
- const char *pend;
- const char *script_name = apr_table_get(r->subprocess_env, "SCRIPT_NAME");
- pend = script_name + strlen(script_name);
- if (r->path_info && *r->path_info) {
- pend = script_name + ap_find_path_info(script_name, r->path_info) - 1;
- }
- while (pend != script_name && *pend != '/') {
- pend--;
- }
- apr_table_setn(r->subprocess_env, "SCRIPT_NAME", pend);
- ap_log_rerror(APLOG_MARK, APLOG_TRACE4, 0, r,
- "fpm:virtual_script: Modified SCRIPT_NAME to: %s",
- pend);
- }
-
/* XXX are there any FastCGI specific env vars we need to send? */
/* Give admins final option to fine-tune env vars */
Re: svn commit: r1800306 - in /httpd/httpd/trunk: CHANGES
modules/mappers/mod_actions.c modules/proxy/mod_proxy_fcgi.c
Posted by Eric Covener <co...@gmail.com>.
On Fri, Jun 30, 2017 at 8:32 AM, Jim Jagielski <ji...@jagunet.com> wrote:
> I still think that the below has value and should not be/have-been
> reverted.
>
> Anyone opposed to me re-adding it to trunk and removing it
> from the backport proposal?
Would an fcgi query it and look at PATH_INFO/TRANSLATED instead or
e.g. SCRIPT_NAME/TRANSLATED?
--
Eric Covener
covener@gmail.com
Re: svn commit: r1800306 - in /httpd/httpd/trunk: CHANGES
modules/mappers/mod_actions.c modules/proxy/mod_proxy_fcgi.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
You already have my unconditional +1 to bring us back to a trusted state.
On Jun 30, 2017 16:55, "Jacob Champion" <ch...@gmail.com> wrote:
> On 06/30/2017 11:40 AM, Jacob Champion wrote:
>
>> As far as I can tell it has no downsides, so my only request is that we
>> add it to CHANGES (or some documentation, somewhere) and get a test in
>> place before it goes back in. I may be able to get to that later this
>> afternoon.
>>
>
> This is taking me longer than I wanted, so in the interest of time, I've
> just gone ahead and added the un-revert to the backport proposal. I hope to
> get to the tests for it next week.
>
> Regression tests still pass for me; vote away!
>
> --Jacob
>
Re: svn commit: r1800306 - in /httpd/httpd/trunk: CHANGES
modules/mappers/mod_actions.c modules/proxy/mod_proxy_fcgi.c
Posted by Jacob Champion <ch...@gmail.com>.
On 06/30/2017 11:40 AM, Jacob Champion wrote:
> As far as I can tell it has no downsides, so my only request is that we
> add it to CHANGES (or some documentation, somewhere) and get a test in
> place before it goes back in. I may be able to get to that later this
> afternoon.
This is taking me longer than I wanted, so in the interest of time, I've
just gone ahead and added the un-revert to the backport proposal. I hope
to get to the tests for it next week.
Regression tests still pass for me; vote away!
--Jacob
Re: svn commit: r1800306 - in /httpd/httpd/trunk: CHANGES
modules/mappers/mod_actions.c modules/proxy/mod_proxy_fcgi.c
Posted by Jacob Champion <ch...@gmail.com>.
On 06/30/2017 08:41 AM, Jacob Champion wrote:
> On 06/30/2017 08:37 AM, Jim Jagielski wrote:
>> Well, in 2.4.26 is WAS/IS an entry in notes available to modules
>
> Well... hm. I guess that's a valid point. My preference is still to
> remove it since it's undocumented, but if anyone else would like to see
> it back in, I'm fine with that.
After mulling it over a bit... I think you've convinced me, Jim. I would
probably die of shame if someone started using that note in 2.4.26 and
my "final patch" just extended the regression train. ;P
As far as I can tell it has no downsides, so my only request is that we
add it to CHANGES (or some documentation, somewhere) and get a test in
place before it goes back in. I may be able to get to that later this
afternoon.
--Jacob
Re: svn commit: r1800306 - in /httpd/httpd/trunk: CHANGES
modules/mappers/mod_actions.c modules/proxy/mod_proxy_fcgi.c
Posted by Jacob Champion <ch...@gmail.com>.
On 06/30/2017 08:37 AM, Jim Jagielski wrote:
> Well, in 2.4.26 is WAS/IS an entry in notes available to modules
Well... hm. I guess that's a valid point. My preference is still to
remove it since it's undocumented, but if anyone else would like to see
it back in, I'm fine with that.
Other opinions?
--Jacob
Re: svn commit: r1800306 - in /httpd/httpd/trunk: CHANGES
modules/mappers/mod_actions.c modules/proxy/mod_proxy_fcgi.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
Well, in 2.4.26 is WAS/IS an entry in notes available to modules,
and since we don't know who/what may not being using or expecting
it, and since it's useful info anyway and not a performance hit,
it seems "prudent" to me. But I'm fine either way.
> On Jun 30, 2017, at 11:28 AM, Jacob Champion <ch...@gmail.com> wrote:
>
> On 06/30/2017 05:32 AM, Jim Jagielski wrote:
>> I still think that the below has value and should not be/have-been
>> reverted.
>
> I'm not arguing that it doesn't have value in theory, but IMO it doesn't belong in 2.4.x without a client. Right now it's just dead code.
>
>> Anyone opposed to me re-adding it to trunk and removing it
>> from the backport proposal?
>
> Yes. Unless you have a use case for it at this moment, I'd prefer that it be re-backported if and when that use case materializes, but I will defer to the majority here.
>
> --Jacob
Re: svn commit: r1800306 - in /httpd/httpd/trunk: CHANGES
modules/mappers/mod_actions.c modules/proxy/mod_proxy_fcgi.c
Posted by Jacob Champion <ch...@gmail.com>.
On 06/30/2017 05:32 AM, Jim Jagielski wrote:
> I still think that the below has value and should not be/have-been
> reverted.
I'm not arguing that it doesn't have value in theory, but IMO it doesn't
belong in 2.4.x without a client. Right now it's just dead code.
> Anyone opposed to me re-adding it to trunk and removing it
> from the backport proposal?
Yes. Unless you have a use case for it at this moment, I'd prefer that
it be re-backported if and when that use case materializes, but I will
defer to the majority here.
--Jacob
Re: svn commit: r1800306 - in /httpd/httpd/trunk: CHANGES
modules/mappers/mod_actions.c modules/proxy/mod_proxy_fcgi.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
I still think that the below has value and should not be/have-been
reverted.
Anyone opposed to me re-adding it to trunk and removing it
from the backport proposal?
> On Jun 29, 2017, at 1:43 PM, jchampion@apache.org wrote:
>
> Modified: httpd/httpd/trunk/modules/mappers/mod_actions.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_actions.c?rev=1800306&r1=1800305&r2=1800306&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/mappers/mod_actions.c (original)
> +++ httpd/httpd/trunk/modules/mappers/mod_actions.c Thu Jun 29 17:43:48 2017
> @@ -186,8 +186,7 @@ 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))) {
> - int virtual = (*t++ == '0' ? 0 : 1);
> - if (!virtual && r->finfo.filetype == APR_NOFILE) {
> + if (*t++ == '0' && 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;
> @@ -198,9 +197,6 @@ 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->notes, "virtual_script", "1");
> - }
> }
>
> if (script == NULL)
>