Subject: Re: bug#23982: [master] Treat errno EINPROGRESS and ENOTCONN as EAGAIN for async connection
From: Paul Eggert <[email-masked]>
To: Eli Zaretskii <[email-masked]>, Ted Zlatanov <[email-masked]>
Cc: [email-masked], [email-masked], [email-masked]
Date: Wed, 3 Aug 2016 02:02:11 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

Eli Zaretskii wrote:
> I'm building Emacs with GnuTLS 3.3.11 since many moons ago.

OK, I installed the attached patches to GNU Emacs master. The first one requires 
GnuTLS 2.12.2 or later. The second one uses the GNUTLS_NONBLOCK flag, and works 
around the apparent GnuTLS bug with EINPROGRESS and ENOTCONN.

From b283c603b546f678c3eb7d31b06c28b7da285962 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Wed, 3 Aug 2016 04:33:21 -0400
Subject: [PATCH 1/2] Require GnuTLS 2.12.2 or later
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* configure.ac, etc/NEWS: Require GnuTLS 2.12.2 or later,
instead of merely 2.6.6 or later (Bug#23982#20).
* src/gnutls.c (init_gnutls_functions, emacs_gnutls_handshake)
[WINDOWSNT]: Don’t bother with gnutls_check_version or
gnutls_transport_set_lowat, as they are no longer needed now that
we require gnutls 2.12.2 or later.
---
 configure.ac |  2 +-
 etc/NEWS     |  3 +++
 src/gnutls.c | 23 -----------------------
 3 files changed, 4 insertions(+), 24 deletions(-)

diff --git a/configure.ac b/configure.ac
index cca555c..e8aeae2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2748,7 +2748,7 @@ AC_DEFUN
 
 HAVE_GNUTLS=no
 if test "${with_gnutls}" = "yes" ; then
-  EMACS_CHECK_MODULES([LIBGNUTLS], [gnutls >= 2.6.6],
+  EMACS_CHECK_MODULES([LIBGNUTLS], [gnutls >= 2.12.2],
     [HAVE_GNUTLS=yes], [HAVE_GNUTLS=no])
   if test "${HAVE_GNUTLS}" = "yes"; then
     AC_DEFINE(HAVE_GNUTLS, 1, [Define if using GnuTLS.])
diff --git a/etc/NEWS b/etc/NEWS
index 6462eff..fadf4c2 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -28,6 +28,9 @@ otherwise leave it unmarked.
 ** By default libgnutls is now required when building Emacs.
 Use 'configure --with-gnutls=no' to build even when GnuTLS is missing.
 
+** GnuTLS version 2.12.2 or later is now required, instead of merely
+version 2.6.6 or later.
+
 ** The new option 'configure --enable-gcc-warnings=warn-only' causes
 GCC to issue warnings without stopping the build.  This behavior is
 now the default in developer builds.  As before, use
diff --git a/src/gnutls.c b/src/gnutls.c
index 7f05ac4..681e298 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -110,8 +110,6 @@ DEF_DLL_FN (ssize_t, gnutls_record_send,
 	    (gnutls_session_t, const void *, size_t));
 DEF_DLL_FN (const char *, gnutls_strerror, (int));
 DEF_DLL_FN (void, gnutls_transport_set_errno, (gnutls_session_t, int));
-DEF_DLL_FN (const char *, gnutls_check_version, (const char *));
-DEF_DLL_FN (void, gnutls_transport_set_lowat, (gnutls_session_t, int));
 DEF_DLL_FN (void, gnutls_transport_set_ptr2,
 	    (gnutls_session_t, gnutls_transport_ptr_t,
 	     gnutls_transport_ptr_t));
@@ -225,11 +223,6 @@ init_gnutls_functions (void)
   LOAD_DLL_FN (library, gnutls_record_send);
   LOAD_DLL_FN (library, gnutls_strerror);
   LOAD_DLL_FN (library, gnutls_transport_set_errno);
-  LOAD_DLL_FN (library, gnutls_check_version);
-  /* We don't need to call gnutls_transport_set_lowat in GnuTLS 2.11.1
-     and later, and the function was removed entirely in 3.0.0.  */
-  if (!fn_gnutls_check_version ("2.11.1"))
-    LOAD_DLL_FN (library, gnutls_transport_set_lowat);
   LOAD_DLL_FN (library, gnutls_transport_set_ptr2);
   LOAD_DLL_FN (library, gnutls_transport_set_pull_function);
   LOAD_DLL_FN (library, gnutls_transport_set_push_function);
@@ -290,7 +283,6 @@ init_gnutls_functions (void)
 # define gnutls_certificate_set_x509_trust_file fn_gnutls_certificate_set_x509_trust_file
 # define gnutls_certificate_type_get fn_gnutls_certificate_type_get
 # define gnutls_certificate_verify_peers2 fn_gnutls_certificate_verify_peers2
-# define gnutls_check_version fn_gnutls_check_version
 # define gnutls_cipher_get fn_gnutls_cipher_get
 # define gnutls_cipher_get_name fn_gnutls_cipher_get_name
 # define gnutls_credentials_set fn_gnutls_credentials_set
@@ -321,7 +313,6 @@ init_gnutls_functions (void)
 # define gnutls_sign_get_name fn_gnutls_sign_get_name
 # define gnutls_strerror fn_gnutls_strerror
 # define gnutls_transport_set_errno fn_gnutls_transport_set_errno
-# define gnutls_transport_set_lowat fn_gnutls_transport_set_lowat
 # define gnutls_transport_set_ptr2 fn_gnutls_transport_set_ptr2
 # define gnutls_transport_set_pull_function fn_gnutls_transport_set_pull_function
 # define gnutls_transport_set_push_function fn_gnutls_transport_set_push_function
@@ -439,20 +430,6 @@ emacs_gnutls_handshake (struct Lisp_Process *proc)
 				 (gnutls_transport_ptr_t) proc);
       gnutls_transport_set_push_function (state, &emacs_gnutls_push);
       gnutls_transport_set_pull_function (state, &emacs_gnutls_pull);
-
-      /* For non blocking sockets or other custom made pull/push
-	 functions the gnutls_transport_set_lowat must be called, with
-	 a zero low water mark value. (GnuTLS 2.10.4 documentation)
-
-	 (Note: this is probably not strictly necessary as the lowat
-	  value is only used when no custom pull/push functions are
-	  set.)  */
-      /* According to GnuTLS NEWS file, lowat level has been set to
-	 zero by default in version 2.11.1, and the function
-	 gnutls_transport_set_lowat was removed from the library in
-	 version 2.99.0.  */
-      if (!gnutls_check_version ("2.11.1"))
-	gnutls_transport_set_lowat (state, 0);
 #else
       /* This is how GnuTLS takes sockets: as file descriptors passed
 	 in.  For an Emacs process socket, infd and outfd are the
-- 
2.5.5

From 3029f74ad5b8f989e26673a31c257fa2aa8d7304 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Wed, 3 Aug 2016 01:54:20 -0700
Subject: [PATCH 2/2] Fix non-blocking GnuTLS with slow connection

Although the problem is reported for OS X (Bug#23982), it seems to
be possible on other POSIXish platforms.
* src/gnutls.c (emacs_gnutls_nonblock_errno) [!WINDOWSNT]:
New function.
(emacs_gnutls_handshake) [!WINDOWSNT]:
Use it as the errno function, if non-blocking.
(Fgnutls_boot): Use GNUTLS_NONBLOCK if non-blocking.
---
 src/gnutls.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/src/gnutls.c b/src/gnutls.c
index 681e298..e3e9311 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -411,6 +411,31 @@ gnutls_try_handshake (struct Lisp_Process *proc)
   return ret;
 }
 
+#ifndef WINDOWSNT
+static int
+emacs_gnutls_nonblock_errno (gnutls_transport_ptr_t ptr)
+{
+  int err = errno;
+
+  switch (err)
+    {
+# ifdef _AIX
+      /* This is taken from the GnuTLS system_errno function circa 2016;
+	 see <http://savannah.gnu.org/support/?107464>.  */
+    case 0:
+      errno = EAGAIN;
+      /* Fall through.  */
+# endif
+    case EINPROGRESS:
+    case ENOTCONN:
+      return EAGAIN;
+
+    default:
+      return err;
+    }
+}
+#endif
+
 static int
 emacs_gnutls_handshake (struct Lisp_Process *proc)
 {
@@ -437,6 +462,9 @@ emacs_gnutls_handshake (struct Lisp_Process *proc)
       gnutls_transport_set_ptr2 (state,
 				 (void *) (intptr_t) proc->infd,
 				 (void *) (intptr_t) proc->outfd);
+      if (proc->is_non_blocking_client)
+	gnutls_transport_set_errno_function (state,
+					     emacs_gnutls_nonblock_errno);
 #endif
 
       proc->gnutls_initstage = GNUTLS_STAGE_TRANSPORT_POINTERS_SET;
@@ -1574,7 +1602,8 @@ one trustfile (usually a CA bundle).  */)
   /* Call gnutls_init here: */
 
   GNUTLS_LOG (1, max_log_level, "gnutls_init");
-  ret = gnutls_init (&state, GNUTLS_CLIENT);
+  int nonblock = XPROCESS (proc)->is_non_blocking_client ? GNUTLS_NONBLOCK : 0;
+  ret = gnutls_init (&state, GNUTLS_CLIENT | nonblock);
   XPROCESS (proc)->gnutls_state = state;
   if (ret < GNUTLS_E_SUCCESS)
     return gnutls_make_error (ret);
-- 
2.5.5