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

Incorrect socket close causes data truncation on NetFx #13219

Open
mconnew opened this issue Aug 17, 2019 · 3 comments
Open

Incorrect socket close causes data truncation on NetFx #13219

mconnew opened this issue Aug 17, 2019 · 3 comments
Labels
affected-very-few This issue impacts very few customers area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions bug This issue describes a behavior which is not expected - a bug. feature-kestrel severity-nice-to-have This label is used by an internal tool
Milestone

Comments

@mconnew
Copy link
Member

mconnew commented Aug 17, 2019

Describe the bug

When using a custom connection handler, when closing the connection by calling Complete on the input and output pipes, the method AspNetCore uses to close the socket can lead to some bytes which were "completed" on the output pipe to not be sent before the socket is closed. This only happens on NetFx and doesn't occur on .NET Core.

CoreWCF implements the WCF Net.Tcp framing protocol. As part of this protocol, when closing the connection a final single framing FIN byte is sent. Then the connection is closed. Due to NetFx having a different way how it closes the socket, that final FIN byte might not get sent. It's very timing sensitive as insertion of a small delay before closing results in the final byte being sent.

AspNetCore closes the socket by calling Socket.Shutdown(SocketShutdown.Both), then calls Dispose on the socket. The correct sequence is:

Socket.Shutdown(SocketShutdown.Send);
int bytesRead = Socket.Receive(dummy, 0, 1, SocketFlags.None); // dummy is a buffer of at least 1 byte. Also need a timeout configures
if (bytesRead > 0)
{
Socket.Abort();
// Potentially throw of log
}
else
{
Socket.Close(); // Preferably with a timeout
}

To Reproduce

Steps to reproduce the behavior:

  1. Using this version of ASP.NET Core: I've reproduced on 2.1 and 2.2. 3.0 doesn't run on NetFx'
  2. Run this code - complicated as discovered as part of CoreWCF development. I Could create a branch without my work around (delay) which you could run the unit tests to validate the fix. If this is something which would be useful, let me know.

Expected behavior

All bytes sent to be received by remote party

@halter73
Copy link
Member

This is something we should look at doing again in future releases if we can prove that not doing this can cause truncation on .NET core. I doubt we would patch for this. If we were to consider this for 2.1, it would have to behind an AppContextSwitch since this change would be pretty significant.

@halter73 halter73 added this to the 5.0.0 milestone Aug 20, 2019
@mconnew
Copy link
Member Author

mconnew commented Aug 20, 2019

The Close code in Socket is different in NetFx and .NET Core. The Socket.Close code in .NET Core looks like it wouldn't cause this problem.

@ghost
Copy link

ghost commented Jul 27, 2020

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@jkotalik jkotalik added affected-very-few This issue impacts very few customers bug This issue describes a behavior which is not expected - a bug. severity-nice-to-have This label is used by an internal tool labels Nov 13, 2020 — with ASP.NET Core Issue Ranking
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected-very-few This issue impacts very few customers area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions bug This issue describes a behavior which is not expected - a bug. feature-kestrel severity-nice-to-have This label is used by an internal tool
Projects
None yet
Development

No branches or pull requests

6 participants