Skip to content

Fspes 138 #58

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

Merged
MichalFrends1 merged 9 commits into from
Jun 12, 2026
Merged

Fspes 138 #58

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5db6c2b
possible fix
MatteoDelOmbra Jun 12, 2026
c66593e
change tests to make them independent from hhtpbin.org
MatteoDelOmbra Jun 12, 2026
505be3e
add changelg and version
MatteoDelOmbra Jun 12, 2026
dc8b8e8
fix tests
MatteoDelOmbra Jun 12, 2026
c8d607a
change image
MatteoDelOmbra Jun 12, 2026
b8bfb84
change workflows to linux
MatteoDelOmbra Jun 12, 2026
a7e9e45
skip test for linux
MatteoDelOmbra Jun 12, 2026
f10d324
mstest change
MatteoDelOmbra Jun 12, 2026
44334aa
fix tests
MatteoDelOmbra Jun 12, 2026
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
2 changes: 1 addition & 1 deletion .github/workflows/Request_build_and_test_on_main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@

jobs:
build:
uses: FrendsPlatform/FrendsTasks/.github/workflows/build_main.yml@main
uses: FrendsPlatform/FrendsTasks/.github/workflows/linux_build_main.yml@main

@coderabbitai coderabbitai Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Mutable reusable workflow refs in .github/workflows/Request_build_and_test_on_main.yml and .github/workflows/Request_build_and_test_on_push.yml.
Both files pin to @main for external reusable workflows. The shared root cause is mutable upstream execution in CI; pin both uses: references to immutable full commit SHAs.

🧰 Tools
🪛 GitHub Check: CodeQL

[warning] 13-17: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{}}

🪛 zizmor (1.25.2)

[error] 13-13: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/Request_build_and_test_on_main.yml at line 13, The
reusable workflow reference "uses:
FrendsPlatform/FrendsTasks/.github/workflows/linux_build_main.yml@main" is
pinned to a mutable ref; update that `uses:` value in both
Request_build_and_test_on_main.yml and Request_build_and_test_on_push.yml to the
immutable full commit SHA (replace `@main` with `@<full-commit-sha>`), ensuring
both workflow files reference the identical exact SHA of the upstream
FrendsPlatform/FrendsTasks commit.

Source: Linters/SAST tools

with:
workdir: Frends.HTTP.Request
secrets:
badge_service_api_key: ${{ secrets.BADGE_SERVICE_API_KEY }}

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {}
2 changes: 1 addition & 1 deletion .github/workflows/Request_build_and_test_on_push.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@

jobs:
build:
uses: FrendsPlatform/FrendsTasks/.github/workflows/build_test.yml@main
uses: FrendsPlatform/FrendsTasks/.github/workflows/linux_build_test.yml@main
with:
workdir: Frends.HTTP.Request
secrets:
badge_service_api_key: ${{ secrets.BADGE_SERVICE_API_KEY }}
test_feed_api_key: ${{ secrets.TASKS_TEST_FEED_API_KEY }}

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {}
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*.user
*.userosscache
*.sln.docstates
.idea/

# User-specific files (MonoDevelop/Xamarin Studio)
*.userprefs
Expand Down
6 changes: 6 additions & 0 deletions Frends.HTTP.Request/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## [1.12.0] - 2026-06-12

### Fixed

- Fixed an issue related to concurrency problem with static fields.
Comment on lines +3 to +7

@coderabbitai coderabbitai Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Document the .NET 8 target bump as a breaking change.

This release also changes Frends.HTTP.Request.csproj from net6.0 to net8.0, so existing consumers cannot upgrade in place on .NET 6. Please add a breaking-change note with upgrade guidance under 1.12.0 instead of only listing the concurrency fix. As per coding guidelines, Frends.*/CHANGELOG.md must “Include all functional changes and indicate breaking changes with upgrade notes.”

🧰 Tools
🪛 LanguageTool

