The psprintf code makes the assumption that the apr_vformatter code does
not allocate from the pool being used. The %pI implementation breaks
this assumption because apr_sockaddr_ip_get does palloc; simple uses of
%pI can end up printing a partial string depending on the state of the
pool on entry.
This can be fixed by introducing a non-allocating variant of
apr_sockaddr_ip_get(); I called this apr_sockaddr_ip_getbuf here. Any
objections?
Index: include/apr_network_io.h
===================================================================
--- include/apr_network_io.h (revision 278993)
+++ include/apr_network_io.h (working copy)
@@ -669,6 +669,15 @@
apr_sockaddr_t *sockaddr);
/**
+ * Write the IP address (in numeric address string format) of the APR
+ * socket address @a sockaddr into the buffer @a buf (of size @a buflen).
+ * @param addr The IP address.
+ * @param sockaddr The socket address to reference.
+ */
+APR_DECLARE(apr_status_t) apr_sockaddr_ip_getbuf(char *buf, apr_size_t buflen,
+ apr_sockaddr_t *sockaddr);
+
+/**
* See if the IP addresses in two APR socket addresses are
* equivalent. Appropriate logic is present for comparing
* IPv4-mapped IPv6 addresses with IPv4 addresses.
Index: strings/apr_snprintf.c
===================================================================
--- strings/apr_snprintf.c (revision 278993)
+++ strings/apr_snprintf.c (working copy)
@@ -474,7 +474,14 @@
p = conv_10(sa->port, TRUE, &is_negative, p, &sub_len);
*--p = ':';
- apr_sockaddr_ip_get(&ipaddr_str, sa);
+ ipaddr_str = buf_end - NUM_BUF_SIZE;
+ if (apr_sockaddr_ip_getbuf(ipaddr_str, sa->addr_str_len, sa)) {
+ /* Should only fail if the buffer is too small, which it
+ * should not be; but fail safe anyway: */
+ *--p = '?';
+ *len = buf_end - p;
+ return p;
+ }
sub_len = strlen(ipaddr_str);
#if APR_HAVE_IPV6
if (sa->family == APR_INET6 &&
Index: network_io/unix/sockaddr.c
===================================================================
--- network_io/unix/sockaddr.c (revision 278993)
+++ network_io/unix/sockaddr.c (working copy)
@@ -98,27 +98,37 @@
}
}
-APR_DECLARE(apr_status_t) apr_sockaddr_ip_get(char **addr,
- apr_sockaddr_t *sockaddr)
+APR_DECLARE(apr_status_t) apr_sockaddr_ip_getbuf(char *buf, apr_size_t buflen,
+ apr_sockaddr_t *sockaddr)
{
- *addr = apr_palloc(sockaddr->pool, sockaddr->addr_str_len);
- apr_inet_ntop(sockaddr->family,
- sockaddr->ipaddr_ptr,
- *addr,
- sockaddr->addr_str_len);
+ if (!apr_inet_ntop(sockaddr->family, sockaddr->ipaddr_ptr, buf, buflen)) {
+ return APR_ENOSPC;
+ }
+
#if APR_HAVE_IPV6
- if (sockaddr->family == AF_INET6 &&
- IN6_IS_ADDR_V4MAPPED((struct in6_addr *)sockaddr->ipaddr_ptr)) {
+ if (sockaddr->family == AF_INET6
+ && IN6_IS_ADDR_V4MAPPED((struct in6_addr *)sockaddr->ipaddr_ptr)
+ && buflen > strlen("::ffff:")) {
/* This is an IPv4-mapped IPv6 address; drop the leading
* part of the address string so we're left with the familiar
* IPv4 format.
*/
- *addr += strlen("::ffff:");
+ memmove(buf, buf + strlen("::ffff:"),
+ strlen(buf + strlen("::ffff:")));
}
#endif
+ /* ensure NUL termination if the buffer is too short */
+ buf[buflen-1] = '\0';
return APR_SUCCESS;
}
+APR_DECLARE(apr_status_t) apr_sockaddr_ip_get(char **addr,
+ apr_sockaddr_t *sockaddr)
+{
+ *addr = apr_palloc(sockaddr->pool, sockaddr->addr_str_len);
+ return apr_sockaddr_ip_getbuf(*addr, sockaddr->addr_str_len, sockaddr);
+}
+
void apr_sockaddr_vars_set(apr_sockaddr_t *addr, int family, apr_port_t port)
{
addr->family = family;
Index: test/testsock.c
===================================================================
--- test/testsock.c (revision 278993)
+++ test/testsock.c (working copy)
@@ -203,6 +203,19 @@
APR_ASSERT_SUCCESS(tc, "Problem closing socket", rv);
}
+static void test_print_addr(abts_case *tc, void *data)
+{
+ apr_sockaddr_t *sa;
+ char *s;
+
+ APR_ASSERT_SUCCESS(tc, "Problem generating sockaddr",
+ apr_sockaddr_info_get(&sa, "0.0.0.0", APR_INET, 80, 0,
p));
+
+ s = apr_psprintf(p, "foo %pI bar", sa);
+
+ ABTS_STR_EQUAL(tc, "foo 0.0.0.0:80 bar", s);
+}
+
abts_suite *testsock(abts_suite *suite)
{
suite = ADD_SUITE(suite)
@@ -212,6 +225,7 @@
abts_run_test(suite, test_send, NULL);
abts_run_test(suite, test_recv, NULL);
abts_run_test(suite, test_timeout, NULL);
+ abts_run_test(suite, test_print_addr, NULL);
return suite;
}
