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

@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 added a commit to jay/curl that referenced this pull request Feb 7, 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#9431
Ref: curl#10253

Closes curl#10409
jay added a commit to jay/curl that referenced this pull request Feb 7, 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
jay added a commit to jay/curl that referenced this pull request Feb 8, 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
jay added a commit to jay/curl that referenced this pull request Feb 8, 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
jay added a commit to jay/curl that referenced this pull request Feb 8, 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
jay added a commit to jay/curl that referenced this pull request Feb 8, 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
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
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.

None yet

2 participants