From 8ce56f76d9159a65ddf483587e30945899d125fc Mon Sep 17 00:00:00 2001 From: Rurido Date: Mon, 16 Aug 2021 07:49:22 +0200 Subject: [PATCH 1/3] Added host header to proxy connect request in order to be compliant with HTTP 1.1 (#877) Co-authored-by: RR55 --- .../Network/TcpConnection/TcpConnectionFactory.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Titanium.Web.Proxy/Network/TcpConnection/TcpConnectionFactory.cs b/src/Titanium.Web.Proxy/Network/TcpConnection/TcpConnectionFactory.cs index 429e1dadc..5d1c433fb 100644 --- a/src/Titanium.Web.Proxy/Network/TcpConnection/TcpConnectionFactory.cs +++ b/src/Titanium.Web.Proxy/Network/TcpConnection/TcpConnectionFactory.cs @@ -520,15 +520,17 @@ internal Task GetServerConnection(ProxyServer proxyServer, if (externalProxy != null && externalProxy.ProxyType == ExternalProxyType.Http && (isConnect || isHttps)) { - var authority = $"{remoteHostName}:{remotePort}".GetByteString(); - var connectRequest = new ConnectRequest(authority) + var authority = $"{remoteHostName}:{remotePort}"; + var authorityBytes = authority.GetByteString(); + var connectRequest = new ConnectRequest(authorityBytes) { IsHttps = isHttps, - RequestUriString8 = authority, + RequestUriString8 = authorityBytes, HttpVersion = httpVersion }; connectRequest.Headers.AddHeader(KnownHeaders.Connection, KnownHeaders.ConnectionKeepAlive); + connectRequest.Headers.AddHeader(KnownHeaders.Host, authority); if (!string.IsNullOrEmpty(externalProxy.UserName) && externalProxy.Password != null) { From d56086deeb37ec019af021235f6f0ff0df4e57bf Mon Sep 17 00:00:00 2001 From: justcoding121 Date: Mon, 16 Aug 2021 00:30:07 -0600 Subject: [PATCH 2/3] Increase default cache connection count --- src/Titanium.Web.Proxy/ProxyServer.cs | 4 ++-- .../NestedProxyTests.cs | 7 +++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/Titanium.Web.Proxy/ProxyServer.cs b/src/Titanium.Web.Proxy/ProxyServer.cs index 377e31d6a..938691a04 100644 --- a/src/Titanium.Web.Proxy/ProxyServer.cs +++ b/src/Titanium.Web.Proxy/ProxyServer.cs @@ -208,9 +208,9 @@ public ProxyServer(string? rootCertificateName, string? rootCertificateIssuerNam /// /// Maximum number of concurrent connections per remote host in cache. /// Only valid when connection pooling is enabled. - /// Default value is 2. + /// Default value is 4. /// - public int MaxCachedConnections { get; set; } = 2; + public int MaxCachedConnections { get; set; } = 4; /// /// Number of seconds to linger when Tcp connection is in TIME_WAIT state. diff --git a/tests/Titanium.Web.Proxy.IntegrationTests/NestedProxyTests.cs b/tests/Titanium.Web.Proxy.IntegrationTests/NestedProxyTests.cs index 84d23eadc..49d7aa3f9 100644 --- a/tests/Titanium.Web.Proxy.IntegrationTests/NestedProxyTests.cs +++ b/tests/Titanium.Web.Proxy.IntegrationTests/NestedProxyTests.cs @@ -195,14 +195,14 @@ public async Task Nested_Proxy_Farm_With_Connection_Cache_Should_Not_Hang() var proxies2 = new List(); //create a level 2 upstream proxy farm that forwards to server - for (int i = 0; i < 10; i++) + for (int i = 0; i < 2; i++) { var proxy2 = testSuite.GetProxy(); proxy2.ProxyBasicAuthenticateFunc += (_, _, _) => { return Task.FromResult(true); }; - + proxy2.MaxCachedConnections = 100; proxies2.Add(proxy2); } @@ -212,9 +212,8 @@ public async Task Nested_Proxy_Farm_With_Connection_Cache_Should_Not_Hang() for (int i = 0; i < 10; i++) { var proxy1 = testSuite.GetProxy(); - //proxy1.EnableConnectionPool = false; var proxy2 = proxies2[rnd.Next() % proxies2.Count]; - + proxy1.MaxCachedConnections = 100; var explicitEndpoint = proxy1.ProxyEndPoints.OfType().First(); explicitEndpoint.BeforeTunnelConnectRequest += (_, e) => { From 4d71be5bb7d99f0097ccf60b974f8913fa12492c Mon Sep 17 00:00:00 2001 From: justcoding121 Date: Tue, 17 Aug 2021 17:23:57 -0600 Subject: [PATCH 3/3] #826 Fix retry logic --- ... => RetryableServerConnectionException.cs} | 9 +++---- src/Titanium.Web.Proxy/Network/RetryPolicy.cs | 8 +++--- .../Network/Streams/HttpServerStream.cs | 26 +++++++------------ .../Network/Streams/HttpStream.cs | 17 ++++++++++-- src/Titanium.Web.Proxy/ProxyServer.cs | 2 +- src/Titanium.Web.Proxy/RequestHandler.cs | 9 +++++-- .../NestedProxyTests.cs | 4 +-- 7 files changed, 41 insertions(+), 34 deletions(-) rename src/Titanium.Web.Proxy/Exceptions/{ServerConnectionException.cs => RetryableServerConnectionException.cs} (51%) diff --git a/src/Titanium.Web.Proxy/Exceptions/ServerConnectionException.cs b/src/Titanium.Web.Proxy/Exceptions/RetryableServerConnectionException.cs similarity index 51% rename from src/Titanium.Web.Proxy/Exceptions/ServerConnectionException.cs rename to src/Titanium.Web.Proxy/Exceptions/RetryableServerConnectionException.cs index d885c74e7..ad07df990 100644 --- a/src/Titanium.Web.Proxy/Exceptions/ServerConnectionException.cs +++ b/src/Titanium.Web.Proxy/Exceptions/RetryableServerConnectionException.cs @@ -3,12 +3,12 @@ namespace Titanium.Web.Proxy.Exceptions { /// - /// The server connection was closed upon first read with the new connection from pool. + /// The server connection was closed upon first write with the new connection from pool. /// Should retry the request with a new connection. /// - public class ServerConnectionException : ProxyException + public class RetryableServerConnectionException : ProxyException { - internal ServerConnectionException(string message) : base(message) + internal RetryableServerConnectionException(string message) : base(message) { } @@ -17,9 +17,8 @@ internal ServerConnectionException(string message) : base(message) /// /// /// - internal ServerConnectionException(string message, Exception e) : base(message, e) + internal RetryableServerConnectionException(string message, Exception e) : base(message, e) { } - } } diff --git a/src/Titanium.Web.Proxy/Network/RetryPolicy.cs b/src/Titanium.Web.Proxy/Network/RetryPolicy.cs index a2f724554..19863cba0 100644 --- a/src/Titanium.Web.Proxy/Network/RetryPolicy.cs +++ b/src/Titanium.Web.Proxy/Network/RetryPolicy.cs @@ -35,14 +35,12 @@ internal async Task ExecuteAsync(Func ReadResponseStatus(CancellationToken cancellationToken = default) { - try - { - string httpStatus = await ReadLineAsync(cancellationToken) ?? - throw new ServerConnectionException("Server connection was closed."); - - if (httpStatus == string.Empty) - { - // is this really possible? - httpStatus = await ReadLineAsync(cancellationToken) ?? - throw new ServerConnectionException("Server connection was closed. Response status is empty."); - } + string httpStatus = await ReadLineAsync(cancellationToken) ?? + throw new IOException("Invalid http status code."); - Response.ParseResponseLine(httpStatus, out var version, out int statusCode, out string description); - return new ResponseStatusInfo { Version = version, StatusCode = statusCode, Description = description }; - } - catch (Exception e) when (!(e is ServerConnectionException)) + if (httpStatus == string.Empty) { - throw new ServerConnectionException("Server connection was closed. Exception while reading the response status.", e); + // is this really possible? + httpStatus = await ReadLineAsync(cancellationToken) ?? + throw new IOException("Response status is empty."); } + + Response.ParseResponseLine(httpStatus, out var version, out int statusCode, out string description); + return new ResponseStatusInfo { Version = version, StatusCode = statusCode, Description = description }; + } } } diff --git a/src/Titanium.Web.Proxy/Network/Streams/HttpStream.cs b/src/Titanium.Web.Proxy/Network/Streams/HttpStream.cs index 3f4e2f75b..9e38996ee 100644 --- a/src/Titanium.Web.Proxy/Network/Streams/HttpStream.cs +++ b/src/Titanium.Web.Proxy/Network/Streams/HttpStream.cs @@ -932,7 +932,21 @@ public ValueTask WriteLineAsync(string value, CancellationToken cancellationToke internal async Task WriteHeadersAsync(HeaderBuilder headerBuilder, CancellationToken cancellationToken = default) { var buffer = headerBuilder.GetBuffer(); - await WriteAsync(buffer.Array, buffer.Offset, buffer.Count, true, cancellationToken); + + try + { + await WriteAsync(buffer.Array, buffer.Offset, buffer.Count, true, cancellationToken); + } + catch (IOException e) + { + //throw this as ServerConnectionException so that RetryPolicy can retry with a new server connection. + if (this is HttpServerStream) + { + throw new RetryableServerConnectionException("Server connection was closed. Exception while sending request line and headers.", e); + } + + throw; + } } /// @@ -1226,7 +1240,6 @@ private async Task copyBytesToStream(IHttpStreamWriter writer, long count, bool protected async ValueTask WriteAsync(RequestResponseBase requestResponse, HeaderBuilder headerBuilder, CancellationToken cancellationToken = default) { var body = requestResponse.CompressBodyAndUpdateContentLength(); - headerBuilder.WriteHeaders(requestResponse.Headers); await WriteHeadersAsync(headerBuilder, cancellationToken); diff --git a/src/Titanium.Web.Proxy/ProxyServer.cs b/src/Titanium.Web.Proxy/ProxyServer.cs index 938691a04..a0bca42c0 100644 --- a/src/Titanium.Web.Proxy/ProxyServer.cs +++ b/src/Titanium.Web.Proxy/ProxyServer.cs @@ -123,7 +123,7 @@ public ProxyServer(string? rootCertificateName, string? rootCertificateIssuerNam /// /// Number of times to retry upon network failures when connection pool is enabled. /// - public int NetworkFailureRetryAttempts { get; set; } = 0; + public int NetworkFailureRetryAttempts { get; set; } = 1; /// /// Is the proxy currently running? diff --git a/src/Titanium.Web.Proxy/RequestHandler.cs b/src/Titanium.Web.Proxy/RequestHandler.cs index 103f309f9..005c2daa7 100644 --- a/src/Titanium.Web.Proxy/RequestHandler.cs +++ b/src/Titanium.Web.Proxy/RequestHandler.cs @@ -288,8 +288,13 @@ private async Task handleHttpSessionRequest(SessionEventArgs args, noCache, cancellationToken); - // for connection pool, retry fails until cache is exhausted. - return await retryPolicy().ExecuteAsync(async connection => + /// Retry with new connection if the initial stream.WriteAsync call to server fails. + /// i.e if request line and headers failed to get send. + /// Do not retry after reading data from client stream, + /// because subsequent try will not have data to read from client + /// and will hang at clientStream.ReadAsync call. + /// So, throw RetryableServerConnectionException only when we are sure we can retry safely. + return await retryPolicy().ExecuteAsync(async connection => { // set the connection and send request headers args.HttpClient.SetConnection(connection); diff --git a/tests/Titanium.Web.Proxy.IntegrationTests/NestedProxyTests.cs b/tests/Titanium.Web.Proxy.IntegrationTests/NestedProxyTests.cs index 49d7aa3f9..5a95aa8fd 100644 --- a/tests/Titanium.Web.Proxy.IntegrationTests/NestedProxyTests.cs +++ b/tests/Titanium.Web.Proxy.IntegrationTests/NestedProxyTests.cs @@ -195,14 +195,13 @@ public async Task Nested_Proxy_Farm_With_Connection_Cache_Should_Not_Hang() var proxies2 = new List(); //create a level 2 upstream proxy farm that forwards to server - for (int i = 0; i < 2; i++) + for (int i = 0; i < 10; i++) { var proxy2 = testSuite.GetProxy(); proxy2.ProxyBasicAuthenticateFunc += (_, _, _) => { return Task.FromResult(true); }; - proxy2.MaxCachedConnections = 100; proxies2.Add(proxy2); } @@ -213,7 +212,6 @@ public async Task Nested_Proxy_Farm_With_Connection_Cache_Should_Not_Hang() { var proxy1 = testSuite.GetProxy(); var proxy2 = proxies2[rnd.Next() % proxies2.Count]; - proxy1.MaxCachedConnections = 100; var explicitEndpoint = proxy1.ProxyEndPoints.OfType().First(); explicitEndpoint.BeforeTunnelConnectRequest += (_, e) => {