sendf.c: added ability to call recv() before send() as workaround

WinSock destroys recv() buffer if send() is failed. As result - server
response may be lost if server sent it while curl is still sending
request. This behavior noticeable on HTTP server short replies if
libcurl use several send() for request (usually for POST request).
To workaround this problem, libcurl use recv() before every send() and
keeps received data in intermediate buffer for further processing.

Fixes: #657
Closes: #668

diff --git a/lib/curl_setup.h b/lib/curl_setup.h
index da26e48cbe7d9479505aab4dc7728d9085e36cbc..d78873fe53a9e1fe6ccf98843ee88baea1939ddb 100644 (file)
--- a/lib/curl_setup.h
+++ b/lib/curl_setup.h
@@ -722,4 +722,19 @@ endings either CRLF or LF so 't' is appropriate.
 #define FOPEN_WRITETEXT "w"
 #endif
 
+/* WinSock destroys recv() buffer when send() failed.
+ * Enabled automatically for Windows and for Cygwin as Cygwin sockets are
+ * wrappers for WinSock sockets. https://github.com/curl/curl/issues/657
+ * Define DONT_USE_RECV_BEFORE_SEND_WORKAROUND to force disable workaround.
+ */
+#if !defined(DONT_USE_RECV_BEFORE_SEND_WORKAROUND)
+#  if defined(WIN32) || defined(__CYGWIN__)
+#    define USE_RECV_BEFORE_SEND_WORKAROUND
+#  endif
+#else  /* DONT_USE_RECV_BEFORE_SEND_WORKAROUNDS */
+#  ifdef USE_RECV_BEFORE_SEND_WORKAROUND
+#    undef USE_RECV_BEFORE_SEND_WORKAROUND
+#  endif
+#endif /* DONT_USE_RECV_BEFORE_SEND_WORKAROUNDS */
+
 #endif /* HEADER_CURL_SETUP_H */

diff --git a/lib/sendf.c b/lib/sendf.c
index a75c5c743c0fcd1b603177c6a508a63fefbf6466..23bcfa2c983e7b0b5fef82c916c2c2234368c94f 100644 (file)
--- a/lib/sendf.c
+++ b/lib/sendf.c
@@ -33,6 +33,7 @@
 #include "non-ascii.h"
 #include "curl_printf.h"
 #include "strerror.h"
+#include "select.h"
 
 /* The last #include files should be: */
 #include "curl_memory.h"
@@ -120,6 +121,90 @@ static size_t convert_lineends(struct SessionHandle *data,
 }
 #endif /* CURL_DO_LINEEND_CONV */
 
