Solve http buffering issues and hangs once and for all (hopefully!) by
simply not trying to return exactly what the caller asked for - just return whatever we got and let the caller be the judge of whether it was enough. If an error occurs or the connection times out after we already received some data, return a short read, under the assumption that the next call will fail or time out before we read anything. As it turns out, none of the code that calls fetch_read() assumes an all-or-nothing result anyway, except for a couple of lines where we read the CR LF at the end of a hunk in HTTP hunked encoding, so the changes outside of fetch_read() and http_readfn() are minimal. While there, replace select(2) with poll(2). MFC after: 3 days
This commit is contained in:
parent
b540294a53
commit
215a27f1a4
Notes:
svn2git
2020-12-20 02:59:44 +00:00
svn path=/head/; revision=261230
@ -41,6 +41,7 @@ __FBSDID("$FreeBSD$");
|
||||
#include <errno.h>
|
||||
#include <fcntl.h>
|
||||
#include <netdb.h>
|
||||
#include <poll.h>
|
||||
#include <pwd.h>
|
||||
#include <stdarg.h>
|
||||
#include <stdlib.h>
|
||||
@ -641,7 +642,7 @@ fetch_ssl_verify_hname(X509 *cert, const char *host)
|
||||
struct addrinfo *ip;
|
||||
STACK_OF(GENERAL_NAME) *altnames;
|
||||
X509_NAME *subject;
|
||||
int ret;
|
||||
int ret;
|
||||
|
||||
ret = 0;
|
||||
ip = fetch_ssl_get_numeric_addrinfo(host, strlen(host));
|
||||
@ -913,33 +914,6 @@ fetch_ssl_read(SSL *ssl, char *buf, size_t len)
|
||||
}
|
||||
#endif
|
||||
|
||||
/*
|
||||
* Cache some data that was read from a socket but cannot be immediately
|
||||
* returned because of an interrupted system call.
|
||||
*/
|
||||
static int
|
||||
fetch_cache_data(conn_t *conn, char *src, size_t nbytes)
|
||||
{
|
||||
char *tmp;
|
||||
|
||||
if (conn->cache.size < nbytes) {
|
||||
tmp = realloc(conn->cache.buf, nbytes);
|
||||
if (tmp == NULL) {
|
||||
fetch_syserr();
|
||||
return (-1);
|
||||
}
|
||||
conn->cache.buf = tmp;
|
||||
conn->cache.size = nbytes;
|
||||
}
|
||||
|
||||
memcpy(conn->cache.buf, src, nbytes);
|
||||
conn->cache.len = nbytes;
|
||||
conn->cache.pos = 0;
|
||||
|
||||
return (0);
|
||||
}
|
||||
|
||||
|
||||
static ssize_t
|
||||
fetch_socket_read(int sd, char *buf, size_t len)
|
||||
{
|
||||
@ -962,46 +936,31 @@ ssize_t
|
||||
fetch_read(conn_t *conn, char *buf, size_t len)
|
||||
{
|
||||
struct timeval now, timeout, delta;
|
||||
fd_set readfds;
|
||||
ssize_t rlen, total;
|
||||
char *start;
|
||||
struct pollfd pfd;
|
||||
ssize_t rlen;
|
||||
int deltams;
|
||||
|
||||
if (fetchTimeout > 0) {
|
||||
gettimeofday(&timeout, NULL);
|
||||
timeout.tv_sec += fetchTimeout;
|
||||
}
|
||||
|
||||
total = 0;
|
||||
start = buf;
|
||||
deltams = INFTIM;
|
||||
memset(&pfd, 0, sizeof pfd);
|
||||
pfd.fd = conn->sd;
|
||||
pfd.events = POLLIN | POLLERR;
|
||||
|
||||
if (conn->cache.len > 0) {
|
||||
/*
|
||||
* The last invocation of fetch_read was interrupted by a
|
||||
* signal after some data had been read from the socket. Copy
|
||||
* the cached data into the supplied buffer before trying to
|
||||
* read from the socket again.
|
||||
*/
|
||||
total = (conn->cache.len < len) ? conn->cache.len : len;
|
||||
memcpy(buf, conn->cache.buf, total);
|
||||
|
||||
conn->cache.len -= total;
|
||||
conn->cache.pos += total;
|
||||
len -= total;
|
||||
buf += total;
|
||||
}
|
||||
|
||||
while (len > 0) {
|
||||
for (;;) {
|
||||
/*
|
||||
* The socket is non-blocking. Instead of the canonical
|
||||
* select() -> read(), we do the following:
|
||||
* poll() -> read(), we do the following:
|
||||
*
|
||||
* 1) call read() or SSL_read().
|
||||
* 2) if an error occurred, return -1.
|
||||
* 3) if we received data but we still expect more,
|
||||
* update our counters and loop.
|
||||
* 2) if we received some data, return it.
|
||||
* 3) if an error occurred, return -1.
|
||||
* 4) if read() or SSL_read() signaled EOF, return.
|
||||
* 5) if we did not receive any data but we're not at EOF,
|
||||
* call select().
|
||||
* call poll().
|
||||
*
|
||||
* In the SSL case, this is necessary because if we
|
||||
* receive a close notification, we have to call
|
||||
@ -1017,46 +976,34 @@ fetch_read(conn_t *conn, char *buf, size_t len)
|
||||
else
|
||||
#endif
|
||||
rlen = fetch_socket_read(conn->sd, buf, len);
|
||||
if (rlen == 0) {
|
||||
if (rlen > 0) {
|
||||
break;
|
||||
} else if (rlen > 0) {
|
||||
len -= rlen;
|
||||
buf += rlen;
|
||||
total += rlen;
|
||||
continue;
|
||||
} else if (rlen == FETCH_READ_ERROR) {
|
||||
if (errno == EINTR)
|
||||
fetch_cache_data(conn, start, total);
|
||||
break;
|
||||
return (-1);
|
||||
}
|
||||
// assert(rlen == FETCH_READ_WAIT);
|
||||
FD_ZERO(&readfds);
|
||||
while (!FD_ISSET(conn->sd, &readfds)) {
|
||||
FD_SET(conn->sd, &readfds);
|
||||
if (fetchTimeout > 0) {
|
||||
gettimeofday(&now, NULL);
|
||||
if (!timercmp(&timeout, &now, >)) {
|
||||
errno = ETIMEDOUT;
|
||||
fetch_syserr();
|
||||
return (-1);
|
||||
}
|
||||
timersub(&timeout, &now, &delta);
|
||||
}
|
||||
errno = 0;
|
||||
if (select(conn->sd + 1, &readfds, NULL, NULL,
|
||||
fetchTimeout > 0 ? &delta : NULL) < 0) {
|
||||
if (errno == EINTR) {
|
||||
if (fetchRestartCalls)
|
||||
continue;
|
||||
/* Save anything that was read. */
|
||||
fetch_cache_data(conn, start, total);
|
||||
}
|
||||
if (fetchTimeout > 0) {
|
||||
gettimeofday(&now, NULL);
|
||||
if (!timercmp(&timeout, &now, >)) {
|
||||
errno = ETIMEDOUT;
|
||||
fetch_syserr();
|
||||
return (-1);
|
||||
}
|
||||
timersub(&timeout, &now, &delta);
|
||||
deltams = delta.tv_sec * 1000 +
|
||||
delta.tv_usec / 1000;;
|
||||
}
|
||||
errno = 0;
|
||||
pfd.revents = 0;
|
||||
if (poll(&pfd, 1, deltams) < 0) {
|
||||
if (errno == EINTR && fetchRestartCalls)
|
||||
continue;
|
||||
fetch_syserr();
|
||||
return (-1);
|
||||
}
|
||||
}
|
||||
return (total);
|
||||
return (rlen);
|
||||
}
|
||||
|
||||
|
||||
@ -1130,20 +1077,21 @@ ssize_t
|
||||
fetch_writev(conn_t *conn, struct iovec *iov, int iovcnt)
|
||||
{
|
||||
struct timeval now, timeout, delta;
|
||||
fd_set writefds;
|
||||
struct pollfd pfd;
|
||||
ssize_t wlen, total;
|
||||
int r;
|
||||
int deltams, r;
|
||||
|
||||
memset(&pfd, 0, sizeof pfd);
|
||||
if (fetchTimeout) {
|
||||
FD_ZERO(&writefds);
|
||||
pfd.fd = conn->sd;
|
||||
pfd.events = POLLOUT | POLLERR;
|
||||
gettimeofday(&timeout, NULL);
|
||||
timeout.tv_sec += fetchTimeout;
|
||||
}
|
||||
|
||||
total = 0;
|
||||
while (iovcnt > 0) {
|
||||
while (fetchTimeout && !FD_ISSET(conn->sd, &writefds)) {
|
||||
FD_SET(conn->sd, &writefds);
|
||||
while (fetchTimeout && pfd.revents == 0) {
|
||||
gettimeofday(&now, NULL);
|
||||
delta.tv_sec = timeout.tv_sec - now.tv_sec;
|
||||
delta.tv_usec = timeout.tv_usec - now.tv_usec;
|
||||
@ -1156,9 +1104,9 @@ fetch_writev(conn_t *conn, struct iovec *iov, int iovcnt)
|
||||
fetch_syserr();
|
||||
return (-1);
|
||||
}
|
||||
deltams = delta.tv_sec * 1000 + delta.tv_usec / 1000;;
|
||||
errno = 0;
|
||||
r = select(conn->sd + 1, NULL, &writefds, NULL, &delta);
|
||||
if (r == -1) {
|
||||
if ((r = poll(&pfd, 1, deltams)) == -1) {
|
||||
if (errno == EINTR && fetchRestartCalls)
|
||||
continue;
|
||||
return (-1);
|
||||
@ -1250,7 +1198,6 @@ fetch_close(conn_t *conn)
|
||||
}
|
||||
#endif
|
||||
ret = close(conn->sd);
|
||||
free(conn->cache.buf);
|
||||
free(conn->buf);
|
||||
free(conn);
|
||||
return (ret);
|
||||
|
@ -52,13 +52,6 @@ struct fetchconn {
|
||||
size_t bufsize; /* buffer size */
|
||||
size_t buflen; /* length of buffer contents */
|
||||
int err; /* last protocol reply code */
|
||||
struct { /* data cached after an interrupted
|
||||
read */
|
||||
char *buf;
|
||||
size_t size;
|
||||
size_t pos;
|
||||
size_t len;
|
||||
} cache;
|
||||
#ifdef WITH_SSL
|
||||
SSL *ssl; /* SSL handle */
|
||||
SSL_CTX *ssl_ctx; /* SSL context */
|
||||
|
@ -208,6 +208,7 @@ static int
|
||||
http_fillbuf(struct httpio *io, size_t len)
|
||||
{
|
||||
ssize_t nbytes;
|
||||
char ch;
|
||||
|
||||
if (io->error)
|
||||
return (-1);
|
||||
@ -249,10 +250,8 @@ http_fillbuf(struct httpio *io, size_t len)
|
||||
io->chunksize -= io->buflen;
|
||||
|
||||
if (io->chunksize == 0) {
|
||||
char endl[2];
|
||||
|
||||
if (fetch_read(io->conn, endl, 2) != 2 ||
|
||||
endl[0] != '\r' || endl[1] != '\n')
|
||||
if (fetch_read(io->conn, &ch, 1) != 1 || ch != '\r' ||
|
||||
fetch_read(io->conn, &ch, 1) != 1 || ch != '\n')
|
||||
return (-1);
|
||||
}
|
||||
|
||||
@ -268,31 +267,28 @@ static int
|
||||
http_readfn(void *v, char *buf, int len)
|
||||
{
|
||||
struct httpio *io = (struct httpio *)v;
|
||||
int l, pos;
|
||||
int rlen;
|
||||
|
||||
if (io->error)
|
||||
return (-1);
|
||||
if (io->eof)
|
||||
return (0);
|
||||
|
||||
for (pos = 0; len > 0; pos += l, len -= l) {
|
||||
/* empty buffer */
|
||||
if (!io->buf || io->bufpos == io->buflen)
|
||||
if (http_fillbuf(io, len) < 1)
|
||||
break;
|
||||
l = io->buflen - io->bufpos;
|
||||
if (len < l)
|
||||
l = len;
|
||||
memcpy(buf + pos, io->buf + io->bufpos, l);
|
||||
io->bufpos += l;
|
||||
/* empty buffer */
|
||||
if (!io->buf || io->bufpos == io->buflen) {
|
||||
if (http_fillbuf(io, len) < 1) {
|
||||
if (io->error == EINTR)
|
||||
io->error = 0;
|
||||
return (-1);
|
||||
}
|
||||
}
|
||||
|
||||
if (!pos && io->error) {
|
||||
if (io->error == EINTR)
|
||||
io->error = 0;
|
||||
return (-1);
|
||||
}
|
||||
return (pos);
|
||||
rlen = io->buflen - io->bufpos;
|
||||
if (len < rlen)
|
||||
rlen = len;
|
||||
memcpy(buf, io->buf + io->bufpos, rlen);
|
||||
io->bufpos += rlen;
|
||||
return (rlen);
|
||||
}
|
||||
|
||||
/*
|
||||
|
Loading…
Reference in New Issue
Block a user