HttpWebRequest timeout broken with proxy/ssl, repro attached

[coderb]

running netcoreapp3.1 under win x64.

see https://github.com/coderb/DotnetCore31TimeoutBug

i've isolated a case where HttpWebRequest hangs and does not timeout properly even though Timeout is set.

in this particular case i am tunneling SSL through a proxy (via CONNECT) and the proxy server accepts the connection, completes the SSL handshake, returns the response http headers (but does not include the Content-Length header) and then hangs.

in most other cases the client times out properly (eg if the server hangs prior to ssl handshake or if the server does return the Content-Length header.)

i found this while investigating what i thought were hangs due to ReadWriteTimeout being unsupported #21845 #28404 ( @stephentoub please hear my call ) but this seems to be a distinct bug.

i would recommend auditing the code involved in the handling of timeouts when ssl and or a proxy is used because it seems to be a potentially lesser tested code path.

---

here's the stack on the client when it is not timing out:

 	[Managed to Native Transition]	
 	System.Net.Sockets.dll!System.Net.Sockets.SocketPal.Receive(System.Net.Sockets.SafeSocketHandle handle, System.Span<byte> buffer, System.Net.Sockets.SocketFlags socketFlags, out int bytesTransferred)	Unknown
 	System.Net.Sockets.dll!System.Net.Sockets.Socket.Receive(byte[] buffer, int offset, int size, System.Net.Sockets.SocketFlags socketFlags, out System.Net.Sockets.SocketError errorCode)	Unknown
 	System.Net.Sockets.dll!System.Net.Sockets.NetworkStream.Read(byte[] buffer, int offset, int size)	Unknown
 	System.Net.Http.dll!System.Net.Http.HttpConnection.ReadBuffered(System.Span<byte> destination)	Unknown
 	System.Net.Http.dll!System.Net.Http.HttpConnection.RawConnectionStream.Read(System.Span<byte> buffer)	Unknown
 	System.Net.Http.dll!System.Net.Http.HttpBaseStream.Read(byte[] buffer, int offset, int count)	Unknown
 	System.Net.Security.dll!System.Net.Security.SslStream.FillBufferAsync<System.Net.Security.SslStream.SslReadSync>(System.Net.Security.SslStream.SslReadSync adapter, int minSize)	Unknown
 	System.Net.Security.dll!System.Net.Security.SslStream.ReadAsyncInternal<System.Net.Security.SslStream.SslReadSync>(System.Net.Security.SslStream.SslReadSync adapter, System.Memory<byte> buffer)	Unknown
 	System.Net.Security.dll!System.Net.Security.SslStream.Read(byte[] buffer, int offset, int count)	Unknown
 	System.Private.CoreLib.dll!System.IO.Stream.Read(System.Span<byte> buffer)	Unknown
 	System.Net.Http.dll!System.Net.Http.HttpConnection.Read(System.Span<byte> destination)	Unknown
 	System.Net.Http.dll!System.Net.Http.HttpConnection.ConnectionCloseReadStream.Read(System.Span<byte> buffer)	Unknown
>	BugClient.dll!ConsoleApp1.Program.ReadToMemoryStream(System.IO.Stream stream) Line 33	C#
 	BugClient.dll!ConsoleApp1.Program.Run() Line 22	C#
 	BugClient.dll!ConsoleApp1.Program.Main(string[] args) Line 13	C#

---

and another stack i captured somewhere else (no repro)

 	[Managed to Native Transition]	
 	System.Net.Sockets.dll!System.Net.Sockets.SocketPal.Receive(System.Net.Sockets.SafeSocketHandle handle, System.Span<byte> buffer, System.Net.Sockets.SocketFlags socketFlags, out int bytesTransferred)	Unknown
 	System.Net.Sockets.dll!System.Net.Sockets.Socket.Receive(byte[] buffer, int offset, int size, System.Net.Sockets.SocketFlags socketFlags, out System.Net.Sockets.SocketError errorCode)	Unknown
 	System.Net.Sockets.dll!System.Net.Sockets.NetworkStream.Read(byte[] buffer, int offset, int size)	Unknown
 	System.Net.Http.dll!System.Net.Http.HttpConnection.ReadBuffered(System.Span<byte> destination) Line 2214	C#
 	System.Net.Http.dll!System.Net.Http.HttpConnection.RawConnectionStream.Read(System.Span<byte> buffer) Line 848	C#
 	System.Net.Http.dll!System.Net.Http.HttpBaseStream.Read(byte[] buffer, int offset, int count) Line 107	C#
 	System.Net.Security.dll!System.Net.Security.SslStream.FillBufferAsync<System.Net.Security.SslStream.SslReadSync>(System.Net.Security.SslStream.SslReadSync adapter, int minSize)	Unknown
 	System.Net.Security.dll!System.Net.Security.SslStream.ReadAsyncInternal<System.Net.Security.SslStream.SslReadSync>(System.Net.Security.SslStream.SslReadSync adapter, System.Memory<byte> buffer)	Unknown
 	System.Net.Security.dll!System.Net.Security.SslStream.Read(byte[] buffer, int offset, int count)	Unknown
 	System.Private.CoreLib.dll!System.IO.Stream.Read(System.Span<byte> buffer)	Unknown
 	System.Net.Http.dll!System.Net.Http.HttpConnection.Read(System.Span<byte> destination) Line 2171	C#
 	System.Net.Http.dll!System.Net.Http.HttpConnection.ContentLengthReadStream.Read(System.Span<byte> buffer) Line 535	C#