[style] ~7-~7: Consider using a different verb for a more formal wording.
Context: ... ## [1.12.0] - 2026-06-12 ### Fixed - Fixed an issue related to concurrency problem...

(FIX_RESOLVE)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Frends.HTTP.Request/CHANGELOG.md` around lines 3 - 7, The changelog entry for
1.12.0 omits a breaking change: the project target was changed from net6.0 to
net8.0 in Frends.HTTP.Request.csproj; update the 1.12.0 section in
Frends.HTTP.Request/CHANGELOG.md to add a “Breaking” note that states the target
framework bump (net6.0 -> net8.0), explain that consumers on .NET 6 cannot
upgrade in place, and provide upgrade guidance (e.g., retarget to .NET 8 or use
an earlier package version) so users know how to proceed.

Source: Coding guidelines


## [1.11.0] - 2026-05-07

### Added
Expand Down
3 changes: 2 additions & 1 deletion Frends.HTTP.Request/Frends.HTTP.Request.Tests/Frends.HTTP.Request.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>net6.0</TargetFramework>
<TargetFramework>net8.0</TargetFramework>
<IsPackable>false</IsPackable>
</PropertyGroup>
Comment on lines 3 to 6

@coderabbitai coderabbitai Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Required project metadata is still missing from this .csproj.

The new net8.0 target is fine, but this project file still omits the required metadata fields (Version, Authors, Description, RepositoryUrl, GenerateDocumentationFile, PackageLicenseExpression). Please add them here or document a repo-level exception for non-packable test projects. As per coding guidelines, Frends.*/Frends.*/*.csproj must target .NET 6 or 8 and include those fields with MIT licensing metadata.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@Frends.HTTP.Request/Frends.HTTP.Request.Tests/Frends.HTTP.Request.Tests.csproj`
around lines 3 - 6, The test project's .csproj is missing required NuGet/package
metadata; update the Frends.HTTP.Request.Tests.csproj PropertyGroup to include
Version, Authors, Description, RepositoryUrl, GenerateDocumentationFile, and
PackageLicenseExpression (e.g., MIT) even though IsPackable is false, or add a
note documenting a repo-level exception; ensure the XML properties are added
alongside TargetFramework and IsPackable so tools and CI validate the project
metadata.

Source: Coding guidelines


Expand All @@ -18,6 +18,7 @@
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="Testcontainers" Version="4.12.0" />
</ItemGroup>

<ItemGroup>
Expand Down
89 changes: 89 additions & 0 deletions Frends.HTTP.Request/Frends.HTTP.Request.Tests/HttpBinContainer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
using System;
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;
using DotNet.Testcontainers.Builders;
using DotNet.Testcontainers.Containers;

namespace Frends.HTTP.Request.Tests;

internal static class HttpBinContainer
{
private static readonly SemaphoreSlim StartStopLock = new(1, 1);
private static IContainer container;
private static string baseUrl;

public static string BaseUrl => baseUrl ?? throw new InvalidOperationException("HTTP test container has not been started.");

public static async Task StartAsync()
{
if (container != null)
return;

await StartStopLock.WaitAsync().ConfigureAwait(false);
try
{
if (container != null)
return;

container = new ContainerBuilder("kennethreitz/httpbin:latest")
.WithPortBinding(80, true)
.WithCleanUp(true)
.Build();

await container.StartAsync().ConfigureAwait(false);

var mappedPort = container.GetMappedPublicPort(80);
baseUrl = $"http://localhost:{mappedPort}";

await WaitUntilReadyAsync(baseUrl).ConfigureAwait(false);
}
finally
{
StartStopLock.Release();
}
}

public static async Task StopAsync()
{
await StartStopLock.WaitAsync().ConfigureAwait(false);
try
{
if (container == null)
return;

await container.DisposeAsync().ConfigureAwait(false);
container = null;
baseUrl = null;
}
finally
{
StartStopLock.Release();
}
}

private static async Task WaitUntilReadyAsync(string baseUrl)
{
using var client = new HttpClient();

for (var attempt = 0; attempt < 30; attempt++)
{
try
{
var response = await client.GetAsync($"{baseUrl}/status/200").ConfigureAwait(false);
if (response.IsSuccessStatusCode)
return;
}
catch
{
// Container may still be starting up.
}

await Task.Delay(TimeSpan.FromMilliseconds(500)).ConfigureAwait(false);
}

throw new InvalidOperationException("The HTTP test container did not become ready in time.");
}
}


