Skip to content

Commit

Permalink
Merge GH PR #454
Browse files Browse the repository at this point in the history 
  *) mod_proxy: Fix DNS requests and connections closed before the
     configured addressTTL.  BZ 69126.  [Yann Ylavic]



git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1918543 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
@icing
icing committed Jun 24, 2024
1 parent d276cfe commit c66fe1a
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 30 deletions.
2 changes: 2 additions & 0 deletions changes-entries/fix_proxy_determine_address.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
*) mod_proxy: Fix DNS requests and connections closed before the
configured addressTTL. BZ 69126. [Yann Ylavic]
94 changes: 64 additions & 30 deletions modules/proxy/proxy_util.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1512,8 +1512,9 @@ static void socket_cleanup(proxy_conn_rec *conn)
apr_pool_clear(conn->scpool);
}

static void address_cleanup(proxy_conn_rec *conn)
static void conn_cleanup(proxy_conn_rec *conn)
{
socket_cleanup(conn);
conn->address = NULL;
conn->addr = NULL;
conn->hostname = NULL;
Expand All @@ -1522,9 +1523,6 @@ static void address_cleanup(proxy_conn_rec *conn)
if (conn->uds_pool) {
apr_pool_clear(conn->uds_pool);
}
if (conn->sock) {
socket_cleanup(conn);
}
}

static apr_status_t conn_pool_cleanup(void *theworker)
Expand Down Expand Up @@ -2784,8 +2782,8 @@ static apr_status_t worker_address_resolve(proxy_worker *worker,
apr_sockaddr_t *addr = *paddr;
for (; addr; addr = addr->next) {
addrs = apr_psprintf(pool, "%s%s%pI",
addrs ? ", " : "",
addrs ? addrs : "",
addrs ? ", " : "",
addr);
}
if (r) {
Expand Down Expand Up @@ -2959,15 +2957,16 @@ PROXY_DECLARE(apr_status_t) ap_proxy_determine_address(const char *proxy_functio
*/
address->expiry = apr_atomic_read32(&worker->s->address_expiry);
if (address->expiry <= now) {
apr_uint32_t new_expiry = address->expiry + ttl;
while (new_expiry <= now) {
new_expiry += ttl;
}
new_expiry = apr_atomic_cas32(&worker->s->address_expiry,
new_expiry, address->expiry);
/* race lost? well the expiry should grow anyway.. */
AP_DEBUG_ASSERT(new_expiry > now);
address->expiry = new_expiry;
apr_uint32_t prev, next = (now + ttl) - (now % ttl);
do {
prev = apr_atomic_cas32(&worker->s->address_expiry,
next, address->expiry);
if (prev == address->expiry) {
address->expiry = next;
break;
}
address->expiry = prev;
} while (prev <= now);
}
}
else {
Expand Down Expand Up @@ -3008,13 +3007,40 @@ PROXY_DECLARE(apr_status_t) ap_proxy_determine_address(const char *proxy_functio

PROXY_THREAD_UNLOCK(worker);

/* Kill any socket using the old address */
if (conn->sock) {
if (r ? APLOGrdebug(r) : APLOGdebug(s)) {
/* XXX: this requires the old conn->addr[ess] to still
* be alive since it's not copied by apr_socket_connect()
* in ap_proxy_connect_backend().
*/
/* Release the old conn address */
if (conn->address) {
/* On Windows and OS/2, apr_socket_connect() called from
* ap_proxy_connect_backend() does a simple pointer copy of
* its given conn->addr[->next] into conn->sock->remote_addr.
* Thus conn->addr cannot be freed if the conn->sock should be
* kept alive (same new and old addresses) and the old address
* is still in conn->sock->remote_addr. In this case we rather
* delay the release of the old address by moving the cleanup
* to conn->scpool such that it runs when the socket is closed.
* In any other case, including other platforms, just release
* the old address now since conn->sock->remote_addr is either
* obsolete (socket forcibly closed) or a copy on conn->scpool
* already (not a dangling pointer).
*/
int keep_addr_alive = 0,
keep_conn_alive = (conn->sock && conn->addr &&
proxy_addrs_equal(conn->addr,
address->addr));
if (keep_conn_alive) {
#if defined(WIN32) || defined(OS2)
apr_sockaddr_t *remote_addr = NULL;
apr_socket_addr_get(&remote_addr, APR_REMOTE, conn->sock);
for (addr = conn->addr; addr; addr = addr->next) {
if (addr == remote_addr) {
keep_addr_alive = 1;
break;
}
}
#else
/* Nothing to do, keep_addr_alive = 0 */
#endif
}
else if (conn->sock && (r ? APLOGrdebug(r) : APLOGdebug(s))) {
apr_sockaddr_t *local_addr = NULL;
apr_sockaddr_t *remote_addr = NULL;
apr_socket_addr_get(&local_addr, APR_LOCAL, conn->sock);
Expand All @@ -3032,18 +3058,26 @@ PROXY_DECLARE(apr_status_t) ap_proxy_determine_address(const char *proxy_functio
local_addr, remote_addr);
}
}
socket_cleanup(conn);
if (keep_addr_alive) {
apr_pool_cleanup_kill(conn->pool, conn->address,
proxy_address_cleanup);
apr_pool_cleanup_register(conn->scpool, conn->address,
proxy_address_cleanup,
apr_pool_cleanup_null);
}
else {
apr_pool_cleanup_run(conn->pool, conn->address,
proxy_address_cleanup);
if (!keep_conn_alive) {
conn_cleanup(conn);
}
}
}

/* Kill the old address (if any) and use the new one */
if (conn->address) {
apr_pool_cleanup_run(conn->pool, conn->address,
proxy_address_cleanup);
}
/* Use the new address */
apr_pool_cleanup_register(conn->pool, address,
proxy_address_cleanup,
apr_pool_cleanup_null);
address_cleanup(conn);
conn->address = address;
conn->hostname = address->hostname;
conn->port = address->hostport;
Expand Down Expand Up @@ -3125,7 +3159,7 @@ ap_proxy_determine_connection(apr_pool_t *p, request_rec *r,
if (!conn->uds_path || strcmp(conn->uds_path, uds_path) != 0) {
apr_pool_t *pool = conn->pool;
if (conn->uds_path) {
address_cleanup(conn);
conn_cleanup(conn);
if (!conn->uds_pool) {
apr_pool_create(&conn->uds_pool, worker->cp->dns_pool);
}
Expand Down Expand Up @@ -3226,7 +3260,7 @@ ap_proxy_determine_connection(apr_pool_t *p, request_rec *r,
if (conn->hostname
&& (conn->port != hostport
|| ap_cstr_casecmp(conn->hostname, hostname) != 0)) {
address_cleanup(conn);
conn_cleanup(conn);
}

/* Resolve the connection address with the determined hostname/port */
Expand Down

0 comments on commit c66fe1a

Please sign in to comment.