Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from

Conversation

jay
Copy link
Member

@jay jay commented Feb 3, 2023

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:

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 behavior, though it could possibly be handled more correctly at a higher level (eg transfer handling checks for server error response before it has completed sending its request).

Ref: #9431
Ref: #10253

Closes #xxxx


/cc @Karlson2k

@jay jay added the Windows Windows-specific label Feb 3, 2023
@jay jay force-pushed the disable_recv-before-send branch from 2154327 to a5c1b05 Compare February 3, 2023 08:13
@Karlson2k
Copy link
Contributor

Maybe just disable recv-before-send for the handshakes and use it for the data transfers?
It should be easy to implement and better than complete disable of this feature.

@Karlson2k
Copy link
Contributor

Of the other hand, this workaround was implemented for HTTP servers not following RFC requirements: https://www.rfc-editor.org/rfc/rfc9112#section-9.6-9
In the ideal world everything should work without this workaround, but in practice disabling it may lower compatibility with existing servers.

@jay jay force-pushed the disable_recv-before-send branch from a5c1b05 to 99ce088 Compare February 7, 2023 07:29
@jay jay force-pushed the disable_recv-before-send branch from 99ce088 to 668468c Compare February 7, 2023 07:31
@jay jay force-pushed the disable_recv-before-send branch from 668468c to 3a41ab5 Compare February 8, 2023 03:21
@jay jay force-pushed the disable_recv-before-send branch from 3a41ab5 to 36ce18f Compare February 8, 2023 08:28
@jay jay force-pushed the disable_recv-before-send branch from 36ce18f to aee6960 Compare February 8, 2023 08:33
@jay jay force-pushed the disable_recv-before-send branch from aee6960 to 53b32b0 Compare February 8, 2023 08:36
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 (curl#9431)
- HTTP/2 arbitrary hangs (curl#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: curl#657
Ref: curl#668
Ref: curl#720

Ref: curl#9431
Ref: curl#10253

Closes curl#10409
@jay jay force-pushed the disable_recv-before-send branch from 53b32b0 to c2b2799 Compare February 8, 2023 08:41
@jay
Copy link
Member Author

jay commented Feb 9, 2023

In the ideal world everything should work without this workaround, but in practice disabling it may lower compatibility with existing servers.

Thanks for your feedback. Yes, unfortunately premature server responses may be lost on Windows if they're followed by RST.

@jay jay closed this in b4b6e4f Feb 9, 2023
@jay jay deleted the disable_recv-before-send branch February 9, 2023 06:34
bch pushed a commit to bch/curl that referenced this pull request Jul 19, 2023
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 (curl#9431)
- HTTP/2 arbitrary hangs (curl#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: curl#657
Ref: curl#668
Ref: curl#720

Ref: curl#9431
Ref: curl#10253

Closes curl#10409
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Windows Windows-specific
Development

Successfully merging this pull request may close these issues.

2 participants