Kevin J. McCarthy wrote: > Kevin J. McCarthy wrote: > > I wasn't really happy having a loop counter option for GnuTLS. It turns > > out the GNUTLS_E_AGAIN is translated directly from EAGAIN/EWOULDBLOCK. > > Since we're not using non-blocking io, this will only be triggered by > > our timeout. > > Sorry, I was wrong. It will return GNUTLS_E_AGAIN for situations other > than the timeout we set, so my patch is incorrect. I'll have to think > about this some more.
After looking some more, I've re-convinced myself the patch approach is actually okay. It looks like GnuTLS will return the GNUTLS_E_AGAIN for a partial read only if recv returns a EAGAIN or EINTR. Otherwise it will keep looping until it gets the needed amount. (see _gnutls_stream_read() and _gnutls_io_read_buffered()) The EAGAIN should only occur for a timeout on no data, and the EINTR should only occur for a signal. Since the timeout is ours and we aren't setting signals, there is no more reason to loop or handle these errors than there is with raw_socket_read or raw_socket_write. Therefore I believe patch should be okay after all. I am attaching a somewhat more aggressive patch that removes the looping from tls_socket_read, and returns tls_socket_write to its behavior of aborting on any error. Feedback and testing is definitely appreciated! -- Kevin J. McCarthy GPG Fingerprint: 8975 A9B3 3AA3 7910 385C 5308 ADEF 7684 8031 6BDA http://www.8t8.us/configs/gpg-key-transition-statement.txt
# HG changeset patch
# User Kevin McCarthy <ke...@8t8.us>
# Date 1437016507 25200
# Wed Jul 15 20:15:07 2015 -0700
# Node ID f9da32c0615726601a5ae5cd5e9211aa7afca12e
# Parent 2ca89bed64480780d0a435e89c13dba06c748094
Add $socket_timeout configuration option.
(closes #3491) (see #3640) (see #3533) (see #3074)
This allows socket reads and writes to time out rather than lock up
completely in some circumstances. Note that Mutt won't currently try
to do anything intelligent with the failure - it will simply abort the
operation and/or close the mailbox.
$socket_timeout by default is set to a conservative 30 seconds, to
hopefully avoid causing other problems.
Change tls_socket_read() to abort on GNUTLS_E_AGAIN or
GNUTLS_E_INTERRUPTED, instead of looping. These will only occur if the
recv returns EAGAIN or EINTR, and there is no more reason to retry here
than there is with raw_socket_read or raw_socket_write.
Thanks to Allen "egbert." for the original patch.
diff --git a/globals.h b/globals.h
--- a/globals.h
+++ b/globals.h
@@ -198,16 +198,17 @@
WHERE short MenuContext;
WHERE short PagerContext;
WHERE short PagerIndexLines;
WHERE short ReadInc;
WHERE short ReflowWrap;
WHERE short SaveHist;
WHERE short SendmailWait;
WHERE short SleepTime INITVAL (1);
+WHERE short SocketTimeout;
WHERE short TimeInc;
WHERE short Timeout;
WHERE short Wrap;
WHERE short WrapHeaders;
WHERE short WriteInc;
WHERE short ScoreThresholdDelete;
WHERE short ScoreThresholdRead;
diff --git a/init.h b/init.h
--- a/init.h
+++ b/init.h
@@ -2883,16 +2883,22 @@
** smtp[s]://[user[:pass]@]host[:port]
** .te
** .pp
** where ``[...]'' denotes an optional part.
** Setting this variable overrides the value of the $$sendmail
** variable.
*/
#endif /* USE_SMTP */
+ { "socket_timeout", DT_NUM, R_NONE, UL &SocketTimeout, 30 },
+ /*
+ ** .pp
+ ** Set socket send and receive timeouts to this many seconds. When
+ ** set to zero, send and receive operations will not time out.
+ */
{ "sort", DT_SORT, R_INDEX|R_RESORT, UL &Sort, SORT_DATE },
/*
** .pp
** Specifies how to sort messages in the ``index'' menu. Valid values
** are:
** .il
** .dd date or date-sent
** .dd date-received
diff --git a/mutt_socket.c b/mutt_socket.c
--- a/mutt_socket.c
+++ b/mutt_socket.c
@@ -339,16 +339,17 @@
return 0;
}
/* socket_connect: set up to connect to a socket fd. */
static int socket_connect (int fd, struct sockaddr* sa)
{
int sa_size;
int save_errno;
+ struct timeval socket_timeout_tv = { 0, 0 };
if (sa->sa_family == AF_INET)
sa_size = sizeof (struct sockaddr_in);
#ifdef HAVE_GETADDRINFO
else if (sa->sa_family == AF_INET6)
sa_size = sizeof (struct sockaddr_in6);
#endif
else
@@ -359,16 +360,35 @@
if (ConnectTimeout > 0)
alarm (ConnectTimeout);
mutt_allow_interrupt (1);
save_errno = 0;
+ if (SocketTimeout > 0)
+ {
+ socket_timeout_tv.tv_sec = SocketTimeout;
+ if (setsockopt (fd, SOL_SOCKET, SO_RCVTIMEO, &socket_timeout_tv,
+ sizeof(struct timeval)))
+ {
+ mutt_perror ("setsockopt");
+ mutt_sleep(3);
+ return -1;
+ }
+ if (setsockopt (fd, SOL_SOCKET, SO_SNDTIMEO, &socket_timeout_tv,
+ sizeof(struct timeval)))
+ {
+ mutt_perror ("setsockopt");
+ mutt_sleep(3);
+ return -1;
+ }
+ }
+
if (connect (fd, sa, sa_size) < 0)
{
save_errno = errno;
dprint (2, (debugfile, "Connection failed. errno: %d...\n", errno));
SigInt = 0; /* reset in case we caught SIGINTR while in connect() */
}
if (ConnectTimeout > 0)
diff --git a/mutt_ssl_gnutls.c b/mutt_ssl_gnutls.c
--- a/mutt_ssl_gnutls.c
+++ b/mutt_ssl_gnutls.c
@@ -134,26 +134,23 @@
if (!data)
{
mutt_error (_("Error: no TLS socket open"));
mutt_sleep (2);
return -1;
}
- do {
- ret = gnutls_record_recv (data->state, buf, len);
- if (ret < 0 && gnutls_error_is_fatal(ret) == 1)
- {
- mutt_error ("tls_socket_read (%s)", gnutls_strerror (ret));
- mutt_sleep (4);
- return -1;
- }
+ ret = gnutls_record_recv (data->state, buf, len);
+ if (ret < 0)
+ {
+ mutt_error ("tls_socket_read (%s)", gnutls_strerror (ret));
+ mutt_sleep (4);
+ return -1;
}
- while (ret == GNUTLS_E_AGAIN || ret == GNUTLS_E_INTERRUPTED);
return ret;
}
static int tls_socket_write (CONNECTION* conn, const char* buf, size_t len)
{
tlssockdata *data = conn->sockdata;
int ret;
@@ -166,25 +163,22 @@
return -1;
}
do
{
ret = gnutls_record_send (data->state, buf + sent, len - sent);
if (ret < 0)
{
- if (gnutls_error_is_fatal(ret) == 1)
- {
- mutt_error ("tls_socket_write (%s)", gnutls_strerror (ret));
- mutt_sleep (4);
- return -1;
- }
- return ret;
+ mutt_error ("tls_socket_write (%s)", gnutls_strerror (ret));
+ mutt_sleep (4);
+ return -1;
}
- sent += ret;
+ else
+ sent += ret;
} while (sent < len);
return sent;
}
static int tls_socket_open (CONNECTION* conn)
{
if (raw_socket_open (conn) < 0)
signature.asc
Description: PGP signature
