From: Stefan Eissing <stefan@eissing.org>
Date: Tue, 16 May 2023 08:24:07 +0000 (+0200)
Subject: cf-socket: completely remove the disabled USE_RECV_BEFORE_SEND_WORKAROUND
X-Git-Tag: curl-8_1_1~31
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=408eb87bb3fa042120ff1805702346f4c9eb1a52;p=thirdparty%2Fcurl.git

cf-socket: completely remove the disabled USE_RECV_BEFORE_SEND_WORKAROUND

Closes #11118
---

diff --git a/lib/cf-socket.c b/lib/cf-socket.c
index 0ad918bef9..55f1e06f4b 100644
--- a/lib/cf-socket.c
+++ b/lib/cf-socket.c
@@ -1235,20 +1235,6 @@ static ssize_t cf_socket_send(struct Curl_cfilter *cf, struct Curl_easy *data,
   curl_socket_t fdsave;
   ssize_t nwritten;
 
-#ifdef USE_RECV_BEFORE_SEND_WORKAROUND
-  /* 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. */
-  if(ctx->buffer_recv && !Curl_bufq_is_full(&ctx->recvbuf)) {
-    nwritten = Curl_bufq_slurp(&ctx->recvbuf, nw_in_read, &rctx, err);
-    if(nwritten < 0 && *err != CURLE_AGAIN) {
-      return -1;
-    }
-  }
-#endif
-
   *err = CURLE_OK;
   fdsave = cf->conn->sock[cf->sockindex];
   cf->conn->sock[cf->sockindex] = ctx->sock;
@@ -1405,20 +1391,11 @@ static void cf_socket_active(struct Curl_cfilter *cf, struct Curl_easy *data)
     conn_set_primary_ip(cf, data);
     set_local_ip(cf, data);
     Curl_persistconninfo(data, cf->conn, ctx->l_ip, ctx->l_port);
-    /* We buffer only for TCP transfers that do not install their own read
-     * function. Those may still have expectations about socket behaviours from
-     * the past.
-     *
-     * Note buffering is currently disabled by default because we have stalls
+    /* buffering is currently disabled by default because we have stalls
      * in parallel transfers where not all buffered data is consumed and no
      * socket events happen.
      */
-#ifdef USE_RECV_BEFORE_SEND_WORKAROUND
-    ctx->buffer_recv = (ctx->transport == TRNSPRT_TCP &&
-                        (cf->conn->recv[cf->sockindex] == Curl_conn_recv));
-#else
     ctx->buffer_recv = FALSE;
-#endif
   }
   ctx->active = TRUE;
 }
diff --git a/lib/curl_setup.h b/lib/curl_setup.h
index ef30aa1fbe..38cf6ffc12 100644
--- a/lib/curl_setup.h
+++ b/lib/curl_setup.h
@@ -777,23 +777,6 @@ endings either CRLF or LF so 't' is appropriate.
 #define FOPEN_APPENDTEXT "a"
 #endif
 
-/* Windows workaround to recv before every send, because apparently Winsock
- * destroys destroys recv() buffer when send() failed.
- * This workaround is now disabled by default since it caused hard to fix bugs.
- * Define USE_RECV_BEFORE_SEND_WORKAROUND to enable it.
- * https://github.com/curl/curl/issues/657
- * https://github.com/curl/curl/pull/10409
- */
-#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_WORKAROUND */
-#  ifdef USE_RECV_BEFORE_SEND_WORKAROUND
-#    undef USE_RECV_BEFORE_SEND_WORKAROUND
-#  endif
-#endif /* DONT_USE_RECV_BEFORE_SEND_WORKAROUND */
-
 /* for systems that don't detect this in configure */
 #ifndef CURL_SA_FAMILY_T
 #  if defined(HAVE_SA_FAMILY_T)
diff --git a/tests/data/test1517 b/tests/data/test1517
index bc663ab63a..539dd6208f 100644
--- a/tests/data/test1517
+++ b/tests/data/test1517
@@ -11,9 +11,9 @@ early response
 #
 # This test checks to make sure curl can call recv() without failing after a
 # send() fails on the same socket (#657). Most OSes should support this
-# natively but on Windows curl must be built with a workaround (#668) for the
-# test to succeed. The precheck will skip this test on Windows if curl was
-# built without the workaround (USE_RECV_BEFORE_SEND_WORKAROUND isn't defined).
+# natively. On Windows there were reported problems and a workaround via
+# define USE_RECV_BEFORE_SEND_WORKAROUND was added.
+# This proved problematic in other cases and was removed again.
 #
 # Server-side
 <reply>
diff --git a/tests/http/test_07_upload.py b/tests/http/test_07_upload.py
index 5b4c1d8a86..6b8f3dbc4d 100644
--- a/tests/http/test_07_upload.py
+++ b/tests/http/test_07_upload.py
@@ -227,6 +227,21 @@ class TestUpload:
             respdata = open(curl.response_file(i)).readlines()
             assert respdata == exp_data
 
+    # issue #10591
+    @pytest.mark.parametrize("proto", ['http/1.1', 'h2', 'h3'])
+    def test_07_32_issue_10591(self, env: Env, httpd, nghttpx, repeat, proto):
+        if proto == 'h3' and not env.have_h3():
+            pytest.skip("h3 not supported")
+        if proto == 'h3' and env.curl_uses_lib('msh3'):
+            pytest.skip("msh3 fails here")
+        fdata = os.path.join(env.gen_dir, 'data-10m')
+        count = 1
+        curl = CurlClient(env=env)
+        url = f'https://{env.authority_for(env.domain1, proto)}/curltest/put?id=[0-{count-1}]'
+        r = curl.http_put(urls=[url], fdata=fdata, alpn_proto=proto)
+        r.check_response(count=count, http_status=200)
+        r.check_response(count=count, http_status=200)
+
     def check_download(self, count, srcfile, curl):
         for i in range(count):
             dfile = curl.download_file(i)
diff --git a/tests/libtest/lib1517.c b/tests/libtest/lib1517.c
index f396165941..706b5567c9 100644
--- a/tests/libtest/lib1517.c
+++ b/tests/libtest/lib1517.c
@@ -61,9 +61,9 @@ int test(char *URL)
   struct WriteThis pooh;
 
   if(!strcmp(URL, "check")) {
-#if (defined(WIN32) || defined(__CYGWIN__)) && \
-    !defined(USE_RECV_BEFORE_SEND_WORKAROUND)
-    printf("test requires recv-before-send workaround on Windows\n");
+#if (defined(WIN32) || defined(__CYGWIN__))
+    printf("Windows TCP does not deliver response data but reports "
+           "CONNABORTED\n");
     return 1; /* skip since test will fail on Windows without workaround */
 #else
     return 0; /* sure, run this! */