Author: brane
Date: Sat Jul 26 20:11:05 2025
New Revision: 1927485
Log:
In the Unbound resolver, finally convert the results to apr_sockaddr_t.
* src/resolve.c:
- Do not include <arpa/inet.h> or <netinet/in.h>, they're no longer used.
- Declare APR_WANT_MEMFUNC and APR_WANT_BYTEFUNC throught <apr_want.h>.
- Include <apr_strings.h>.
- Update the top-level todo list.
(INET_ADDRSTRLEN, INET6_ADDRSTRLEN): Define if not defined.
(resolve_result): Remove the 'err' member, it's no longer needed.
(unbound_resolve_task): Add 'host_port_str', copied from apr_uri_t.
(resolve_convert): New; convert struct ub_result to apr_sockaddr_t.
(resolve_finalize): Call resolve_convert and calculate the final status.
(resolve_callback): Remove resolve debug logging, now in resolve_convert.
Fix the interpretation of the return value from apr_atomic_dec32().
(resolve_address_async): Initialize unbound_resolve_task::host_port_str.
Fix the interpretation of the return value from apr_atomic_dec32().
(resolve) [thread-pool resolver]: Use APR's address stringifier in the
debug logging code.
Modified:
serf/trunk/src/resolve.c
Modified: serf/trunk/src/resolve.c
==============================================================================
--- serf/trunk/src/resolve.c Sat Jul 26 18:45:01 2025 (r1927484)
+++ serf/trunk/src/resolve.c Sat Jul 26 20:11:05 2025 (r1927485)
@@ -21,18 +21,14 @@
#include <apr.h>
#include <apr_version.h>
-/* Include the headers needed for inet_ntop and related structs.
- On Windows, we'll always get <Winsock2.h> from <apr.h>. */
-#if APR_HAVE_NETINET_IN_H
-#include <netinet/in.h>
-#endif
-#if APR_HAVE_ARPA_INET_H
-#include <arpa/inet.h>
-#endif
+#define APR_WANT_BYTEFUNC
+#define APR_WANT_MEMFUNC
+#include <apr_want.h>
#include <apr_errno.h>
#include <apr_pools.h>
#include <apr_atomic.h>
+#include <apr_strings.h>
#include <apr_network_io.h>
#include <apr_thread_mutex.h>
#include <apr_thread_pool.h>
@@ -51,6 +47,13 @@
#include "serf.h"
#include "serf_private.h"
+/* Stringified address lengths. */
+#ifndef INET_ADDRSTRLEN
+#define INET_ADDRSTRLEN sizeof("255.255.255.255")
+#endif
+#ifndef INET6_ADDRSTRLEN
+#define INET6_ADDRSTRLEN
sizeof("ffff:ffff:ffff:ffff:ffff:ffff:255.255.255.255")
+#endif
#define HAVE_ASYNC_RESOLVER (SERF_HAVE_ASYNC_RESOLVER || APR_HAS_THREADS)
@@ -60,8 +63,8 @@
* - Wake the poll/select in serf_context_run() when new resolve
* results are available.
*
- * TODO for Unbound:
- * - Convert unbound results to apr_sockaddr_t.
+ * - Unbound: Detect when the "hostname" is actually a stringified IP address.
+ * The resolver doesn't handle that, so we have to short-circuit that case.
*/
@@ -277,7 +280,6 @@ typedef struct unbound_resolve_task reso
struct resolve_result
{
- int err;
apr_status_t status;
struct ub_result* ub_result;
resolve_task_t *task;
@@ -287,6 +289,7 @@ struct resolve_result
struct unbound_resolve_task
{
serf_context_t *ctx;
+ char *host_port_str;
apr_port_t host_port;
/* There can be one or two pending results, depending on whether
@@ -299,15 +302,126 @@ struct unbound_resolve_task
apr_pool_t *resolve_pool;
};
+static apr_status_t resolve_convert(apr_sockaddr_t **host_address,
+ apr_int32_t family,
+ const struct resolve_result *result)
+{
+ const struct unbound_resolve_task *const task = result->task;
+ const struct ub_result *const ub_result = result->ub_result;
+ apr_pool_t *const resolve_pool = task->resolve_pool;
+ apr_status_t status = result->status;
+
+ if (status == APR_SUCCESS)
+ {
+ char *hostname = apr_pstrdup(resolve_pool, (ub_result->canonname
+ ? ub_result->canonname
+ : ub_result->qname));
+ apr_socklen_t salen;
+ int ipaddr_len;
+ int addr_str_len;
+ int i;
+
+ if (family == APR_INET) {
+ salen = sizeof(struct sockaddr_in);
+ ipaddr_len = sizeof(struct in_addr);
+ addr_str_len = INET_ADDRSTRLEN;
+ }
+#if APR_HAVE_IPV6
+ else {
+ salen = sizeof(struct sockaddr_in6);
+ ipaddr_len = sizeof(struct in6_addr);
+ addr_str_len = INET6_ADDRSTRLEN;
+ }
+#endif
+
+ /* Check that all result sizes are correct. */
+ for (i = 0; ub_result->data[i]; ++i)
+ {
+ if (ub_result->len[i] != ipaddr_len) {
+ /* TODO: Error callback */
+ serf__log(LOGLVL_ERROR, LOGCOMP_CONN, __FILE__,
+ task->ctx->config,
+ "unbound resolve: [%s] invalid address: "
+ " length %d, expected %d\n",
+ result->qtype, ub_result->len[i], ipaddr_len);
+ status = APR_EINVAL;
+ goto cleanup;
+ }
+ }
+
+ for (i = 0; ub_result->data[i]; ++i)
+ {
+ apr_sockaddr_t *addr = apr_palloc(resolve_pool, sizeof(*addr));
+ addr->pool = resolve_pool;
+ addr->hostname = hostname;
+ addr->servname = task->host_port_str;
+ addr->port = task->host_port;
+ addr->family = family;
+ addr->salen = salen;
+ addr->ipaddr_len = ipaddr_len;
+ addr->addr_str_len = addr_str_len;
+ addr->next = *host_address;
+
+ memset(&addr->sa, 0, sizeof(addr->sa));
+ if (family == APR_INET) {
+ addr->ipaddr_ptr = &addr->sa.sin.sin_addr.s_addr;
+ addr->sa.sin.sin_family = APR_INET;
+ addr->sa.sin.sin_port = htons(addr->port);
+ }
+#if APR_HAVE_IPV6
+ else {
+ addr->ipaddr_ptr = &addr->sa.sin6.sin6_addr.s6_addr;
+ addr->sa.sin6.sin6_family = APR_INET6;
+ addr->sa.sin6.sin6_port = htons(addr->port);
+ }
+#endif
+ memcpy(addr->ipaddr_ptr, ub_result->data[i], ipaddr_len);
+ *host_address = addr;
+
+ if (serf__log_enabled(LOGLVL_DEBUG, LOGCOMP_CONN,
+ task->ctx->config)) {
+ char buf[INET6_ADDRSTRLEN];
+ if (!apr_sockaddr_ip_getbuf(buf, sizeof(buf), addr)) {
+ serf__log(LOGLVL_DEBUG, LOGCOMP_CONN,
+ __FILE__, task->ctx->config,
+ "unbound resolve: [%s] %s: %s\n",
+ result->qtype, addr->hostname, buf);
+ }
+ }
+ }
+ }
+
+ cleanup:
+ if (result->ub_result)
+ ub_resolve_free(result->ub_result);
+
+ return status;
+}
+
static void resolve_finalize(resolve_task_t *task)
{
- /* TODO: Convert ub_result to apr_sockaddr_t */
- if (task->results[0].ub_result)
- ub_resolve_free(task->results[0].ub_result);
- if (task->results[1].ub_result)
- ub_resolve_free(task->results[1].ub_result);
+ apr_sockaddr_t *host_address = NULL;
+ apr_status_t status, status6;
+
+ status = resolve_convert(&host_address, APR_INET, &task->results[0]);
+#if APR_HAVE_IPV6
+ status6 = resolve_convert(&host_address, APR_INET6, &task->results[1]);
+#else
+ status6 = APR_SUCCESS;
+#endif
- push_resolve_result(task->ctx, NULL, APR_EAFNOSUPPORT,
+ if (!host_address) {
+ if (status == APR_SUCCESS)
+ status = status6;
+ if (status == APR_SUCCESS)
+ status = APR_ENOENT;
+ }
+ else {
+ /* If we got a result, ee don't care if one of the resolves failed. */
+ status = APR_SUCCESS;
+ }
+
+ push_resolve_result(task->ctx, host_address, status,
task->resolved, task->resolved_baton,
task->resolve_pool);
}
@@ -369,33 +483,11 @@ static void resolve_callback(void* baton
}
}
- resolve_result->err = err;
resolve_result->status = status;
resolve_result->ub_result = ub_result;
- if (status == APR_SUCCESS
- && serf__log_enabled(LOGLVL_DEBUG, LOGCOMP_CONN, task->ctx->config))
- {
- char buf[INET6_ADDRSTRLEN];
- const socklen_t len = sizeof(buf);
- int i;
-
- for (i = 0; ub_result->data && ub_result->data[i]; ++i) {
- const char *address = "(AF-unknown)";
-
- if (ub_result->len[i] == sizeof(struct in_addr))
- address = inet_ntop(AF_INET, ub_result->data[i], buf, len);
- else if (ub_result->len[i] == sizeof(struct in6_addr))
- address = inet_ntop(AF_INET6, ub_result->data[i], buf, len);
- serf__log(LOGLVL_DEBUG, LOGCOMP_CONN,
- __FILE__, task->ctx->config,
- "unbound resolve: [%s] %s: %s\n",
- resolve_result->qtype, ub_result->qname, address);
- }
- }
-
/* The last pending task combines and publishes the results. */
- if (apr_atomic_dec32(&task->pending_results) == 1)
+ if (apr_atomic_dec32(&task->pending_results) == 0)
resolve_finalize(task);
}
@@ -412,6 +504,7 @@ static apr_status_t resolve_address_asyn
int err4 = 0, err6 = 0;
task->ctx = ctx;
+ task->host_port_str = apr_pstrdup(resolve_pool, host_info.port_str);
task->host_port = host_info.port;
#if APR_HAVE_IPV6
@@ -419,7 +512,6 @@ static apr_status_t resolve_address_asyn
#else
task->pending_results = 1;
#endif
- task->results[0].err = task->results[1].err = 0;
task->results[0].status = task->results[1].status = APR_SUCCESS;
task->results[0].ub_result = task->results[1].ub_result = NULL;
task->results[0].task = task->results[1].task = task;
@@ -474,7 +566,7 @@ static apr_status_t resolve_address_asyn
/* If one of the tasks failed and the other has already completed,
we have to do the result processing here. Note that the Unbound
callbacks can be called synchronously from ub_resolve_async(). */
- if (pending_results == 1)
+ if (pending_results == 0)
resolve_finalize(task);
}
@@ -589,14 +681,11 @@ static void *APR_THREAD_FUNC resolve(apr
while (addr)
{
char buf[INET6_ADDRSTRLEN];
- const socklen_t len = sizeof(buf);
- const char *address = "(AF-unknown)";
-
- if (addr->family == APR_INET || addr->family == APR_INET6)
- address = inet_ntop(addr->family, addr->ipaddr_ptr, buf, len);
- serf__log(LOGLVL_DEBUG, LOGCOMP_CONN,
- __FILE__, task->ctx->config,
- "apr async resolve: %s: %s\n", addr->hostname, address);
+ if (!apr_sockaddr_ip_getbuf(buf, sizeof(buf), addr)) {
+ serf__log(LOGLVL_DEBUG, LOGCOMP_CONN,
+ __FILE__, task->ctx->config,
+ "apr async resolve: %s: %s\n", addr->hostname, buf);
+ }
addr = addr->next;
}
}
