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.

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

Address several static code analysis warnings #84

Merged
merged 12 commits into from
Nov 18, 2023
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ private static bool ElemEquals(JsonElement left, JsonElement right, JsonSerializ
JsonValueKind.False => true,
JsonValueKind.True => true,
JsonValueKind.String => left.ValueEquals(right.GetString()),
JsonValueKind.Number => left.GetRawText().Equals(right.GetRawText()),
JsonValueKind.Number => string.Equals(left.GetRawText(), right.GetRawText(), StringComparison.Ordinal),
JsonValueKind.Array => ArrayEquals(left, right, serializerOptions),
JsonValueKind.Object => ObjectEquals(left, right, serializerOptions),
_ => false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ public bool Equals(string? x, string? y)

public int GetHashCode(string obj)
{
#if NET6_0_OR_GREATER || NETSTANDARD2_1_OR_GREATER
return obj.GetHashCode(StringComparison.Ordinal);
#else
return obj.GetHashCode();
#endif
}
}
60 changes: 45 additions & 15 deletions src/MockHttp.Server/MockHttpServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public sealed class MockHttpServer : IDisposable
[DebuggerBrowsable(DebuggerBrowsableState.Never)]
private IWebHost? _host;
[DebuggerBrowsable(DebuggerBrowsableState.Never)]
private readonly string _hostUrl;
private readonly Uri _hostUri;
[DebuggerBrowsable(DebuggerBrowsableState.Never)]
private Action<IApplicationBuilder>? _configureAppBuilder;

Expand All @@ -32,8 +32,19 @@ public sealed class MockHttpServer : IDisposable
/// </summary>
/// <param name="mockHttpHandler">The mock http handler.</param>
/// <param name="hostUrl">The host URL the mock HTTP server will listen on.</param>
[Obsolete("Use the overload accepting an System.Uri.")]
public MockHttpServer(MockHttpHandler mockHttpHandler, string hostUrl)
: this(mockHttpHandler, null, hostUrl)
: this(mockHttpHandler, GetHostUrl(hostUrl))
{
}

/// <summary>
/// Initializes a new instance of the <see cref="MockHttpServer" /> using specified <paramref name="mockHttpHandler" /> and configures it to listen on specified <paramref name="hostUri" />.
/// </summary>
/// <param name="mockHttpHandler">The mock http handler.</param>
/// <param name="hostUri">The host URI the mock HTTP server will listen on.</param>
public MockHttpServer(MockHttpHandler mockHttpHandler, Uri hostUri)
: this(mockHttpHandler, null, hostUri)
{
}

Expand All @@ -43,12 +54,24 @@ public MockHttpServer(MockHttpHandler mockHttpHandler, string hostUrl)
/// <param name="mockHttpHandler">The mock http handler.</param>
/// <param name="loggerFactory">The logger factory to use to log pipeline requests to.</param>
/// <param name="hostUrl">The host URL the mock HTTP server will listen on.</param>
[Obsolete("Use the overload accepting an System.Uri.")]
public MockHttpServer(MockHttpHandler mockHttpHandler, ILoggerFactory? loggerFactory, string hostUrl)
: this(mockHttpHandler, loggerFactory, GetHostUrl(hostUrl))
{
}

/// <summary>
/// Initializes a new instance of the <see cref="MockHttpServer" /> using specified <paramref name="mockHttpHandler" /> and configures it to listen on specified <paramref name="hostUri" />.
/// </summary>
/// <param name="mockHttpHandler">The mock http handler.</param>
/// <param name="loggerFactory">The logger factory to use to log pipeline requests to.</param>
/// <param name="hostUri">The host URI the mock HTTP server will listen on.</param>
public MockHttpServer(MockHttpHandler mockHttpHandler, ILoggerFactory? loggerFactory, Uri hostUri)
{
Handler = mockHttpHandler ?? throw new ArgumentNullException(nameof(mockHttpHandler));
_webHostBuilder = CreateWebHostBuilder(loggerFactory);
_hostUrl = GetHostUrl(hostUrl);
_webHostBuilder.UseUrls(_hostUrl);
_hostUri = new Uri(hostUri, "/"); // Ensure base URL.
_webHostBuilder.UseUrls(_hostUri.ToString());
}

