dotnet / dotnet/aspnetcore

Incorrect socket close causes data truncation on NetFx

Open
#13,219 3 comments 0 reactions 0 assignees View on GitHub
affected-very-few area-networking bug feature-kestrel severity-nice-to-have
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 6h
Merged PRs (30d)
290

Description

### 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](https://github.com/CoreWCF/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

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.