-
Notifications
You must be signed in to change notification settings - Fork 82
HttpWebRequest and WebResponse To HttpClient #1018
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
base: MAJOR.migrate-to-httpclient
Are you sure you want to change the base?
Changes from 1 commit
e71969c
888ca7c
8900fd8
092d572
5d56ad3
39ff74d
e6a1476
8afd20f
523ca49
d262690
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,13 @@ | ||
| using System; | ||
| using System.Net; | ||
| using System.Net.Http; | ||
|
|
||
| namespace NGitLab; | ||
|
|
||
| public sealed class GitLabRequestResult | ||
|
Check failure on line 6 in NGitLab/GitLabRequestResult.cs
|
||
| { | ||
| public HttpWebRequest Request { get; set; } | ||
| public HttpRequestMessage Request { get; set; } | ||
|
Check failure on line 8 in NGitLab/GitLabRequestResult.cs
|
||
|
|
||
| public WebResponse Response { get; set; } | ||
| public HttpResponseMessage Response { get; set; } | ||
|
Check failure on line 10 in NGitLab/GitLabRequestResult.cs
|
||
|
|
||
| public Exception Exception { get; set; } | ||
|
Check failure on line 12 in NGitLab/GitLabRequestResult.cs
|
||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -76,33 +76,33 @@ | |||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| public WebResponse GetResponse(RequestOptions options) | ||||||||||
| public HttpResponseMessage GetResponse(RequestOptions options) | ||||||||||
| { | ||||||||||
| Func<WebResponse> getResponseImpl = () => GetResponseImpl(options); | ||||||||||
| Func<HttpResponseMessage> getResponseImpl = () => GetResponseImpl(options); | ||||||||||
|
|
||||||||||
| return getResponseImpl.Retry(options.ShouldRetry, | ||||||||||
| options.RetryInterval, | ||||||||||
| options.RetryCount, | ||||||||||
| options.IsIncremental); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| public Task<WebResponse> GetResponseAsync(RequestOptions options, CancellationToken cancellationToken) | ||||||||||
| public Task<HttpResponseMessage> GetResponseAsync(RequestOptions options, CancellationToken cancellationToken) | ||||||||||
| { | ||||||||||
| Func<Task<WebResponse>> getResponseImpl = () => GetResponseImplAsync(options, cancellationToken); | ||||||||||
| Func<Task<HttpResponseMessage>> getResponseImpl = () => GetResponseImplAsync(options, cancellationToken); | ||||||||||
|
|
||||||||||
| return getResponseImpl.RetryAsync(options.ShouldRetry, | ||||||||||
| options.RetryInterval, | ||||||||||
| options.RetryCount, | ||||||||||
| options.IsIncremental); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private WebResponse GetResponseImpl(RequestOptions options) | ||||||||||
| private HttpResponseMessage GetResponseImpl(RequestOptions options) | ||||||||||
| { | ||||||||||
| var result = new GitLabRequestResult(); | ||||||||||
| try | ||||||||||
| { | ||||||||||
| result.Request = CreateRequest(options); | ||||||||||
| result.Response = result.Request.GetResponse(); | ||||||||||
| result.Response = _httpClient.SendAsync(result.Request).GetAwaiter().GetResult(); | ||||||||||
| } | ||||||||||
| catch (Exception ex) | ||||||||||
| { | ||||||||||
|
|
@@ -116,13 +116,13 @@ | |||||||||
| return result.Exception is not null ? throw result.Exception : result.Response; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private async Task<WebResponse> GetResponseImplAsync(RequestOptions options, CancellationToken cancellationToken) | ||||||||||
| private async Task<HttpResponseMessage> GetResponseImplAsync(RequestOptions options, CancellationToken cancellationToken) | ||||||||||
| { | ||||||||||
| var result = new GitLabRequestResult(); | ||||||||||
| try | ||||||||||
| { | ||||||||||
| result.Request = CreateRequest(options); | ||||||||||
| result.Response = await result.Request.GetResponseAsync().ConfigureAwait(false); | ||||||||||
| result.Response = await _httpClient.SendAsync(result.Request, cancellationToken).ConfigureAwait(false); | ||||||||||
| } | ||||||||||
| catch (Exception ex) | ||||||||||
| { | ||||||||||
|
|
@@ -165,20 +165,22 @@ | |||||||||
| }; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private HttpWebRequest CreateRequest(RequestOptions options) | ||||||||||
| private static HttpClient _httpClient = null; | ||||||||||
|
|
||||||||||
| private HttpRequestMessage CreateRequest(RequestOptions options) | ||||||||||
| { | ||||||||||
| var request = WebRequest.CreateHttp(Url); | ||||||||||
| request.Method = Method.ToString().ToUpperInvariant(); | ||||||||||
| request.Accept = "application/json"; | ||||||||||
| request.Headers = Headers; | ||||||||||
| request.AutomaticDecompression = DecompressionMethods.GZip; | ||||||||||
| request.Timeout = (int)options.HttpClientTimeout.TotalMilliseconds; | ||||||||||
| request.ReadWriteTimeout = (int)options.HttpClientTimeout.TotalMilliseconds; | ||||||||||
| if (options.Proxy != null) | ||||||||||
| if (_httpClient != null) | ||||||||||
| { | ||||||||||
| request.Proxy = options.Proxy; | ||||||||||
| var handler = new HttpClientHandler | ||||||||||
| { | ||||||||||
| AutomaticDecompression = DecompressionMethods.GZip, | ||||||||||
| Proxy = options.Proxy, | ||||||||||
| UseProxy = options.Proxy != null, | ||||||||||
| }; | ||||||||||
| _httpClient = new HttpClient(handler); | ||||||||||
| _httpClient.Timeout = options.HttpClientTimeout; | ||||||||||
| } | ||||||||||
|
Check failure on line 182 in NGitLab/Impl/HttpRequestor.GitLabRequest.cs
|
||||||||||
maikebing marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||
|
|
||||||||||
| var request = new HttpRequestMessage(new HttpMethod(Method.ToString().ToUpperInvariant()), Url); | ||||||||||
| if (HasOutput) | ||||||||||
| { | ||||||||||
maikebing marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
| if (FormData != null) | ||||||||||
|
|
@@ -196,29 +198,27 @@ | |||||||||
| } | ||||||||||
| else if (Method == MethodType.Put) | ||||||||||
| { | ||||||||||
| request.ContentLength = 0; | ||||||||||
| //request.ContentLength = 0; | ||||||||||
|
Check failure on line 201 in NGitLab/Impl/HttpRequestor.GitLabRequest.cs
|
||||||||||
| } | ||||||||||
|
|
||||||||||
| return request; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private void AddJsonData(HttpWebRequest request, RequestOptions options) | ||||||||||
| private void AddJsonData(HttpRequestMessage request, RequestOptions options) | ||||||||||
| { | ||||||||||
| request.ContentType = "application/json"; | ||||||||||
|
|
||||||||||
| request.Content.Headers.ContentType.MediaType = "application/json"; | ||||||||||
| using var writer = new StreamWriter(options.GetRequestStream(request)); | ||||||||||
| writer.Write(JsonData); | ||||||||||
| writer.Flush(); | ||||||||||
| writer.Close(); | ||||||||||
maikebing marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||
| } | ||||||||||
|
|
||||||||||
| public void AddFileData(HttpWebRequest request, RequestOptions options) | ||||||||||
| public void AddFileData(HttpRequestMessage request, RequestOptions options) | ||||||||||
| { | ||||||||||
| var boundary = $"--------------------------{DateTime.UtcNow.Ticks.ToStringInvariant()}"; | ||||||||||
| if (Data is not FormDataContent formData) | ||||||||||
| return; | ||||||||||
| request.ContentType = "multipart/form-data; boundary=" + boundary; | ||||||||||
|
|
||||||||||
| request.Content.Headers.ContentType.MediaType = "multipart/form-data; boundary=" + boundary; | ||||||||||
| using var uploadContent = new MultipartFormDataContent(boundary) | ||||||||||
| { | ||||||||||
| { new StreamContent(formData.Stream), "file", formData.Name }, | ||||||||||
|
|
@@ -227,10 +227,9 @@ | |||||||||
| uploadContent.CopyToAsync(options.GetRequestStream(request)).Wait(); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| public void AddUrlEncodedData(HttpWebRequest request, RequestOptions options) | ||||||||||
| public void AddUrlEncodedData(HttpRequestMessage request, RequestOptions options) | ||||||||||
| { | ||||||||||
| request.ContentType = "application/x-www-form-urlencoded"; | ||||||||||
|
|
||||||||||
| request.Content.Headers.ContentType.MediaType = "application/x-www-form-urlencoded"; | ||||||||||
| using var content = new FormUrlEncodedContent(UrlEncodedData.Values); | ||||||||||
| content.CopyToAsync(options.GetRequestStream(request)).Wait(); | ||||||||||
|
Comment on lines
+179
to
181
|
||||||||||
| request.Content.Headers.ContentType.MediaType = "application/x-www-form-urlencoded"; | |
| using var content = new FormUrlEncodedContent(UrlEncodedData.Values); | |
| content.CopyToAsync(options.GetRequestStream(request)).Wait(); | |
| request.Content = new FormUrlEncodedContent(UrlEncodedData.Values); |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |||||
| using System.IO; | ||||||
| using System.Linq; | ||||||
| using System.Net; | ||||||
| using System.Net.Http; | ||||||
| using System.Threading; | ||||||
| using System.Threading.Tasks; | ||||||
| using NGitLab.Impl.Json; | ||||||
|
|
@@ -144,7 +145,7 @@ | |||||
| using var response = request.GetResponse(_options); | ||||||
| if (parser != null) | ||||||
| { | ||||||
| using var stream = response.GetResponseStream(); | ||||||
| using var stream = response.Content.ReadAsStreamAsync().GetAwaiter().GetResult(); | ||||||
maikebing marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| parser(stream, new WebHeadersDictionaryAdaptor(response.Headers)); | ||||||
| } | ||||||
| } | ||||||
|
|
@@ -159,7 +160,7 @@ | |||||
| using var response = await request.GetResponseAsync(_options, cancellationToken).ConfigureAwait(false); | ||||||
| if (parser != null) | ||||||
| { | ||||||
| using var stream = response.GetResponseStream(); | ||||||
| using var stream = response.Content.ReadAsStreamAsync().GetAwaiter().GetResult(); | ||||||
maikebing marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| await parser(stream, new WebHeadersDictionaryAdaptor(response.Headers)).ConfigureAwait(false); | ||||||
| } | ||||||
| } | ||||||
|
|
@@ -213,8 +214,7 @@ | |||||
| using (var response = await request.GetResponseAsync(_options, cancellationToken).ConfigureAwait(false)) | ||||||
| { | ||||||
| nextUrlToLoad = GetNextPageUrl(response); | ||||||
|
|
||||||
| using var stream = response.GetResponseStream(); | ||||||
| using var stream = response.Content.ReadAsStreamAsync().GetAwaiter().GetResult(); | ||||||
|
||||||
| using var stream = response.Content.ReadAsStreamAsync().GetAwaiter().GetResult(); | |
| using var stream = await response.Content.ReadAsStreamAsync().ConfigureAwait(false); |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using GetAwaiter().GetResult() in synchronous code can cause deadlocks in some contexts. For .NET 8.0, consider using the synchronous ReadAsStream() method with conditional compilation:
#if NET472 || NETSTANDARD2_0
using var stream = response.Content.ReadAsStreamAsync().GetAwaiter().GetResult();
#else
using var stream = response.Content.ReadAsStream();
#endifThis matches the pattern already used in RequestOptions.GetRequestStream.
Outdated
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The FirstOrDefault() call on line 250 can return null if the "Link" header doesn't exist, but the next line tries to call FirstOrDefault() on it, which will throw a NullReferenceException. The code should handle the null case:
private static Uri GetNextPageUrl(HttpResponseMessage response)
{
// <http://localhost:1080/api/v3/projects?page=2&per_page=0>; rel="next", <http://localhost:1080/api/v3/projects?page=1&per_page=0>; rel="first", <http://localhost:1080/api/v3/projects?page=2&per_page=0>; rel="last"
var linkHeader = response.Headers.FirstOrDefault(f => f.Key == "Link");
var link = linkHeader.Value?.FirstOrDefault();
string[] nextLink = null;
if (!string.IsNullOrEmpty(link))
{
nextLink = link.Split(',')
.Select(l => l.Split(';'))
.FirstOrDefault(pair => pair[1].Contains("next"));
}
return nextLink != null ? new Uri(nextLink[0].Trim('<', '>', ' ')) : null;
}Check failure on line 257 in NGitLab/Impl/HttpRequestor.cs
GitHub Actions / create_nuget
Closing brace should be followed by blank line (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1513.md)
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,6 +1,7 @@ | ||||||
| using System; | ||||||
| using System.IO; | ||||||
| using System.Net; | ||||||
| using System.Net.Http; | ||||||
| using NGitLab.Impl; | ||||||
|
|
||||||
| namespace NGitLab; | ||||||
|
|
@@ -77,8 +78,13 @@ public virtual void ProcessGitLabRequestResult(GitLabRequestResult e) | |||||
| { | ||||||
| } | ||||||
|
|
||||||
| internal virtual Stream GetRequestStream(HttpWebRequest request) | ||||||
| internal virtual Stream GetRequestStream(HttpRequestMessage request) | ||||||
| { | ||||||
| return request.GetRequestStream(); | ||||||
| #if NET472 || NETSTANDARD2_0 | ||||||
|
||||||
| #if NET472 || NETSTANDARD2_0 | |
| #if NET472 || NETSTANDARD2_0 |
Uh oh!
There was an error while loading. Please reload this page.