Project

General

Profile

Feature #2903

gw backend redesign

Added by stbuehler 3 months ago. Updated about 2 months ago.

Status:
Fixed
Priority:
Low
Assignee:
-
Category:
mod_proxy
Target version:
Start date:
2018-08-18
Due date:
% Done:

100%

Estimated time:
Missing in 1.5.x:

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.


Related issues

Related to Bug #2902: mod_proxy's “proxy.forwarded” option seems ignored when used with mod_auth.Fixed2018-08-18

Actions

Associated revisions

Revision a458c2e7 (diff)
Added by gstrauss about 2 months ago

[mod_proxy,mod_wstunnel] copy full plugin_config (fixes #2903)

x-ref:
"gw backend redesign"
https://redmine.lighttpd.net/issues/2903

History

#1

Updated by stbuehler 3 months ago

  • Related to Bug #2902: mod_proxy's “proxy.forwarded” option seems ignored when used with mod_auth. added
#2

Updated by gstrauss 3 months 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.

#3

Updated by gstrauss about 2 months 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.

#4

Updated by gstrauss about 2 months ago

  • Status changed from Patch Pending to Fixed
  • % Done changed from 0 to 100

Also available in: Atom