On 9/21/23 3:15 PM, yla...@apache.org wrote:
> Author: ylavic
> Date: Thu Sep 21 13:15:35 2023
> New Revision: 1912459
>
> URL: http://svn.apache.org/viewvc?rev=1912459&view=rev
> Log:
> mod_proxy: Handle backend address renewal with address_ttl= parameter.
>
> Define a new proxy_address struct holding the current/latest sockaddr in use
> by each proxy worker and conn. Since backend addresses can be updated when
> their TTL expires and while connections are being processed, each address is
> refcounted and freed only when the last worker (or conn) using it grabs the
> new one.
>
> The lifetime of the addresses is handled at a single place by the new
> ap_proxy_determine_address() function. It guarantees to bind the
> current/latest
> backend address to the passed in conn (or do nothing if it's up to date
> already).
> The function is called indirectly by ap_proxy_determine_connection() for the
> proxy modules that use it, or directly by mod_proxy_ftp and mod_proxy_hcheck.
> It also is called eventually by ap_proxy_connect_backend() when connect()ing
> all
> the current addresses fails, to check (PROXY_DETERMINE_ADDRESS_CHECK) if some
> new addrs are available.
>
> This commit is also a rework of the lifetime of conn->addr, conn->hostname
> and conn->forward, using the conn->uds_pool and conn->fwd_pool for the cases
> where the backend is connected through a UDS socket and a remote CONNECT proxy
> respectively.
>
> * include/ap_mmn.h:
> Minor bump for new function/fields.
>
> * modules/proxy/mod_proxy.h (struct proxy_address,
> ap_proxy_determine_addresss()):
> Declare ap_proxy_determine_addresss() and opaque struct proxy_address,
> new fields to structs proxy_conn_rec/proxy_worker_shared/proxy_worker.
>
> * modules/proxy/mod_proxy.c (set_worker_param):
> Parse/set the new worker->address_ttl parameter.
>
> * modules/proxy/proxy_util.c (proxy_util_register_hooks(),
> ap_proxy_initialize_worker(),
> ap_proxy_connection_reusable(),
> ap_proxyerror(), proxyerror_core(),
> init_conn_pool(), make_conn_subpool(),
> connection_make(), connection_cleanup(),
> connection_constructor()):
> Initialize *proxy_start_time in proxy_util_register_hooks() as the epoch
> from which expiration times are relative (i.e. seconds stored in an uint32_t
> for atomic changes).
> Make sure worker->s->is_address_reusable and worker->s->disablereuse are
> consistant in ap_proxy_initialize_worker(), thus no need to check for both
> in ap_proxy_connection_reusable().
> New proxyerror_core() helper taking an apr_status_t to log, wrap in
> ap_proxyerror().
> New make_conn_subpool() to create worker->cp->{pool,dns} with their own
> allocator.
> New connection_make() helper to factorize code in connection_cleanup() and
> connection_constructor().
>
> * modules/proxy/proxy_util.c (proxy_address_inc(), proxy_address_dec(),
> proxy_address_cleanup(),
> proxy_address_set_expired(),
> worker_address_get(), worker_address_set(),
> worker_address_resolve(), proxy_addrs_equal(),
> ap_proxy_determine_address(),
> ap_proxy_determine_connection(),
> ap_proxy_connect_backend()):
> Implement ap_proxy_determine_address() using the above helpers for atomic
> changes,
> and call it from ap_proxy_determine_connection() and
> ap_proxy_connect_backend().
>
> * modules/proxy/mod_proxy_ftp.c (proxy_ftp_handler):
> Use ap_proxy_determine_address() and use the returned backend->addr.
>
> * modules/proxy/mod_proxy_hcheck.c (hc_determine_connection, hc_get_backend,
> hc_init_worker, hc_watchdog_callback):
> Use ap_proxy_determine_address() in hc_determine_connection() and call the
> latter from hc_get_backend(), replace hc_init_worker() by hc_init_baton()
> which now calls hc_get_hcworker() and hc_get_backend() to resolve the first
> address at init time.
>
> * modules/proxy/mod_proxy_http.c (proxy_http_handler):
> Use backend->addr and ->hostname instead of worker->cp->addr and
> worker->s->hostname_ex respectively.
>
> * modules/proxy/mod_proxy_ajp.c (ap_proxy_ajp_request):
> Use backend->addr and ->hostname instead of worker->cp->addr and
> worker->s->hostname_ex respectively.
>
>
> Closes #367
>
>
> Modified:
> httpd/httpd/trunk/include/ap_mmn.h
> httpd/httpd/trunk/modules/proxy/mod_proxy.c
> httpd/httpd/trunk/modules/proxy/mod_proxy.h
> httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c
> httpd/httpd/trunk/modules/proxy/mod_proxy_ftp.c
> httpd/httpd/trunk/modules/proxy/mod_proxy_hcheck.c
> httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
> httpd/httpd/trunk/modules/proxy/proxy_util.c
>
> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1912459&r1=1912458&r2=1912459&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Thu Sep 21 13:15:35 2023
> @@ -2683,56 +3116,83 @@ ap_proxy_determine_connection(apr_pool_t
> * to check host and port on the conn and be careful about
> * spilling the cached addr from the worker.
> */
> - uds_path = (*worker->s->uds_path ? worker->s->uds_path :
> apr_table_get(r->notes, "uds_path"));
> + uds_path = (*worker->s->uds_path
> + ? worker->s->uds_path
> + : apr_table_get(r->notes, "uds_path"));
> if (uds_path) {
> - if (conn->uds_path == NULL) {
> - /* use (*conn)->pool instead of worker->cp->pool to match
> lifetime */
> - conn->uds_path = apr_pstrdup(conn->pool, uds_path);
> - }
> - if (conn->uds_path) {
> - ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02545)
> - "%s: has determined UDS as %s",
> - uri->scheme, conn->uds_path);
> - }
> - else {
> - /* should never happen */
> - ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02546)
> - "%s: cannot determine UDS (%s)",
> - uri->scheme, uds_path);
> -
> - }
> - /*
> - * In UDS cases, some structs are NULL. Protect from de-refs
> - * and provide info for logging at the same time.
> - */
> - if (!conn->addr) {
> - apr_sockaddr_t *sa;
> - apr_sockaddr_info_get(&sa, NULL, APR_UNSPEC, 0, 0, conn->pool);
> - conn->addr = sa;
> + if (!conn->uds_path || strcmp(conn->uds_path, uds_path) != 0) {
> + apr_pool_t *pool = conn->pool;
> + if (conn->uds_path) {
> + address_cleanup(conn);
> + if (!conn->uds_pool) {
> + apr_pool_create(&conn->uds_pool, worker->cp->dns_pool);
> + }
> + pool = conn->uds_pool;
> + }
> + /*
> + * In UDS cases, some structs are NULL. Protect from de-refs
> + * and provide info for logging at the same time.
> + */
> +#if APR_HAVE_SOCKADDR_UN
> + apr_sockaddr_info_get(&conn->addr, uds_path, APR_UNIX, 0, 0,
> pool);
> + if (conn->addr && conn->addr->hostname) {
> + conn->uds_path = conn->addr->hostname;
> + }
> + else {
> + conn->uds_path = apr_pstrdup(pool, uds_path);
> + }
> +#else
> + apr_sockaddr_info_get(&conn->addr, NULL, APR_UNSPEC, 0, 0, pool);
> + conn->uds_path = apr_pstrdup(pool, uds_path);
> +#endif
> + conn->hostname = apr_pstrdup(pool, uri->hostname);
> + conn->port = uri->port;
> }
> - conn->hostname = "httpd-UDS";
> - conn->port = 0;
> + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02545)
> + "%s: has determined UDS as %s (for %s:%hu)",
> + uri->scheme, conn->uds_path, conn->hostname,
> conn->port);
> }
> else {
> - int will_reuse = worker->s->is_address_reusable &&
> !worker->s->disablereuse;
> - if (!conn->hostname || !will_reuse) {
> - if (proxyname) {
> - conn->hostname = apr_pstrdup(conn->pool, proxyname);
> - conn->port = proxyport;
> + const char *hostname = uri->hostname;
> + apr_port_t hostport = uri->port;
> +
> + if (proxyname) {
> + forward_info *forward;
> +
> + hostname = proxyname;
> + hostport = proxyport;
> +
> + /* Reset forward info if they changed */
> + if (conn->is_ssl
> + && (!(forward = conn->forward)
> + || forward->target_port != uri->port
> + || ap_cstr_casecmp(forward->target_host,
> + uri->hostname) != 0)) {
> + apr_pool_t *fwd_pool = conn->pool;
> + if (worker->s->is_address_reusable) {
> + if (conn->fwd_pool) {
> + apr_pool_clear(conn->fwd_pool);
> + }
> + else {
> + apr_pool_create(&conn->fwd_pool, conn->pool);
> + }
Don't we need to
fwd_pool = conn->fwd_pool
??
> + }
> + forward = apr_pcalloc(fwd_pool, sizeof(forward_info));
> + conn->forward = forward;
> +
> /*
> - * If we have a forward proxy and the protocol is HTTPS,
> + * If we have a remote proxy and the protocol is HTTPS,
> * then we need to prepend a HTTP CONNECT request before
> * sending our actual HTTPS requests.
> * Save our real backend data for using it later during HTTP
> CONNECT.
> */
> - if (conn->is_ssl) {
> + {
> const char *proxy_auth;
>
> - forward_info *forward = apr_pcalloc(conn->pool,
> sizeof(forward_info));
> - conn->forward = forward;
> forward->use_http_connect = 1;
> - forward->target_host = apr_pstrdup(conn->pool,
> uri->hostname);
> + forward->target_host = apr_pstrdup(fwd_pool,
> uri->hostname);
> forward->target_port = uri->port;
> +
> /* Do we want to pass Proxy-Authorization along?
> * If we haven't used it, then YES
> * If we have used it then MAYBE: RFC2616 says we MAY
> propagate it.
Regards
Rüdiger