42 changes: 31 additions & 11 deletions Frends.HTTP.Request/Frends.HTTP.Request.Tests/UnitTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.IO;
using System.Runtime.InteropServices;
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -9,8 +10,6 @@
using Assert = NUnit.Framework.Assert;
using System.Net;
using System.Security.Cryptography.X509Certificates;
using System.Reflection;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using NUnit.Framework;
using NUnit.Framework.Legacy;
Expand All @@ -20,18 +19,32 @@ namespace Frends.HTTP.Request.Tests;
[TestClass]
public class UnitTests
{
private const string BasePath = "https://httpbin.org";
private static string BasePath => HttpBinContainer.BaseUrl;

[ClassInitialize]
public static void ClassInitialize(Microsoft.VisualStudio.TestTools.UnitTesting.TestContext _)
{
HttpBinContainer.StartAsync().GetAwaiter().GetResult();
}

[ClassCleanup]
public static void ClassCleanup()
{
HttpBinContainer.StopAsync().GetAwaiter().GetResult();
}

[TestInitialize]
public void TestInitialize()
{
HTTP.ClearClientCache();
}

private static Input GetInputParams(Method.Method method = Method.Method.GET, string url = BasePath,
private static Input GetInputParams(Method.Method method = Method.Method.GET, string url = null,
string message = "",
params Header[] headers)
{
url ??= BasePath;

return new Input
{
Method = method,
Expand All @@ -44,16 +57,17 @@ private static Input GetInputParams(Method.Method method = Method.Method.GET, st
[TestMethod]
public async Task RequestTestGetWithParameters()
{
var expected = "\"args\": {\n \"id\": \"2\", \n \"userId\": \"1\"\n }";
var input = GetInputParams(url: $"{BasePath}/anything?id=2&userId=1");
var options = new Options
{
ConnectionTimeoutSeconds = 60,
};

var result = await HTTP.Request(input, options, CancellationToken.None);
var body = JObject.Parse((string)result.Body);

ClassicAssert.IsTrue(result.Body.Contains(expected));
ClassicAssert.AreEqual("2", body["args"]?["id"]?.Value<string>());
ClassicAssert.AreEqual("1", body["args"]?["userId"]?.Value<string>());
}

[TestMethod]
Expand Down Expand Up @@ -98,7 +112,7 @@ public void RequestShouldThrowExceptionIfOptionIsSet()
await HTTP.Request(input, options, CancellationToken.None));

ClassicAssert.IsTrue(
ex.Message.Contains("Request to 'https://httpbin.org/invalid' failed with status code 404"));
ex.Message.Contains($"Request to '{BasePath}/invalid' failed with status code 404"));
}

[TestMethod]
Expand Down Expand Up @@ -337,8 +351,9 @@ public async Task PatchShouldComeThrough()
};

var result = await HTTP.Request(input, options, CancellationToken.None);
var body = JObject.Parse((string)result.Body);

ClassicAssert.IsTrue(result.Body.Contains("\"data\": \"\\u00e5\\u00e4\\u00f6\""), result.Body);
ClassicAssert.AreEqual(message, body["data"]?.Value<string>(), result.Body);
}

[TestMethod]
Expand Down Expand Up @@ -388,14 +403,19 @@ public async Task RequestTest_GetMethod_ShouldSendEmptyContent()
ConnectionTimeoutSeconds = 60
};
var result = await HTTP.Request(input, options, CancellationToken.None);
var body = JObject.Parse((string)result.Body);