+#ifdef USE_RECV_BEFORE_SEND_WORKAROUND
+static void pre_receive_plain(struct connectdata *conn, int num)
+{
+  const curl_socket_t sockfd = conn->sock[num];
+  struct postponed_data * const psnd = &(conn->postponed[num]);
+  size_t bytestorecv = psnd->allocated_size - psnd->recv_size;
+  /* WinSock will destroy unread received data if send() is
+     failed.
+     To avoid lossage of received data, recv() must be
+     performed before every send() if any incoming data is
+     available. However, skip this, if buffer is already full. */
+  if((conn->handler->protocol&PROTO_FAMILY_HTTP) != 0 &&
+     conn->recv[num] == Curl_recv_plain &&
+     (!psnd->buffer || bytestorecv)) {
+    const int readymask = Curl_socket_check(sockfd, CURL_SOCKET_BAD,
+                                            CURL_SOCKET_BAD, 0);
+    if(readymask != -1 && (readymask & CURL_CSELECT_IN) != 0) {
+      /* Have some incoming data */
+      if(!psnd->buffer) {
+        /* Use buffer double default size for intermediate buffer */
+        psnd->allocated_size = 2 * BUFSIZE;
+        psnd->buffer = malloc(psnd->allocated_size);
+        psnd->recv_size = 0;
+        psnd->recv_processed = 0;
+#ifdef DEBUGBUILD
+        psnd->bindsock = sockfd; /* Used only for DEBUGASSERT */
+#endif /* DEBUGBUILD */
+        bytestorecv = psnd->allocated_size;
+      }
+      if(psnd->buffer) {
+        ssize_t recvedbytes;
+        DEBUGASSERT(psnd->bindsock == sockfd);
+        recvedbytes = sread(sockfd, psnd->buffer + psnd->recv_size,
+                            bytestorecv);
+        if(recvedbytes > 0)
+          psnd->recv_size += recvedbytes;
+      }
+      else
+        psnd->allocated_size = 0;
+    }
+  }
+}
+
+static ssize_t get_pre_recved(struct connectdata *conn, int num, char *buf,
+                              size_t len)
+{
+  struct postponed_data * const psnd = &(conn->postponed[num]);
+  size_t copysize;
+  if(!psnd->buffer)
+    return 0;
+
+  DEBUGASSERT(psnd->allocated_size > 0);
+  DEBUGASSERT(psnd->recv_size <= psnd->allocated_size);
+  DEBUGASSERT(psnd->recv_processed <= psnd->recv_size);
+  /* Check and process data that already received and storied in internal
+     intermediate buffer */
+  if(psnd->recv_size > psnd->recv_processed) {
+    DEBUGASSERT(psnd->bindsock == conn->sock[num]);
+    copysize = CURLMIN(len, psnd->recv_size - psnd->recv_processed);
+    memcpy(buf, psnd->buffer + psnd->recv_processed, copysize);
+    psnd->recv_processed += copysize;
+  }
+  else
+    copysize = 0; /* buffer was allocated, but nothing was received */
+
+  /* Free intermediate buffer if it has no unprocessed data */
+  if(psnd->recv_processed == psnd->recv_size) {
+    free(psnd->buffer);
+    psnd->buffer = NULL;
+    psnd->allocated_size = 0;
+    psnd->recv_size = 0;
+    psnd->recv_processed = 0;
+#ifdef DEBUGBUILD
+    psnd->bindsock = CURL_SOCKET_BAD;
+#endif /* DEBUGBUILD */
+  }
+  return (ssize_t)copysize;
+}
+#else  /* ! USE_RECV_BEFORE_SEND_WORKAROUND */
+/* Use "do-nothing" macros instead of functions when workaround not used */
+#define pre_receive_plain(c,n) do {} WHILE_FALSE
+#define get_pre_recved(c,n,b,l) 0
+#endif /* ! USE_RECV_BEFORE_SEND_WORKAROUND */
+
 /* Curl_infof() is for info message along the way */
 
 void Curl_infof(struct SessionHandle *data, const char *fmt, ...)