/// <inheritdoc />
Expand All @@ -66,13 +89,24 @@ public void Dispose()
/// <summary>
/// Gets the host URL the mock HTTP server will listen on.
/// </summary>
public string HostUrl
[Obsolete("Use the HostUri instead.")]
#pragma warning disable CA1056 // URI-like properties should not be strings
public string HostUrl => HostUri.ToString().TrimEnd('/');
#pragma warning restore CA1056 // URI-like properties should not be strings

/// <summary>
/// Gets the host URI the mock HTTP server will listen on.
/// </summary>
public Uri HostUri
{
get
{
lock (_syncLock)
{
return _host?.ServerFeatures.Get<IServerAddressesFeature>()?.Addresses.First() ?? _hostUrl;
string? url = _host?.ServerFeatures.Get<IServerAddressesFeature>()?.Addresses.First();
return url is null
? _hostUri
: new Uri(url);
}
}
}
Expand Down Expand Up @@ -140,7 +174,7 @@ public Task StopAsync(CancellationToken cancellationToken = default)
/// </summary>
public HttpClient CreateClient()
{
return new HttpClient { BaseAddress = new Uri(HostUrl) };
return new HttpClient { BaseAddress = HostUri };
}

private IWebHostBuilder CreateWebHostBuilder(ILoggerFactory? loggerFactory)
Expand Down Expand Up @@ -173,20 +207,16 @@ internal void Configure(Action<IApplicationBuilder> configureAppBuilder)
_configureAppBuilder = configureAppBuilder;
}

private static string GetHostUrl(string hostUrl)
private static Uri GetHostUrl(string hostUrl)
{
if (hostUrl is null)
{
throw new ArgumentNullException(nameof(hostUrl));
}

if (!Uri.TryCreate(hostUrl, UriKind.Absolute, out Uri? uri))
{
throw new ArgumentException(Resources.Error_HostUrlIsNotValid, nameof(hostUrl));
}

// Ensure we have a proper host URL without path/query.
return $"{uri.Scheme}://{uri.Host}:{uri.Port}";
return Uri.TryCreate(hostUrl, UriKind.Absolute, out Uri? uri)
? uri
: throw new ArgumentException(Resources.Error_HostUrlIsNotValid, nameof(hostUrl));
}