>	System.Net.Http.dll!System.Net.Http.HttpBaseStream.Read(byte[] buffer, int offset, int count) Line 107	C#

---

System.Net.Http.HttpClient System.Net.HttpWebReqeust

[Dotnet-GitSync-Bot]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

[stephentoub]

BugClient.dll!ConsoleApp1.Program.ReadToMemoryStream

The call stack points to your ReadToMemoryStream method, which is only invoked in your program after GetResponse has returned and the response stream accessed. My understanding is that HttpWebRequest.Timeout is designed to apply up until GetResponse completes:
https://docs.microsoft.com/en-us/dotnet/api/system.net.httpwebrequest.timeout?view=netframework-4.8
"Timeout is the number of milliseconds that a subsequent synchronous request made with the GetResponse method waits for a response"
and that if you wanted GetResponse to wait for the whole response stream before returning, you'd need to set AllowReadStreamBuffering to true. Someone on @dotnet/ncl can correct me if I'm off base here.

[coderb]

@stephentoub it's hard to infer why the client times out in ReadToMemoryStream if the server sends a Content-Length header but there is no timeout otherwise and how this would be "per spec". But also as I understand what you are saying, the client really should hang on the subsequent stream Read due again to lack of support for ReadWriteTimeout.

Sorry, but could you remind me one more time why it is so hard to add the missing calls to set the read/write timeout on NetworkStream.ReadTimeout/WriteTimeout HttpConnection.socket (via ioctl SetSocketOption) for dotnet core?

This really is causing me major headaches. I've spent 2 weeks on a rather large solution adding async code paths but ensuring that no synchronous network reads/writes are ever done is a herculean task.

[coderb]

well, since it looks like this issue is set to fester, here's a workaround for those unlucky future travelers who step on this landmine:

        public Stream GetResponseStream(HttpWebResponse resp, int timeoutMillis) {
            var stream = resp.GetResponseStream();
#if NETCOREAPP3_1
            UnbuggerStream(stream, timeoutMillis);
#endif
            return stream;
        }

        private static void UnbuggerStream(Stream stream, int timeoutMillis) {
            if (stream is GZipStream stream2) {
                stream = stream2.BaseStream;
            }
            {
                var fullName = stream.GetType().FullName;
                if (fullName == "System.Net.Http.EmptyReadStream") {
                    return;
                }

                if (
                    fullName == "System.Net.Http.HttpConnection+ChunkedEncodingReadStream" || 
                    fullName == "System.Net.Http.HttpConnection+ConnectionCloseReadStream" ||
                    fullName == "System.Net.Http.HttpConnection+ContentLengthReadStream"
                ) {
                    var _connection = GetFieldNonPublic<object>(stream, "_connection");
                    if (_connection != null) {
                        var _socket = GetFieldNonPublic<Socket>(_connection, "_socket");
                        if (_socket != null) {
                            _socket.ReceiveTimeout = timeoutMillis;
                            _socket.SendTimeout = timeoutMillis;
                            return;
                        }

                        // try proxy structure
                        var _stream = GetFieldNonPublic<Stream>(_connection, "_stream");
                        var innerStream = GetFieldNonPublic<Stream>(_stream, "_innerStream");
                        if (innerStream != null) {
                            _stream = innerStream;
                        }
                        _connection = GetFieldNonPublic<object>(_stream, "_connection");
                        _socket = GetFieldNonPublic<Socket>(_connection, "_socket");
                        if (_socket != null) {
                            _socket.ReceiveTimeout = timeoutMillis;
                            _socket.SendTimeout = timeoutMillis;
                            return;
                        }
                    }
                }
            }

            // working as of netcoreapp3.1, will likely need fixes against other runtimes
            throw new NotImplementedException();
        }

        private static T GetFieldNonPublic<T>(object obj, string fieldName) {
            var fi = obj.GetType().GetField(fieldName, BindingFlags.GetField | BindingFlags.Instance | BindingFlags.NonPublic);
            var result = fi.GetValue(obj);
            return (T)result;
        }

[wfurt]

Is there anything left to do here @stephentoub? (besides discouraging HttpWebRequest usage)

[coderb]

@wfurt fix the bug that ReadWriteTimeout is not implemented or provide a logical rationale as to why it should not be?

[coderb]

@wfurt the problem occurs with HttpClient as well as HttpWebRequest. Perhaps you want to deprecate all synchronous i/o calls in .net since you are refusing to support an existing API that is fundamentally required for reliable synchronous networking.

[wfurt]

Perhaps we should improve documentation and clarify HttpClient.Timeout better. Changing scope of the Timeout to cover response stream would be breaking change IMHO. Similar issues were raised before: people want absolute timeouts or timeouts for cases when server stop sending response e.g. idle timeout.
This should perhaps wait for triage.

[coderb]

@wfurt This is not a documentation problem. If I am downloading a 2GB file my timeout for how long the whole process takes to complete has no relation to how long of a pause between network packets I am will to tolerate. Dont reinvent the wheel. ReadWriteTimeout is a simple call on the underlying socket. I'm at a loss as to why you refuse to make it from HttpWebRequest.

[karelz]

Triage: Given that this is in sync paths which are lower priority, and which are very difficult to change and make work consistently across all cases (HTTP versions and platforms), we should wait for more feedback from more customers to see if it has priority among larger customer base.