@@ -255,6 +340,12 @@ ssize_t Curl_send_plain(struct connectdata *conn, int num,
 {
   curl_socket_t sockfd = conn->sock[num];
   ssize_t bytes_written;
+  /* WinSock will destroy unread received data if send() is
+     failed.
+     To avoid lossage of received data, recv() must be
+     performed before every send() if any incoming data is
+     available. */
+  pre_receive_plain(conn, num);
 
 #ifdef MSG_FASTOPEN /* Linux */
   if(conn->bits.tcp_fastopen) {
@@ -322,7 +413,16 @@ ssize_t Curl_recv_plain(struct connectdata *conn, int num, char *buf,
                         size_t len, CURLcode *code)
 {
   curl_socket_t sockfd = conn->sock[num];
-  ssize_t nread = sread(sockfd, buf, len);
+  ssize_t nread;
+  /* Check and return data that already received and storied in internal
+     intermediate buffer */
+  nread = get_pre_recved(conn, num, buf, len);
+  if(nread > 0) {
+    *code = CURLE_OK;
+    return nread;
+  }
+
+  nread = sread(sockfd, buf, len);
 
   *code = CURLE_OK;
   if(-1 == nread) {

diff --git a/lib/url.c b/lib/url.c
index f1028f34cd2b4ba7f677329b24629e20f7840645..184e270497b5fe6107e4d6325d46a526ed38bd86 100644 (file)
--- a/lib/url.c
+++ b/lib/url.c
@@ -2694,6 +2694,43 @@ CURLcode Curl_setopt(struct SessionHandle *data, CURLoption option,
   return result;
 }
 
+#ifdef USE_RECV_BEFORE_SEND_WORKAROUND
+static void conn_reset_postponed_data(struct connectdata *conn, int num)
+{
+  struct postponed_data * const psnd = &(conn->postponed[num]);
+  if(psnd->buffer) {
+    DEBUGASSERT(psnd->allocated_size > 0);
+    DEBUGASSERT(psnd->recv_size <= psnd->allocated_size);
+    DEBUGASSERT(psnd->recv_processed < psnd->recv_size);
+    DEBUGASSERT(psnd->bindsock != CURL_SOCKET_BAD);
+    free(psnd->buffer);
+    psnd->buffer = NULL;
+    psnd->allocated_size = 0;
+    psnd->recv_size = 0;
+    psnd->recv_processed = 0;
+#ifdef DEBUGBUILD
+    psnd->bindsock = CURL_SOCKET_BAD; /* used only for DEBUGASSERT */
+#endif /* DEBUGBUILD */
+  }
+  else {
+    DEBUGASSERT (psnd->allocated_size == 0);
+    DEBUGASSERT (psnd->recv_size == 0);
+    DEBUGASSERT (psnd->recv_processed == 0);
+    DEBUGASSERT (psnd->bindsock == CURL_SOCKET_BAD);
+  }
+}
+
+static void conn_reset_all_postponed_data(struct connectdata *conn)
+{
+  conn_reset_postponed_data(conn, 0);
+  conn_reset_postponed_data(conn, 1);
+}
+#else  /* ! USE_RECV_BEFORE_SEND_WORKAROUND */
+/* Use "do-nothing" macros instead of functions when workaround not used */
+#define conn_reset_postponed_data(c,n) do {} WHILE_FALSE
+#define conn_reset_all_postponed_data(c) do {} WHILE_FALSE
+#endif /* ! USE_RECV_BEFORE_SEND_WORKAROUND */
+
 static void conn_free(struct connectdata *conn)
 {
   if(!conn)
@@ -2744,6 +2781,8 @@ static void conn_free(struct connectdata *conn)
   Curl_safefree(conn->proxy.rawalloc); /* proxy name buffer */
   Curl_safefree(conn->master_buffer);
 
+  conn_reset_all_postponed_data(conn);
+
   Curl_llist_destroy(conn->send_pipe, NULL);
   Curl_llist_destroy(conn->recv_pipe, NULL);
 
@@ -3851,6 +3890,10 @@ static struct connectdata *allocate_conn(struct SessionHandle *data)
   conn->connection_id = -1;    /* no ID */
   conn->port = -1; /* unknown at this point */
   conn->remote_port = -1; /* unknown */
+#if defined(USE_RECV_BEFORE_SEND_WORKAROUND) && defined(DEBUGBUILD)
+  conn->postponed[0].bindsock = CURL_SOCKET_BAD; /* no file descriptor */
+  conn->postponed[1].bindsock = CURL_SOCKET_BAD; /* no file descriptor */
+#endif /* USE_RECV_BEFORE_SEND_WORKAROUND && DEBUGBUILD */
 
   /* Default protocol-independent behavior doesn't support persistent
      connections, so we set this to force-close. Protocols that support
@@ -5625,6 +5668,9 @@ static void reuse_conn(struct connectdata *old_conn,
   /* persist connection info in session handle */
   Curl_persistconninfo(conn);
 
+  conn_reset_all_postponed_data(old_conn); /* free buffers */
+  conn_reset_all_postponed_data(conn);     /* reset unprocessed data */
+
   /* re-use init */
   conn->bits.reuse = TRUE; /* yes, we're re-using here */
 

diff --git a/lib/urldata.h b/lib/urldata.h
index 0167619292a9ff318a8c22416a5afa543cd4062b..f190fbdd63142228d403f09c55f003898a2be1b5 100644 (file)
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -833,6 +833,20 @@ typedef ssize_t (Curl_recv)(struct connectdata *conn, /* connection data */
                             size_t len,               /* max amount to read */
                             CURLcode *err);           /* error to return */
 
+#ifdef USE_RECV_BEFORE_SEND_WORKAROUND
+struct postponed_data {
+  char *buffer;          /* Temporal store for received data during
+                            sending, must be freed */
+  size_t allocated_size; /* Size of temporal store */
+  size_t recv_size;      /* Size of received data during sending */
+  size_t recv_processed; /* Size of processed part of postponed data */
+#ifdef DEBUGBUILD
+  curl_socket_t bindsock;/* Structure must be bound to specific socket,
+                            used only for DEBUGASSERT */
+#endif /* DEBUGBUILD */
+};
+#endif /* USE_RECV_BEFORE_SEND_WORKAROUND */
+
 /*
  * The connectdata struct contains all fields and variables that should be
  * unique for an entire connection.
@@ -931,6 +945,9 @@ struct connectdata {
   Curl_recv *recv[2];
   Curl_send *send[2];
 
+#ifdef USE_RECV_BEFORE_SEND_WORKAROUND
+  struct postponed_data postponed[2]; /* two buffers for two sockets */
+#endif /* USE_RECV_BEFORE_SEND_WORKAROUND */
   struct ssl_connect_data ssl[2]; /* this is for ssl-stuff */
   struct ssl_config_data ssl_config;
   bool tls_upgraded;