curl_setup: Disable by default recv-before-send in Windows

Prior to this change a workaround for Windows to recv before every send
was enabled by default. The way it works is a recv is called before
every send and saves the received data, in case send fails because in
Windows apparently that can wipe out the socket's internal received
data buffer.

This feature has led to several bugs because the way libcurl operates
it waits on a socket to read or to write, and may not at all times
check for buffered receive data.

Two recent significant bugs this workaround caused:
- Broken Schannel TLS 1.3 connections (#9431)
- HTTP/2 arbitrary hangs (#10253)

The actual code remains though it is disabled by default. Though future
changes to connection filter buffering could improve the situation IMO
it's just not tenable to manage this workaround.

Ref: https://github.com/curl/curl/issues/657
Ref: https://github.com/curl/curl/pull/668
Ref: https://github.com/curl/curl/pull/720

Ref: https://github.com/curl/curl/issues/9431
Ref: https://github.com/curl/curl/issues/10253

Closes https://github.com/curl/curl/pull/10409

diff --git a/lib/curl_setup.h b/lib/curl_setup.h
index 0aa4f4d7468187971964b4a4cd76a83155a75368..2eb9697fd1a99a53a970c944a06770a391c86dd5 100644 (file)
--- a/lib/curl_setup.h
+++ b/lib/curl_setup.h
@@ -777,14 +777,16 @@ endings either CRLF or LF so 't' is appropriate.
 #define FOPEN_APPENDTEXT "a"
 #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.
+/* 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
+/* #    define USE_RECV_BEFORE_SEND_WORKAROUND */
 #  endif
 #else  /* DONT_USE_RECV_BEFORE_SEND_WORKAROUND */
 #  ifdef USE_RECV_BEFORE_SEND_WORKAROUND

diff --git a/tests/data/test1517 b/tests/data/test1517
index 5150ff6edf79a1dc7c21aa8f3a6ca6857eb6d1bb..bc663ab63acde3b93f9dedbf2f1fc83b0c96c62f 100644 (file)
--- a/tests/data/test1517
+++ b/tests/data/test1517
@@ -9,7 +9,11 @@ early response
 </keywords>
 </info>
 #
-# This reproduces issue #657, fixed with PR #668 - on Windows
+# 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).
 #
 # Server-side
 <reply>
@@ -39,6 +43,11 @@ http
 <tool>
 lib%TESTNUMBER
 </tool>
+# precheck is a command line to run before the test, to see if we can execute
+# the test or not
+<precheck>
+./libtest/lib%TESTNUMBER check
+</precheck>
 
  <name>
 HTTP POST, server responds before completed send

diff --git a/tests/libtest/lib1517.c b/tests/libtest/lib1517.c
index dc6a247a69e5a6f38d8ab3343bf3ef81fbe7305c..f3961659414e1b25fb4130a9edf8c929bbb7e22e 100644 (file)
--- a/tests/libtest/lib1517.c
+++ b/tests/libtest/lib1517.c
@@ -60,6 +60,16 @@ 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");
+    return 1; /* skip since test will fail on Windows without workaround */
+#else
+    return 0; /* sure, run this! */
+#endif
+  }
+
   pooh.readptr = data;
   pooh.sizeleft = strlen(data);