bug#22789: 25.1.50; In last master build https connections stop working

[Alain Schneble <a.s@realize.ch>]

[-- Attachment #1: Type: text/plain, Size: 1817 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

> I think this change should be installed regardless, as it fixes an
> oversight.  However, I think it needs to be augmented, because the
> fact that FILE_CONNECT flag is set doesn't necessarily mean the
> connection is in progress: it could have failed already.  We need to
> look at the status as well.
>
> The possible states of the FILE_CONNECT flag and the cp->status values
> are:
>
>   flag    status                      description
>   ----------------------------------------------------------------------------
>   ON      STATUS_READ_READY           reader thread is about to try connect
>   ON      STATUS_READ_FAILED          reader thread waits in _sys_wait_connect
>   ON      STATUS_READ_SUCCEEDED       reader thread successfully connected
>   ON      STATUS_CONNECT_FAILED       reader thread failed to connect
>   OFF     STATUS_READ_ACKNOWLEDGED    sys_select acknowledged successful connect
>   OFF     STATUS_READ_READY           reader thread is about to read
>   OFF     STATUS_READ_IN_PROGRESS     reader thread waits in _sys_read_ahead
>   OFF     STATUS_READ_SUCCEEDED       reader thread succeeded in reading
>   OFF     STATUS_READ_FAILED          reader thread failed to read
>
> So we should only return EWOULDBLOCK when FILE_CONNECT is set _and_
> the status is not STATUS_CONNECT_FAILED.  If FILE_CONNECT is set, but
> the status is STATUS_CONNECT_FAILED, we should instead return the
> value computed from cp->errcode (if it is non-zero).  There's an
> example of that in sys_read.

Thank you very much for these inputs.  I rearranged the patch to include
these two cases and removed another special case that should no longer
be needed as it is covered by the first one.

Is this what you had in mind?  Do you agree with the change?

Thanks.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch --]
[-- Type: text/x-patch, Size: 2727 bytes --]

From 81295b036eb0a43dee968e8aa3f031030589cddd Mon Sep 17 00:00:00 2001
From: Alain Schneble <a.s@realize.ch>
Date: Mon, 7 Mar 2016 23:05:40 +0100
Subject: [PATCH] Resolve non-blocking socket connection issue on w32

* src/w32.c (sys_write): For non-blocking sockets, return immediately
with EWOULDBLOCK if connection is still in progress.  If connection
attempt has failed already, return proper code stashed in cp->errcode.
BTW, this ensures we do not temporarily turn the socket into blocking
mode for the pfn_send call if the connection is in progress.  It turned
out that doing so causes arbitrary GnuTLS handshake failures on
MS-Windows.  (bug#22789)
---
 src/w32.c | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/src/w32.c b/src/w32.c
index ccf7cc3..c553152 100644
--- a/src/w32.c
+++ b/src/w32.c
@@ -8772,6 +8772,30 @@ sys_write (int fd, const void * buffer, unsigned int count)
       unsigned long nblock = 0;
       if (winsock_lib == NULL) emacs_abort ();
 
+      child_process *cp = fd_info[fd].cp;
+
+      /* If this is a non-blocking socket whose connection is in
+	 progress or terminated with an error already, return the
+	 proper error code to the caller. */
+      if (cp != NULL && (fd_info[fd].flags & FILE_CONNECT) != 0)
+	{
+	  /* In case connection is in progress, ENOTCONN that would
+	     result from calling pfn_send is not what callers expect. */
+	  if (cp->status != STATUS_CONNECT_FAILED)
+	    {
+	      errno = EWOULDBLOCK;
+	      return -1;
+	    }
+	  /* In case connection failed, use the actual error code
+	     stashed by '_sys_wait_connect' in cp->errcode. */
+	  else if (cp->errcode != 0)
+	    {
+	      pfn_WSASetLastError (cp->errcode);
+	      set_errno ();
+	      return -1;
+	    }
+	}
+
       /* TODO: implement select() properly so non-blocking I/O works. */
       /* For now, make sure the write blocks.  */
       if (fd_info[fd].flags & FILE_NDELAY)
@@ -8782,14 +8806,8 @@ sys_write (int fd, const void * buffer, unsigned int count)
       if (nchars == SOCKET_ERROR)
         {
 	  set_errno ();
-	  /* If this is a non-blocking socket whose connection is in
-	     progress, return the proper error code to the caller;
-	     ENOTCONN is not what they expect . */
-	  if (errno == ENOTCONN && (fd_info[fd].flags & FILE_CONNECT) != 0)
-	    errno = EWOULDBLOCK;
-	  else
-	    DebPrint (("sys_write.send failed with error %d on socket %ld\n",
-		       pfn_WSAGetLastError (), SOCK_HANDLE (fd)));
+	  DebPrint (("sys_write.send failed with error %d on socket %ld\n",
+		     pfn_WSAGetLastError (), SOCK_HANDLE (fd)));
 	}
 
       /* Set the socket back to non-blocking if it was before,
-- 
2.6.2.windows.1