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.

Sign up for GitHub

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

Fix file upload HTTP client not handling DNS changes #3266

Merged
merged 1 commit into from
Apr 13, 2023
Merged

Fix file upload HTTP client not handling DNS changes #3266

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion iothub/device/src/Transport/Http/HttpClientHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,22 @@ internal sealed class HttpClientHelper : IHttpClientHelper
private readonly ProductInfo _productInfo;
private readonly bool _isClientPrimaryTransportHandler;

// These default values are consistent with Azure.Core default values:
// https://github.com/Azure/azure-sdk-for-net/blob/7e3cf643977591e9041f4c628fd4d28237398e0b/sdk/core/Azure.Core/src/Pipeline/ServicePointHelpers.cs#L28
private const int DefaultMaxConnectionsPerServer = 50;

// How long, in milliseconds, a given cached TCP connection created by this client's HTTP layer will live before being closed.
// If this value is set to any negative value, the connection lease will be infinite. If this value is set to 0, then the TCP connection will close after
// each HTTP request and a new TCP connection will be opened upon the next request.
//
// By closing cached TCP connections and opening a new one upon the next request, the underlying HTTP client has a chance to do a DNS lookup
// to validate that it will send the requests to the correct IP address. While it is atypical for a given IoT hub to change its IP address, it does
// happen when a given IoT hub fails over into a different region.
//
// This default value is consistent with the default value used in Azure.Core
// https://github.com/Azure/azure-sdk-for-net/blob/7e3cf643977591e9041f4c628fd4d28237398e0b/sdk/core/Azure.Core/src/Pipeline/ServicePointHelpers.cs#L29
private static readonly TimeSpan s_defaultConnectionLeaseTimeout = TimeSpan.FromMinutes(5);

public HttpClientHelper(
Uri baseAddress,
IAuthorizationProvider authenticationHeaderProvider,
Expand All @@ -59,6 +75,9 @@ public HttpClientHelper(
_authenticationHeaderProvider = authenticationHeaderProvider;
_defaultErrorMapping = new ReadOnlyDictionary<HttpStatusCode, Func<HttpResponseMessage, Task<Exception>>>(defaultErrorMapping);

ServicePoint servicePoint = ServicePointManager.FindServicePoint(_baseAddress);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this also a work around a user could make in their own app if they are using an SDK that doesn't do this?

servicePoint.ConnectionLeaseTimeout = s_defaultConnectionLeaseTimeout.Milliseconds;

#if NET451
TlsVersions.Instance.SetLegacyAcceptableVersions();

Expand Down Expand Up @@ -105,6 +124,8 @@ public HttpClientHelper(
_httpClientHandler.Proxy = proxy;
}

_httpClientHandler.MaxConnectionsPerServer = DefaultMaxConnectionsPerServer;

_httpClientObj = new HttpClient(_httpClientHandler);
#endif

Expand Down Expand Up @@ -594,5 +615,5 @@ private static async Task<T> ReadAsAsync<T>(HttpContent content, CancellationTok
}

#endif
}
}
}