Assert.That(result.Body.Contains("\"data\": \"\""), result.Body);
Assert.That(body["data"]?.Value<string>(), Is.EqualTo(string.Empty), result.Body);
}

[TestCase(CertificateStoreLocation.CurrentUser, "current user")]
[TestCase(CertificateStoreLocation.LocalMachine, "local machine")]
[DataTestMethod]
[DataRow(CertificateStoreLocation.CurrentUser, "current user")]
[DataRow(CertificateStoreLocation.LocalMachine, "local machine")]
public void CorrectStoreSearched(CertificateStoreLocation storeLocation, string storeLocationText)
{
if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
Microsoft.VisualStudio.TestTools.UnitTesting.Assert.Inconclusive("This test runs only on Windows.");

var handler = new HttpClientHandler();
X509Certificate2[] certificates = Array.Empty<X509Certificate2>();
var options = new Options
Expand Down
4 changes: 2 additions & 2 deletions Frends.HTTP.Request/Frends.HTTP.Request/Frends.HTTP.Request.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>net6.0</TargetFramework>
<Version>1.11.0</Version>
<TargetFramework>net8.0</TargetFramework>
<Version>1.12.0</Version>
<Authors>Frends</Authors>
<Copyright>Frends</Copyright>
<Company>Frends</Company>
Expand Down
34 changes: 13 additions & 21 deletions Frends.HTTP.Request/Frends.HTTP.Request/Request.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
using System.Runtime.CompilerServices;
using System.Diagnostics.CodeAnalysis;
using System.Security.Cryptography.X509Certificates;
using System.Security.Cryptography;
using Frends.HTTP.Request.Definitions;

[assembly: InternalsVisibleTo("Frends.HTTP.Request.Tests")]
Expand All @@ -34,12 +33,6 @@ public static class HTTP
SlidingExpiration = TimeSpan.FromHours(1),
};

private static HttpContent httpContent;
private static HttpClient httpClient;
private static HttpClientHandler httpClientHandler;
private static HttpRequestMessage httpRequestMessage;
private static HttpResponseMessage httpResponseMessage;
private static X509Certificate2[] certificates = Array.Empty<X509Certificate2>();


