On Fri, Aug 2, 2024 at 12:19 PM Yann Ylavic <[email protected]> wrote: > > On Fri, Aug 2, 2024 at 3:26 PM Eric Covener <[email protected]> wrote: > > > > On Fri, Aug 2, 2024 at 9:10 AM Yann Ylavic <[email protected]> wrote: > > > > > > On Fri, Aug 2, 2024 at 1:06 PM Eric Covener <[email protected]> wrote: > > > > > > > > > Yeah, if not under DocumentRoot I don't see how ProxyPass could work, > > > > > but SetHandler should since it's following the whole request > > > > > processing to resolve the filesystem r->filename? > > > > > > > > In the examples I am seeing spot-checking google results, people who > > > > use ProxyPass + FPM hard-code the document root (or wherever the stuff > > > > is) in the 2nd parameter. This includes our own manual and the > > > > unofficial confluence wiki. > > > > > > Ah ok, I think we can do something like this: > > > if (rconf->need_dirwalk) { > > > const char *docroot = ap_document_root(r); > > > if (strncmp(docroot, r->filename, strlen(docroot)) != 0) { > > > r->proxyreq = PROXYREQ_NONE; > > > ap_core_translate(r); > > > } > > > ap_directory_walk(r); > > > } > > > ? > > > > I think users could be happily humming along with some other absolute > > filesystem path in the ProxyPass 2nd arg and would now see it > > docroot-prefixed. > > Are you trying to make the ProxyPass+FPM vars better because they will > > no longer be fixed up by apache_was_here side effects? I think it is > > probably bes to just retain/restore some of the historical bogus > > values if MAY_BE_FPM -- maybe doing them at the last moment where we'd > > do the above. > > Maybe this: > if (rconf) { > if (!rconf->need_dirwalk) { > r->filename = rconf->filename; > } > else { > char *saved_uri = r->uri; > char *saved_path_info = r->path_info; > int i = 0; > > /* Try to find the script without DocumentRoot than with */ > do { > r->path_info = NULL; > r->filename = r->uri = rconf->filename; > if (i) { > r->proxyreq = PROXYREQ_NONE; > ap_core_translate(r); > r->proxyreq = PROXYREQ_REVERSE; > } > ap_directory_walk(r); > } while (r->finfo.filetype != APR_REG && ++i < 2); > > /* If no actual script was found, fall back to the "proxy:" > * SCRIPT_FILENAME dealt with below or by php-fpm directly. > */ > if (i == 2) { > r->path_info = saved_path_info; > r->filename = proxy_filename; > } > /* Restore REQUEST_URI in any case */ > r->uri = saved_uri; > } > } > ?
Looks good to me, curious what you are thinking for changes-entry or is this more for a semblance of sanity in this area of code if we have to come back to it? This seems to (at least): - fix slightly nonsensical combo of SetHandler + proxy-fcgi-pathinfo=full - allows ProxyPass + proxy-fcgi-pathinfo=full to not repeat the document root in the directive I don't see a ton of references to proxy-fcgi-pathinfo in the wild. It was kind of pre-emptive as I remember along with the ProxyFCGISetEnvIf. But one the few I found (on php.net) said "Don't use ProxyPassMatch. Use SetHandler." We could also consider deprecating the use of proxypass specifically with FPM (where the balancer scenario is kind of moot). -- Eric Covener [email protected]
