Feature #2903
closedgw backend redesign
Description
I'm not happy about the API; mod_proxy casts a pointer to its own plugin_data as gw_plugin_data to gw_check_extension which happens to work because in both structs there is a just one pointer between PLUGIN_DATA and the conf member, and the conf member happens to be or start with gw_plugin_config; this is WAY to complicated and fragile, and I wouldn't be surprised if a compiler handles this mess as undefined behavior (the PLUGIN_DATA magic is bad enough, to reason to make it worse).
mod_proxy (and mod_wstunnel too, and maybe others) have config options that are not part of gw_plugin_config; mod_wstunnel seems to copy them manually (one by one) into the handler context, but mod_proxy didn't (which is why the "Forwarded" stuff isn't working AT ALL, see #2902). Again very fragile.
The traditional way was that the handler_ctx had a pointer to plugin_data; for gw_backend this would need to be a void* obviously, as we don't know the plugin. Options that may be needed later (i.e. after some other connection might have been "patched" into conf) the config need to be copied of course; but it should be copied as a complete struct instead of copying individual members.
Updated by stbuehler over 6 years ago
- Related to Bug #2902: mod_proxy's “proxy.forwarded” option seems ignored when used with mod_auth. added
Updated by gstrauss over 6 years ago
The idea is that the gw_plugin_data is like a base class.
As you mentioned, the reason things need to be copied into the handler context is that, for async modules (such as mod_proxy) which do not immediately process and finish handling a request, the p->conf must be copied since p->conf is reused and might change for the next request, which may be running in parallel as lighttpd is communicating with the backend.
In #2902, I overlooked copying a couple members into the handler context when I converted things to the shared backend infrastructure (gw_backend).
Yes, it might be slightly cleaner to copy the whole p->conf into the handler context, but I don't think it that sloppy to simply copy the needed config params as the handler context is set up. Not all of the config processing intermediates are needed in mod_proxy. Since it is not frequent that new members are added, and now that modules have been converted, the new member will not be present in a modules such as mod_proxy until it is copied into the handler context.
Updated by gstrauss over 6 years ago
- Tracker changed from Bug to Feature
- Category set to mod_proxy
- Status changed from New to Patch Pending
- Priority changed from Normal to Low
- Target version changed from 1.4.x to 1.4.51
The gw_handler_ctx structure is required to be the first member of a module's handler_ctx if the module has additional config members. Likewise for gw_plugin_config being first member of plugin_config. If the module does not have additional config members, then the module handler_ctx is gw_handler_ctx and the plugin_config is gw_plugin_config.
For persistence during the lifetime of the request to the backend, handler_ctx contains plugin_config (and, again, gw_plugin_config is part of plugin_config). The gw_plugin_config portion is constructed inside gw_backend.c in gw_check_extension(). A full additional copy of plugin_config from p->conf into hctx->conf would result in some unnecessary copies and would overwrite any changes made in gw_check_extension() beyond straight copy. (Currently there are no changes besides copying a portion of the config in gw_check_extension().)
I'll change mod_proxy and mod_wstunnel to copy the entire plugin_config from p->conf to hctx->conf to keep the code simpler, even though it results in some extra copying.
Updated by gstrauss about 6 years ago
- Status changed from Patch Pending to Fixed
- % Done changed from 0 to 100
Applied in changeset a458c2e7317d9edbfcdf3a2b55010ecb0ce52a83.
Also available in: Atom