internal static void ClearClientCache()
Expand All @@ -66,9 +59,12 @@ public static async Task<Result> Request(
CancellationToken cancellationToken
)
{
HttpClient httpClient = null;
HttpContent httpContent = null;

try
{
if (string.IsNullOrEmpty(input.Url)) throw new ArgumentNullException("Url can not be empty.");
if (string.IsNullOrEmpty(input.Url)) throw new ArgumentNullException(nameof(input), "Url can not be empty.");

@coderabbitai coderabbitai Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Throw against the actual invalid argument here.

For an empty URL, the invalid member is input.Url, not input, and ArgumentNullException is the wrong exception for an empty string. Split this into an input == null guard and an ArgumentException/ArgumentNullException on input.Url so callers get the correct parameter name and exception type.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Frends.HTTP.Request/Frends.HTTP.Request/Request.cs` at line 67, Replace the
single combined check with two guards: first verify the parameter object `input`
is not null and throw ArgumentNullException(nameof(input)) if it is, then
validate the URL specifically by checking `input.Url` and throw an
ArgumentException (or ArgumentNullException if you prefer to distinguish null vs
empty) with the parameter name nameof(input.Url) and a clear message like "Url
cannot be empty." so callers receive the correct exception type and parameter
name for `input` and `input.Url`.


httpClient = GetHttpClientForOptions(options);
var headers = GetHeaderDictionary(input.Headers, options);
Expand All @@ -89,12 +85,10 @@ CancellationToken cancellationToken
switch (input.ResultMethod)
{
case ReturnFormat.String:
var hbody = responseMessage.Content != null
? await responseMessage.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false)
: null;
var hbody = await responseMessage.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false);
var hstatusCode = (int)responseMessage.StatusCode;
var hheaders =
GetResponseHeaderDictionary(responseMessage.Headers, responseMessage.Content?.Headers);
GetResponseHeaderDictionary(responseMessage.Headers, responseMessage.Content.Headers);
response = new Result(hbody, hheaders, hstatusCode);

break;
Expand Down Expand Up @@ -123,15 +117,11 @@ CancellationToken cancellationToken
}
finally
{
httpResponseMessage?.Dispose();
httpRequestMessage?.Dispose();
httpContent?.Dispose();

if (!options.CacheHttpClient)
{
foreach (var cert in certificates) cert?.Dispose();
httpClient?.Dispose();
httpClientHandler?.Dispose();
}
}
}
Expand Down Expand Up @@ -237,9 +227,10 @@ private static HttpClient GetHttpClientForOptions(Options options)
}
}

httpClientHandler = new HttpClientHandler();
var httpClientHandler = new HttpClientHandler();
X509Certificate2[] certificates = Array.Empty<X509Certificate2>();
httpClientHandler.SetHandlerSettingsBasedOnOptions(options, ref certificates);
httpClient = new HttpClient(httpClientHandler);
var httpClient = new HttpClient(httpClientHandler);
httpClient.SetDefaultRequestHeadersBasedOnOptions(options);

if (cacheKey != null) ClientCache.Add(cacheKey, httpClient, CachePolicy);
Expand Down Expand Up @@ -267,7 +258,7 @@ private static async Task<HttpResponseMessage> GetHttpRequestResponseAsync(
{
cancellationToken.ThrowIfCancellationRequested();

httpRequestMessage = new HttpRequestMessage(new HttpMethod(method), new Uri(url));
var httpRequestMessage = new HttpRequestMessage(new HttpMethod(method), new Uri(url));
httpRequestMessage.Content = content;

//Clear default headers
Expand All @@ -291,6 +282,7 @@ private static async Task<HttpResponseMessage> GetHttpRequestResponseAsync(
}
}

HttpResponseMessage httpResponseMessage;
try
{
httpResponseMessage = await client.SendAsync(httpRequestMessage, cancellationToken).ConfigureAwait(false);
Expand All @@ -303,13 +295,13 @@ private static async Task<HttpResponseMessage> GetHttpRequestResponseAsync(
throw;
}

// Cancellation is from inside of the request, mostly likely a timeout
// Cancellation is from inside the request, mostly likely a timeout
throw new Exception("HttpRequest was canceled, most likely due to a timeout.", canceledException);
Comment on lines +298 to 299

@coderabbitai coderabbitai Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Preserve a timeout-specific exception type.

Wrapping an internal timeout as plain Exception makes it much harder for callers to distinguish timeouts from other request failures. Throw a TimeoutException (or preserve the original cancellation exception type with clearer context) so retry and error-handling code can keep matching on a meaningful contract.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Frends.HTTP.Request/Frends.HTTP.Request/Request.cs` around lines 298 - 299,
Replace the generic Exception thrown when a request cancellation occurs with a
timeout-specific exception so callers can detect timeouts: in Request.cs where
you currently do `throw new Exception("HttpRequest was canceled, most likely due
to a timeout.", canceledException);`, throw a TimeoutException (or rethrow the
original canceledException wrapped in a TimeoutException) using the same message
and include the original canceledException as the inner exception so
retry/error-handling logic can match on a meaningful timeout contract.

}


// this check is probably not needed anymore as the new HttpClient does not fail on invalid charsets
if (options.AllowInvalidResponseContentTypeCharSet && httpResponseMessage.Content.Headers?.ContentType != null)
if (options.AllowInvalidResponseContentTypeCharSet && httpResponseMessage.Content.Headers.ContentType != null)
{
httpResponseMessage.Content.Headers.ContentType.CharSet = null;
}
Expand Down
Loading