Commit fb4937a3 authored by Brad Spencer's avatar Brad Spencer Committed by Daniel Stenberg
Browse files

select: with winsock, avoid passing unsupported arguments to select()

"Any two of the parameters, readfds, writefds, or exceptfds, can be
given as null. At least one must be non-null, and any non-null
descriptor set must contain at least one handle to a socket."

http://msdn.microsoft.com/en-ca/library/windows/desktop/ms740141(v=vs.85).aspx

When using select(), cURL doesn't adhere to this (WinSock-specific)
rule, and can ask to monitor empty fd_sets, which leads to select()
returning WSAEINVAL (i.e. EINVAL) and connections failing in mysterious
ways as a result (at least when using the curl_multi_socket_action()
interface).

Bug: http://curl.haxx.se/mail/lib-2014-05/0278.html
parent 1b894565
Loading
Loading
Loading
Loading
+55 −3
Original line number Diff line number Diff line
@@ -5,7 +5,7 @@
 *                            | (__| |_| |  _ <| |___
 *                             \___|\___/|_| \_\_____|
 *
 * Copyright (C) 1998 - 2012, Daniel Stenberg, <daniel@haxx.se>, et al.
 * Copyright (C) 1998 - 2014, Daniel Stenberg, <daniel@haxx.se>, et al.
 *
 * This software is licensed as described in the file COPYING, which
 * you should have received as part of this distribution. The terms
@@ -288,7 +288,37 @@ int Curl_socket_check(curl_socket_t readfd0, /* two sockets to read from */
      pending_tv.tv_sec = 0;
      pending_tv.tv_usec = 0;
    }
    r = select((int)maxfd + 1, &fds_read, &fds_write, &fds_err, ptimeout);

    /* WinSock select() must not be called with an fd_set that contains zero
       fd flags, or it will return WSAEINVAL.  But, it also can't be called
       with no fd_sets at all!  From the documentation:

         Any two of the parameters, readfds, writefds, or exceptfds, can be
         given as null. At least one must be non-null, and any non-null
         descriptor set must contain at least one handle to a socket.

       We know that we have at least one bit set in at least two fd_sets in
       this case, but we may have no bits set in either fds_read or fd_write,
       so check for that and handle it.  Luckily, with WinSock, we can _also_
       ask how many bits are set on an fd_set.

       It is unclear why WinSock doesn't just handle this for us instead of
       calling this an error.

       Note also that WinSock ignores the first argument, so we don't worry
       about the fact that maxfd is computed incorrectly with WinSock (since
       curl_socket_t is unsigned in such cases and thus -1 is the largest
       value).
    */
    r = select((int)maxfd + 1,
#ifndef USE_WINSOCK
               &fds_read,
               &fds_write,
#else
               fds_read.fd_count ? &fds_read : NULL,
               fds_write.fd_count ? &fds_write : NULL,
#endif
               &fds_err, ptimeout);
    if(r != -1)
      break;
    error = SOCKERRNO;
@@ -446,6 +476,16 @@ int Curl_poll(struct pollfd ufds[], unsigned int nfds, int timeout_ms)
    }
  }

#ifdef USE_WINSOCK
  /* WinSock select() can't handle zero events.  See the comment about this in
     Curl_check_socket(). */
  if(fds_read.fd_count == 0 && fds_write.fd_count == 0
     && fds_err.fd_count == 0) {
    r = Curl_wait_ms(timeout_ms);
    return r;
  }
#endif

  ptimeout = (timeout_ms < 0) ? NULL : &pending_tv;

  do {
@@ -457,7 +497,19 @@ int Curl_poll(struct pollfd ufds[], unsigned int nfds, int timeout_ms)
      pending_tv.tv_sec = 0;
      pending_tv.tv_usec = 0;
    }
    r = select((int)maxfd + 1, &fds_read, &fds_write, &fds_err, ptimeout);
    r = select((int)maxfd + 1,
#ifndef USE_WINSOCK
               &fds_read, &fds_write, &fds_err,
#else
               /* WinSock select() can't handle fd_sets with zero bits set, so
                  don't give it such arguments.  See the comment about this in
                  Curl_check_socket().
               */
               fds_read.fd_count ? &fds_read : NULL,
               fds_write.fd_count ? &fds_write : NULL,
               fds_err.fd_count ? &fds_err : NULL,
#endif
               ptimeout);
    if(r != -1)
      break;
    error = SOCKERRNO;