On Wed, Jun 21, 2017 at 2:00 PM, Jacob Champion <champio...@gmail.com> wrote:
> On 06/20/2017 02:19 PM, Jacob Champion wrote:
>>
>> Here's why I'm asking: if I were to propose the attached patch for
>> backport, what is the test case that *should* fail but doesn't?
>> (proxy_fcgi.t passes, no problem.) And once we get that test case, can we
>> show that it actually addresses a valid PHP-FPM use case, or is it a feature
>> without a user?
>
>
> Any thoughts on this?
I still cannot make any sense of what FPM is doing with our input.
I don't think either of the SCRIPT_NAME transformations make sense
just by the virtue of talking to FPM, primarily because it is neither
historically necessary or demonstrated as a problem.
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);
}
I would just as well pull this block out entirely rather than taking
the "fpm||" half of the test out. It seems like if you go out of your
way to run a script with PATH_INFO set as some parameter that we
shouldn't negate that. And like the non path_info case, nobody has
ever asked us for this.
