From cd279db3cfb82b185c82d1655ac1e629594d6e65 Mon Sep 17 00:00:00 2001 From: Yann Ylavic Date: Wed, 19 Oct 2022 15:28:03 +0000 Subject: apr_socket_sendv: WIN32: Follow up to r1902312: Avoid short writes. We possibly (edge cases) can't send an entire iovec array in a single WSASend() call because structs WSABUF and iovec are ABI incompatible. To avoid breaking users that rely on full-write by apr_socket_sendv(), which supposedly is guaranteed by WSASend(), repeat the call until the given iovec array is exhausted. There is no way to provide both full-write and atomicity guarantees for apr_socket_sendv() on Windows, so we choose the former.. * include/apr_network_io.h: Document apr_socket_sendv() full-write/atomicity (non-)guarantees above the system ones. * network_io/win32/sendrecv.c(apr_socket_sendv): Change to a loop on WSASend() when needed, taking care of its API limits w.r.t. the given struct iovec. git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1904699 13f79535-47bb-0310-9956-ffa450edef68 --- include/apr_network_io.h | 6 ++++ network_io/win32/sendrecv.c | 87 +++++++++++++++++++++++++++++---------------- 2 files changed, 63 insertions(+), 30 deletions(-) diff --git a/include/apr_network_io.h b/include/apr_network_io.h index b3536d802..e37b95297 100644 --- a/include/apr_network_io.h +++ b/include/apr_network_io.h @@ -574,6 +574,12 @@ APR_DECLARE(apr_status_t) apr_socket_send(apr_socket_t *sock, const char *buf, * socket option. * The number of bytes actually sent is stored in argument 4. * + * This function does not provide full-write and/or atomicity guarantees + * if the underlying system call does not either. Some systems (like Windows) + * guarantee both for a single system call but this call does not allow for + * as much data as an iovec vector can contain; in this case apr_socket_sendv() + * can issue multiple system calls thus favoring full-write over atomicity. + * * It is possible for both bytes to be sent and an error to be returned. * * APR_EINTR is never returned. diff --git a/network_io/win32/sendrecv.c b/network_io/win32/sendrecv.c index eb4dd241a..c04d27e29 100644 --- a/network_io/win32/sendrecv.c +++ b/network_io/win32/sendrecv.c @@ -20,6 +20,7 @@ #include "apr_network_io.h" #include "apr_lib.h" #include "apr_arch_file_io.h" +#include #if APR_HAVE_TIME_H #include #endif @@ -37,7 +38,11 @@ /* Maximum number of WSABUF allocated for a single apr_socket_sendv() */ #define WSABUF_ON_STACK 50 -#define WSABUF_ON_HEAP 500 +#if APR_SIZEOF_VOIDP < 8 +#define WSABUF_ON_HEAP (APR_DWORD_MAX / sizeof(WSABUF)) +#else +#define WSABUF_ON_HEAP (APR_DWORD_MAX) +#endif APR_DECLARE(apr_status_t) apr_socket_send(apr_socket_t *sock, const char *buf, apr_size_t *len) @@ -94,59 +99,81 @@ APR_DECLARE(apr_status_t) apr_socket_sendv(apr_socket_t *sock, apr_status_t rc = APR_SUCCESS; apr_ssize_t rv; apr_size_t cur_len; + apr_size_t cur_pos = 0; + apr_size_t total_len = 0; apr_size_t nvec = 0; - apr_size_t n; int i; - DWORD dwBytes = 0; WSABUF *pWsaBuf; + *nbytes = 0; /* until further notice */ + for (i = 0; i < in_vec; i++) { cur_len = vec[i].iov_len; - - while (cur_len > APR_DWORD_MAX) { - if (nvec >= WSABUF_ON_HEAP) { - break; - } - nvec++; - cur_len -= APR_DWORD_MAX; - } - if (nvec >= WSABUF_ON_HEAP) { - break; + if (!cur_len) { + continue; } + if (cur_len > APR_SIZE_MAX - total_len) { + /* max total_len can take */ + return APR_EINVAL; + } + total_len += cur_len; nvec++; } + if (!nvec) { + return (in_vec >= 0) ? APR_SUCCESS : APR_EINVAL; + } + if (nvec > WSABUF_ON_HEAP) { + /* max malloc() and/or WSASend() can take */ + nvec = WSABUF_ON_HEAP; + } pWsaBuf = (nvec <= WSABUF_ON_STACK) ? _alloca(sizeof(WSABUF) * (nvec)) : malloc(sizeof(WSABUF) * (nvec)); if (!pWsaBuf) return APR_ENOMEM; - for (n = i = 0; n < nvec; i++) { - char * base = vec[i].iov_base; - cur_len = vec[i].iov_len; + for (i = 0; total_len > 0;) { + DWORD nbuf = 0, nsend = 0, nsent = 0; do { - if (cur_len > APR_DWORD_MAX) { - pWsaBuf[n].buf = base; - pWsaBuf[n].len = APR_DWORD_MAX; - cur_len -= APR_DWORD_MAX; - base += APR_DWORD_MAX; + assert(i < in_vec); + cur_len = vec[i].iov_len - cur_pos; + if (!cur_len) { + assert(cur_pos == 0); + i++; + continue; + } + pWsaBuf[nbuf].buf = (char *)vec[i].iov_base + cur_pos; + if (cur_len > APR_DWORD_MAX - nsend) { + cur_len = APR_DWORD_MAX - nsend; + cur_pos += cur_len; } else { - pWsaBuf[n].buf = base; - pWsaBuf[n].len = (DWORD)cur_len; - cur_len = 0; + cur_pos = 0; + i++; } - } while (++n < nvec && cur_len > 0); - } - rv = WSASend(sock->socketdes, pWsaBuf, nvec, &dwBytes, 0, NULL, NULL); - if (rv == SOCKET_ERROR) { - rc = apr_get_netos_error(); + nsend += cur_len; + pWsaBuf[nbuf++].len = (DWORD)cur_len; + } while (nbuf < nvec && nsend < total_len && nsend < APR_DWORD_MAX); + + rv = WSASend(sock->socketdes, pWsaBuf, nbuf, &nsent, 0, NULL, NULL); + if (rv == SOCKET_ERROR) { + rc = apr_get_netos_error(); + break; + } + *nbytes += nsent; + if (nsent < nsend) { + /* Stop on short/partial write (nonblocking supposedly, but anyway + * we don't guarantee full write if the system does not either). + */ + break; + } + total_len -= nsent; } + if (nvec > WSABUF_ON_STACK) free(pWsaBuf); - *nbytes = dwBytes; return rc; } -- cgit v1.2.1