private void AddMockHttpServerHeader(IApplicationBuilder applicationBuilder)
Expand Down
2 changes: 2 additions & 0 deletions src/MockHttp.Server/Server/ServerRequestHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ public async Task HandleAsync(HttpContext httpContext, Func<Task> _)
{
LogRequestMessage(httpContext, Resources.Error_VerifyMockSetup);

#pragma warning disable CA2000 // Dispose objects before losing scope
httpResponseMessage = new HttpResponseMessage(HttpStatusCode.InternalServerError)
#pragma warning restore CA2000 // Dispose objects before losing scope
{
ReasonPhrase = Resources.Error_VerifyMockSetup,
Content = new StringContent(Resources.Error_VerifyMockSetup + Environment.NewLine + ex, MockHttpHandler.DefaultEncoding, MediaTypes.PlainText)
Expand Down
2 changes: 1 addition & 1 deletion src/MockHttp/Extensions/ResponseBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ public static IWithContentResult Body(this IWithContent builder, Func<Cancellati

return builder.Body(async token =>
{
Stream stream = await streamContentFactory(token);
Stream stream = await streamContentFactory(token).ConfigureAwait(false);
if (!stream.CanRead)
{
throw new InvalidOperationException("Cannot read from stream.");
Expand Down
6 changes: 5 additions & 1 deletion src/MockHttp/Http/DataEscapingHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,11 @@ internal static IEnumerable<KeyValuePair<string, IEnumerable<string>>> Parse(str

private static string UnescapeData(string v)
{
return Uri.UnescapeDataString(v?.Replace("+", "%20")!);
return Uri.UnescapeDataString(v?.Replace("+", "%20"
#if NET7_0_OR_GREATER || NETSTANDARD2_1
, StringComparison.Ordinal
#endif
)!);
}

internal static string Format(IEnumerable<KeyValuePair<string, string>> items)
Expand Down
7 changes: 5 additions & 2 deletions src/MockHttp/Http/HttpHeadersCollection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ namespace MockHttp.Http;

internal sealed class HttpHeadersCollection : HttpHeaders
{
private static readonly char[] HeaderKeyValueSeparator = new[] { ':' };
private static readonly char[] HeaderValueSeparator = new[] { ',' };

public HttpHeadersCollection()
{
}
Expand Down Expand Up @@ -43,7 +46,7 @@ public static HttpHeaders Parse(string headers)
continue;
}

string[] hvp = header.Split(new[] { ':' }, 2, StringSplitOptions.None);
string[] hvp = header.Split(HeaderKeyValueSeparator, 2, StringSplitOptions.None);

string fieldName = hvp.Length > 0 ? hvp[0] : string.Empty;
string? fieldValue = hvp.Length > 1 ? hvp[1] : null;
Expand All @@ -61,7 +64,7 @@ internal static IEnumerable<string> ParseHttpHeaderValue(string? headerValue)
}

return headerValue
.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries)
.Split(HeaderValueSeparator, StringSplitOptions.RemoveEmptyEntries)
.Select(v => v.Trim())
.Where(v => v.Length > 0)
.ToArray();
Expand Down
16 changes: 8 additions & 8 deletions src/MockHttp/Matchers/ContentMatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public ContentMatcher(byte[] content)
/// Gets the expected content in bytes.
/// </summary>
// ReSharper disable once MemberCanBeProtected.Global
protected internal byte[] ByteContent { get; }
protected internal IReadOnlyList<byte> ByteContent { get; }

/// <inheritdoc />
public Task<bool> IsMatchAsync(MockHttpRequestContext requestContext)
Expand All @@ -71,10 +71,10 @@ async Task<bool> InternalIsMatchAsync(MockHttpRequestContext mockHttpRequestCont

if (requestContent is null)
{
return ByteContent.Length == 0;
return ByteContent.Count == 0;
}

if (requestContent.Length == 0 && ByteContent.Length == 0)
if (requestContent.Length == 0 && ByteContent.Count == 0)
{
return true;
}
Expand All @@ -99,17 +99,17 @@ protected virtual bool IsMatch(byte[] requestContent)
/// <inheritdoc />
public override string ToString()
{
if (ByteContent.Length == 0)
if (ByteContent.Count == 0)
{
return "Content: <empty>";
}

if (_encoding is not null)
{
return $"Content: {_encoding.GetString(ByteContent, 0, ByteContent.Length)}";
return $"Content: {_encoding.GetString((byte[])ByteContent, 0, ByteContent.Count)}";
}

int charsToOutput = Math.Min(MaxBytesDisplayed, ByteContent.Length);
int charsToOutput = Math.Min(MaxBytesDisplayed, ByteContent.Count);
var sb = new StringBuilder();
sb.Append("Content: [");
for (int i = 0; i < charsToOutput; i++)
Expand All @@ -121,9 +121,9 @@ public override string ToString()
}
}

if (charsToOutput < ByteContent.Length)
if (charsToOutput < ByteContent.Count)
{
sb.AppendFormat(CultureInfo.InvariantCulture, ",...](Size = {0})", ByteContent.Length);
sb.AppendFormat(CultureInfo.InvariantCulture, ",...](Size = {0})", ByteContent.Count);
}
else
{
Expand Down
4 changes: 2 additions & 2 deletions src/MockHttp/Matchers/PartialContentMatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class PartialContentMatcher : ContentMatcher
public PartialContentMatcher(string content, Encoding? encoding)
: base(content, encoding)
{
if (ByteContent.Length == 0)
if (ByteContent.Count == 0)
{
throw new ArgumentException("Content can not be empty.", nameof(content));
}
Expand All @@ -28,7 +28,7 @@ public PartialContentMatcher(string content, Encoding? encoding)
public PartialContentMatcher(byte[] content)
: base(content)
{
if (ByteContent.Length == 0)
if (ByteContent.Count == 0)
{
throw new ArgumentException("Content can not be empty.", nameof(content));
}
Expand Down
13 changes: 5 additions & 8 deletions src/MockHttp/MockHttpHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ namespace MockHttp;
/// </summary>
public sealed class MockHttpHandler : HttpMessageHandler, IMockConfiguration
{
private readonly ConcurrentCollection<HttpCall> _setups;
private readonly HttpCall _fallbackSetup;
private readonly IDictionary<Type, object> _items;
private readonly ConcurrentCollection<HttpCall> _setups = new();
private readonly HttpCall _fallbackSetup = new();
private readonly Dictionary<Type, object> _items = new();
private readonly ReadOnlyDictionary<Type, object> _readOnlyItems;

/// <summary>
Expand All @@ -29,12 +29,9 @@ public sealed class MockHttpHandler : HttpMessageHandler, IMockConfiguration
/// </summary>
public MockHttpHandler()
{
_setups = new ConcurrentCollection<HttpCall>();
InvokedRequests = new InvokedHttpRequestCollection(this);
_items = new Dictionary<Type, object>();
_readOnlyItems = new ReadOnlyDictionary<Type, object>(_items);

_fallbackSetup = new HttpCall();
Fallback = new FallbackRequestSetupPhrase(_fallbackSetup);
Reset();
}
Expand Down Expand Up @@ -241,7 +238,7 @@ public void VerifyNoOtherRequests()
.Cast<InvokedHttpRequest>()
.Where(r => !r.IsVerified)
.ToList();
if (!unverifiedRequests.Any())
if (unverifiedRequests.Count == 0)
{
return;
}
Expand Down Expand Up @@ -270,7 +267,7 @@ private void Verify(IEnumerable<HttpCall> verifiableSetups)
var expectedInvocations = verifiableSetups
.Where(setup => !setup.VerifyIfInvoked())
.ToList();
if (!expectedInvocations.Any())
if (expectedInvocations.Count == 0)
{
return;
}
Expand Down
7 changes: 5 additions & 2 deletions src/MockHttp/NetworkLatency.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Runtime.CompilerServices;
using MockHttp.Threading;

namespace MockHttp;
Expand All @@ -17,7 +16,9 @@ public class NetworkLatency
static NetworkLatency()
{
// Warmup so that actual simulated latency is more accurate.
#pragma warning disable CA5394 // Do not use insecure randomness - justification: not used in secure context
Random.Next();
#pragma warning restore CA5394 // Do not use insecure randomness
}

private NetworkLatency(Func<TimeSpan> factory, string name)
Expand Down Expand Up @@ -111,7 +112,9 @@ private static NetworkLatency Between(int minMs, int maxMs, string name)

return new NetworkLatency(() =>
{
#pragma warning disable CA5394 // Do not use insecure randomness - justification: not used in secure context
double randomLatency = Random.Next(minMs, maxMs);
#pragma warning restore CA5394
return TimeSpan.FromMilliseconds(randomLatency);
},
name);
Expand Down
2 changes: 1 addition & 1 deletion src/MockHttp/RequestMatching.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
.Where(m => m.GetType() == matcher.GetType())
.ToList();

if ((matcher.IsExclusive && sameTypeMatchers.Any()) || (!matcher.IsExclusive && sameTypeMatchers.Any(m => m.IsExclusive)))
if ((matcher.IsExclusive && sameTypeMatchers.Count > 0) || (!matcher.IsExclusive && sameTypeMatchers.Any(m => m.IsExclusive)))

Check warning on line 73 in src/MockHttp/RequestMatching.cs

View workflow job for this annotation

GitHub Actions / analysis

Collection-specific "Exists" method should be used instead of the "Any" extension. (https://rules.sonarsource.com/csharp/RSPEC-6605)

Check warning on line 73 in src/MockHttp/RequestMatching.cs

View workflow job for this annotation

GitHub Actions / analysis

Collection-specific "Exists" method should be used instead of the "Any" extension. (https://rules.sonarsource.com/csharp/RSPEC-6605)
{
throw new InvalidOperationException($"Cannot add matcher, another matcher of type '{matcher.GetType().FullName}' already is configured.");
}
Expand Down
2 changes: 1 addition & 1 deletion src/MockHttp/Responses/HttpHeaderBehavior.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
: IResponseBehavior
{
//https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html
private static readonly ISet<string> HeadersWithSingleValueOnly = new HashSet<string>
private static readonly HashSet<string> HeadersWithSingleValueOnly = new()
{
// TODO: expand this list.
HeaderNames.Age,
Expand Down Expand Up @@ -86,7 +86,7 @@
headers.Add(name, values);
return true;
}
catch (Exception)

Check warning on line 89 in src/MockHttp/Responses/HttpHeaderBehavior.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

Modify 'TryAdd' to catch a more specific allowed exception type, or rethrow the exception (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1031)

Check warning on line 89 in src/MockHttp/Responses/HttpHeaderBehavior.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

Modify 'TryAdd' to catch a more specific allowed exception type, or rethrow the exception (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1031)

Check warning on line 89 in src/MockHttp/Responses/HttpHeaderBehavior.cs

View workflow job for this annotation

GitHub Actions / analysis

Modify 'TryAdd' to catch a more specific allowed exception type, or rethrow the exception (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1031)

Check warning on line 89 in src/MockHttp/Responses/HttpHeaderBehavior.cs

View workflow job for this annotation

GitHub Actions / analysis

Modify 'TryAdd' to catch a more specific allowed exception type, or rethrow the exception (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1031)

Check warning on line 89 in src/MockHttp/Responses/HttpHeaderBehavior.cs

View workflow job for this annotation

GitHub Actions / build (v4.1.2-pr84.22)

Modify 'TryAdd' to catch a more specific allowed exception type, or rethrow the exception (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1031)

Check warning on line 89 in src/MockHttp/Responses/HttpHeaderBehavior.cs

View workflow job for this annotation

GitHub Actions / build (v4.1.2-pr84.22)

Modify 'TryAdd' to catch a more specific allowed exception type, or rethrow the exception (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1031)
{
return false;
}
Expand Down
2 changes: 2 additions & 0 deletions src/MockHttp/Threading/ConcurrentCollection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ public T this[int index]
if (_items is null)
{
#pragma warning disable S112
#pragma warning disable CA2201 // Do not raise reserved exception types
throw new IndexOutOfRangeException();
#pragma warning restore CA2201 // Do not raise reserved exception types
#pragma warning restore S112
}

Expand Down
10 changes: 9 additions & 1 deletion test/MockHttp.Server.Tests/Fixtures/MockHttpServerFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,15 @@ protected MockHttpServerFixture(string scheme)
.CreateLogger();
LoggerFactory = new SerilogLoggerFactory(logger);
Handler = new MockHttpHandler();
Server = new MockHttpServer(Handler, LoggerFactory, SupportsIpv6() ? $"{scheme}://[::1]:0" : $"{scheme}://127.0.0.1:0");
Server = new MockHttpServer(
Handler,
LoggerFactory,
new Uri(
SupportsIpv6()
? $"{scheme}://[::1]:0"
: $"{scheme}://127.0.0.1:0"
)
);
Server
.Configure(builder => builder
.Use((_, next) =>
Expand Down
Loading
Loading