From d42db40e8040ec916fa76ef3217288d2a87056fd Mon Sep 17 00:00:00 2001 From: maxisoft Date: Thu, 8 Aug 2024 15:30:30 +0200 Subject: [PATCH] Fix JSON serialization issue, improve error handling, and add HttpClient optimizations (#84, improvements) This commit addresses several improvements and bug fixes for the ASF-FreeGames plugin: * Fixed JSON serialization issue: * Resolved compatibility problems with recent ASF versions causing issues with `config.json` loading (`ASFFreeGamesOptionsSaver.cs`). * Implemented a new `SaveOptions` method that validates and writes configuration options to the file in a more robust way. * Added unit tests to ensure proper JSON serialization (`ASFFreeGamesOptionsSaverTests.cs`). * Enhanced error handling: * Improved error message when encountering issues during `config.json` loading (`ASFFreeGames.cs`). * Provided more informative logging in case of unexpected errors (`ASFFreeGamesOptionsLoader.cs`). * Optimized HttpClient usage: * Introduced `SimpleHttpClient` class with improved configuration options (`SimpleHttpClient.cs`). * Set default `MaxConnectionsPerServer` to limit resource usage (`SimpleHttpClient.cs`). * Implemented a workaround for missing `CheckCertificateRevocationList` property (`SimpleHttpClient.cs`). * Improved stream handling in `HttpStreamResponse` class to gracefully handle potential null streams (`SimpleHttpClient.cs`, `HttpStreamResponse.cs`). * Minor improvements: * Added comments and code formatting for better readability. * Updated code to adhere to modern C# practices. These changes ensure compatibility with recent ASF versions, provide better error handling for configuration issues, and optimize the performance and reliability of the plugin's network communication. --- .../ASFFreeGamesOptionsSaverTests.cs | 58 +++++ ASFFreeGames/Commands/GetIp/GetIPCommand.cs | 2 + .../ASFFreeGamesOptionsLoader.cs | 22 +- .../ASFFreeGamesOptionsSaver.cs | 220 ++++++++++++++++++ .../HttpClientSimple/SimpleHttpClient.cs | 93 +++++++- 5 files changed, 383 insertions(+), 12 deletions(-) create mode 100644 ASFFreeGames.Tests/Configurations/ASFFreeGamesOptionsSaverTests.cs create mode 100644 ASFFreeGames/Configurations/ASFFreeGamesOptionsSaver.cs diff --git a/ASFFreeGames.Tests/Configurations/ASFFreeGamesOptionsSaverTests.cs b/ASFFreeGames.Tests/Configurations/ASFFreeGamesOptionsSaverTests.cs new file mode 100644 index 0000000..e809bbd --- /dev/null +++ b/ASFFreeGames.Tests/Configurations/ASFFreeGamesOptionsSaverTests.cs @@ -0,0 +1,58 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Text; +using System.Text.Json; +using ASFFreeGames.Configurations; +using Xunit; + +namespace Maxisoft.ASF.Tests.Configurations; + +public class ASFFreeGamesOptionsSaverTests { + [Fact] +#pragma warning disable CA1707 + public async void SaveOptions_WritesValidJson_And_ParsesCorrectly() { +#pragma warning restore CA1707 + + // Arrange + ASFFreeGamesOptions options = new() { + RecheckInterval = TimeSpan.FromHours(1), + RandomizeRecheckInterval = true, + SkipFreeToPlay = false, + SkipDLC = true, + Blacklist = new HashSet { + "game1", + "game2", + "a gamewith2xquote(\")'", + "game with strange char €$çêà /\\\n\r\t" + }, + VerboseLog = null, + Proxy = "http://localhost:1080", + RedditProxy = "socks5://192.168.1.1:1081" + }; + + using MemoryStream memoryStream = new(); + + // Act + _ = await ASFFreeGamesOptionsSaver.SaveOptions(memoryStream, options).ConfigureAwait(false); + + // Assert - Validate UTF-8 encoding + memoryStream.Position = 0; + Assert.NotEmpty(Encoding.UTF8.GetString(memoryStream.ToArray())); + + // Assert - Parse JSON and access properties + memoryStream.Position = 0; + string json = Encoding.UTF8.GetString(memoryStream.ToArray()); + ASFFreeGamesOptions? deserializedOptions = JsonSerializer.Deserialize(json); + + Assert.NotNull(deserializedOptions); + Assert.Equal(options.RecheckInterval, deserializedOptions.RecheckInterval); + Assert.Equal(options.RandomizeRecheckInterval, deserializedOptions.RandomizeRecheckInterval); + Assert.Equal(options.SkipFreeToPlay, deserializedOptions.SkipFreeToPlay); + Assert.Equal(options.SkipDLC, deserializedOptions.SkipDLC); + Assert.Equal(options.Blacklist, deserializedOptions.Blacklist); + Assert.Equal(options.VerboseLog, deserializedOptions.VerboseLog); + Assert.Equal(options.Proxy, deserializedOptions.Proxy); + Assert.Equal(options.RedditProxy, deserializedOptions.RedditProxy); + } +} diff --git a/ASFFreeGames/Commands/GetIp/GetIPCommand.cs b/ASFFreeGames/Commands/GetIp/GetIPCommand.cs index 62f98e2..d3725d7 100644 --- a/ASFFreeGames/Commands/GetIp/GetIPCommand.cs +++ b/ASFFreeGames/Commands/GetIp/GetIPCommand.cs @@ -44,7 +44,9 @@ internal sealed class GetIPCommand : IBotCommand { } } catch (Exception e) when (e is JsonException or IOException) { +#pragma warning disable CA1863 return IBotCommand.FormatBotResponse(bot, string.Format(CultureInfo.CurrentCulture, Strings.ErrorIsInvalid, e.Message)); +#pragma warning restore CA1863 } return null; diff --git a/ASFFreeGames/Configurations/ASFFreeGamesOptionsLoader.cs b/ASFFreeGames/Configurations/ASFFreeGamesOptionsLoader.cs index fa0e9af..f4fd2b4 100644 --- a/ASFFreeGames/Configurations/ASFFreeGamesOptionsLoader.cs +++ b/ASFFreeGames/Configurations/ASFFreeGamesOptionsLoader.cs @@ -1,4 +1,5 @@ using System; +using System.Buffers; using System.Collections.Generic; using System.IO; using System.Text.Json; @@ -58,13 +59,26 @@ public static async Task Save(ASFFreeGamesOptions options, CancellationToken can #pragma warning disable CA2007 // Use FileOptions.Asynchronous when creating a file stream for async operations - await using FileStream fs = new(path, FileMode.OpenOrCreate, FileAccess.Write, FileShare.None, 4096, FileOptions.Asynchronous); + await using FileStream fs = new(path, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.ReadWrite, 4096, FileOptions.Asynchronous); #pragma warning restore CA2007 #pragma warning restore CAC001 + using IMemoryOwner buffer = MemoryPool.Shared.Rent(checked(fs.Length > 0 ? (int) fs.Length + 1 : 1 << 15)); + int read = await fs.ReadAsync(buffer.Memory, cancellationToken).ConfigureAwait(false); - // Use JsonSerializerOptions.PropertyNamingPolicy to specify the JSON property naming convention - await JsonSerializer.SerializeAsync(fs, options, cancellationToken: cancellationToken).ConfigureAwait(false); - fs.SetLength(fs.Position); + try { + fs.Position = 0; + fs.SetLength(0); + int written = await ASFFreeGamesOptionsSaver.SaveOptions(fs, options, true, cancellationToken).ConfigureAwait(false); + fs.SetLength(written); + } + + catch (Exception) { + fs.Position = 0; + await fs.WriteAsync(buffer.Memory[..read], cancellationToken).ConfigureAwait(false); + fs.SetLength(read); + + throw; + } } finally { Semaphore.Release(); diff --git a/ASFFreeGames/Configurations/ASFFreeGamesOptionsSaver.cs b/ASFFreeGames/Configurations/ASFFreeGamesOptionsSaver.cs new file mode 100644 index 0000000..dededbe --- /dev/null +++ b/ASFFreeGames/Configurations/ASFFreeGamesOptionsSaver.cs @@ -0,0 +1,220 @@ +using System; +using System.Buffers; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Globalization; +using System.IO; +using System.Runtime.CompilerServices; +using System.Text; +using System.Text.Json; +using System.Text.Json.Nodes; +using System.Threading; +using System.Threading.Tasks; + +#nullable enable +namespace ASFFreeGames.Configurations; + +public static class ASFFreeGamesOptionsSaver { + public static async Task SaveOptions([NotNull] Stream stream, [NotNull] ASFFreeGamesOptions options, bool checkValid = true, CancellationToken cancellationToken = default) { + using IMemoryOwner memory = MemoryPool.Shared.Rent(1 << 15); + int written = CreateOptionsBuffer(options, memory); + + if (checkValid) { + PseudoValidate(memory, written); + } + + await stream.WriteAsync(memory.Memory[..written], cancellationToken).ConfigureAwait(false); + + return written; + } + + private static void PseudoValidate(IMemoryOwner memory, int written) { + JsonNode? doc = JsonNode.Parse(Encoding.UTF8.GetString(memory.Memory[..written].Span)); + + doc?["skipFreeToPlay"]?.GetValue(); + } + + internal static int CreateOptionsBuffer(ASFFreeGamesOptions options, IMemoryOwner memory) { + Span buffer = memory.Memory.Span; + buffer.Clear(); + + int written = 0; + written += WriteJsonString("{\n"u8, buffer, written); + + written += WriteNameAndProperty("recheckInterval"u8, options.RecheckInterval, buffer, written); + written += WriteNameAndProperty("randomizeRecheckInterval"u8, options.RandomizeRecheckInterval, buffer, written); + written += WriteNameAndProperty("skipFreeToPlay"u8, options.SkipFreeToPlay, buffer, written); + written += WriteNameAndProperty("skipDLC"u8, options.SkipDLC, buffer, written); + written += WriteNameAndProperty("blacklist"u8, options.Blacklist, buffer, written); + written += WriteNameAndProperty("verboseLog"u8, options.VerboseLog, buffer, written); + written += WriteNameAndProperty("proxy"u8, options.Proxy, buffer, written); + written += WriteNameAndProperty("redditProxy"u8, options.RedditProxy, buffer, written); + RemoveTrailingCommaAndLineReturn(buffer, ref written); + + written += WriteJsonString("\n}"u8, buffer, written); + + // Resize buffer if needed + if (written >= buffer.Length) { + throw new InvalidOperationException("Buffer overflow while saving options"); + } + + return written; + } + + private static void RemoveTrailingCommaAndLineReturn(Span buffer, ref int written) { + int c; + + do { + c = RemoveTrailing(buffer, "\n"u8, ref written); + c += RemoveTrailing(buffer, ","u8, ref written); + } while (c > 0); + } + + private static int RemoveTrailing(Span buffer, ReadOnlySpan target, ref int written) { + Span sub = buffer[..written]; + int c = 0; + + while (!sub.IsEmpty) { + if (sub.EndsWith(target)) { + written -= target.Length; + sub = sub[..written]; + c += 1; + } + else { + break; + } + } + + return c; + } + + [MethodImpl(MethodImplOptions.AggressiveOptimization)] + private static int WriteEscapedJsonString(string str, Span buffer, int written) { + const byte quote = (byte) '"'; + const byte backslash = (byte) '\\'; + + int startIndex = written; + buffer[written++] = quote; + Span cstr = stackalloc char[1]; + ReadOnlySpan span = str.AsSpan(); + + // ReSharper disable once ForCanBeConvertedToForeach + for (int index = 0; index < span.Length; index++) { + char c = span[index]; + + switch (c) { + case '"': + buffer[written++] = backslash; + buffer[written++] = quote; + + break; + case '\\': + buffer[written++] = backslash; + buffer[written++] = backslash; + + break; + case '\b': + buffer[written++] = backslash; + buffer[written++] = (byte) 'b'; + + break; + case '\f': + buffer[written++] = backslash; + buffer[written++] = (byte) 'f'; + + break; + case '\n': + buffer[written++] = backslash; + buffer[written++] = (byte) 'n'; + + break; + case '\r': + buffer[written++] = backslash; + buffer[written++] = (byte) 'r'; + + break; + case '\t': + buffer[written++] = backslash; + buffer[written++] = (byte) 't'; + + break; + default: + // Optimize for common case of ASCII characters + if (c < 128) { + buffer[written++] = (byte) c; + } + else { + cstr[0] = c; + written += WriteJsonString(cstr, buffer, written); + } + + break; + } + } + + buffer[written++] = quote; + + return written - startIndex; + } + + [MethodImpl(MethodImplOptions.AggressiveOptimization)] + private static int WriteNameAndProperty(ReadOnlySpan name, T value, Span buffer, int written) { + int startIndex = written; + written += WriteJsonString("\""u8, buffer, written); + written += WriteJsonString(name, buffer, written); + written += WriteJsonString("\": "u8, buffer, written); + + if (value is null) { + written += WriteJsonString("null"u8, buffer, written); + } + else { + written += value switch { + string str => WriteEscapedJsonString(str, buffer, written), +#pragma warning disable CA1308 + bool b => WriteJsonString(b ? "true"u8 : "false"u8, buffer, written), +#pragma warning restore CA1308 + IReadOnlyCollection collection => WriteJsonArray(collection, buffer, written), + TimeSpan timeSpan => WriteEscapedJsonString(timeSpan.ToString(), buffer, written), + _ => throw new ArgumentException($"Unsupported type for property {Encoding.UTF8.GetString(name)}: {value.GetType()}") + }; + } + + written += WriteJsonString(","u8, buffer, written); + written += WriteJsonString("\n"u8, buffer, written); + + return written - startIndex; + } + + private static int WriteJsonArray(IEnumerable collection, Span buffer, int written) { + int startIndex = written; + written += WriteJsonString("["u8, buffer, written); + bool first = true; + + foreach (string item in collection) { + if (!first) { + written += WriteJsonString(","u8, buffer, written); + } + + written += WriteEscapedJsonString(item, buffer, written); + first = false; + } + + written += WriteJsonString("]"u8, buffer, written); + + return written - startIndex; + } + + [MethodImpl(MethodImplOptions.AggressiveOptimization | MethodImplOptions.AggressiveInlining)] + private static int WriteJsonString(ReadOnlySpan str, Span buffer, int written) { + str.CopyTo(buffer[written..(written + str.Length)]); + + return str.Length; + } + + [MethodImpl(MethodImplOptions.AggressiveOptimization | MethodImplOptions.AggressiveInlining)] + private static int WriteJsonString(ReadOnlySpan str, Span buffer, int written) { + int encodedLength = Encoding.UTF8.GetBytes(str, buffer[written..]); + + return encodedLength; + } +} diff --git a/ASFFreeGames/HttpClientSimple/SimpleHttpClient.cs b/ASFFreeGames/HttpClientSimple/SimpleHttpClient.cs index 248c992..2d67380 100644 --- a/ASFFreeGames/HttpClientSimple/SimpleHttpClient.cs +++ b/ASFFreeGames/HttpClientSimple/SimpleHttpClient.cs @@ -4,6 +4,7 @@ using System.Net; using System.Net.Http; using System.Net.Http.Headers; +using System.Reflection; using System.Runtime.CompilerServices; using System.Text; using System.Threading; @@ -20,8 +21,11 @@ public sealed class SimpleHttpClient : IDisposable { public SimpleHttpClient(IWebProxy? proxy = null, long timeout = 25_000) { HttpClientHandler = new HttpClientHandler { AutomaticDecompression = DecompressionMethods.All, + MaxConnectionsPerServer = 5 }; + SetCheckCertificateRevocationList(HttpClientHandler, true); + if (proxy is not null) { HttpClientHandler.Proxy = proxy; HttpClientHandler.UseProxy = true; @@ -31,9 +35,14 @@ public SimpleHttpClient(IWebProxy? proxy = null, long timeout = 25_000) { } } - HttpClient = new HttpClient(HttpClientHandler, false) { Timeout = TimeSpan.FromMilliseconds(timeout) }; - HttpClient.DefaultRequestVersion = HttpVersion.Version30; - HttpClient.DefaultRequestHeaders.ExpectContinue = false; +#pragma warning disable CA5399 + HttpClient = new HttpClient(HttpClientHandler, false) { + DefaultRequestVersion = HttpVersion.Version30, + Timeout = TimeSpan.FromMilliseconds(timeout) + }; +#pragma warning restore CA5399 + + SetExpectContinueProperty(HttpClient, false); HttpClient.DefaultRequestHeaders.Add("User-Agent", "Lynx/2.8.8dev.9 libwww-FM/2.14 SSL-MM/1.4.1 GNUTLS/2.12.14"); HttpClient.DefaultRequestHeaders.Add("DNT", "1"); @@ -50,12 +59,23 @@ public async Task GetStreamAsync(Uri uri, IEnumerable header in additionalHeaders) { - request.Headers.Add(header.Key, header.Value); + request.Headers.TryAddWithoutValidation(header.Key, header.Value); } } HttpResponseMessage response = await HttpClient.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, cancellationToken).ConfigureAwait(false); - Stream stream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); + Stream? stream = null; + + try { + stream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); + } + catch (Exception) { + if (response.IsSuccessStatusCode) { + throw; // something is wrong + } + + // assume that the caller checks the status code before reading the stream + } return new HttpStreamResponse(response, stream); } @@ -64,11 +84,66 @@ public void Dispose() { HttpClient.Dispose(); HttpClientHandler.Dispose(); } + + # region System.MissingMethodException workaround + private static bool SetCheckCertificateRevocationList(HttpClientHandler httpClientHandler, bool value) { + try { + // Get the type of HttpClientHandler + Type httpClientHandlerType = httpClientHandler.GetType(); + + // Get the property information + PropertyInfo? propertyInfo = httpClientHandlerType.GetProperty("CheckCertificateRevocationList", BindingFlags.Public | BindingFlags.Instance); + + if ((propertyInfo is not null) && propertyInfo.CanWrite) { + // Set the property value + propertyInfo.SetValue(httpClientHandler, true); + + return true; + } + } + catch (Exception ex) { + ArchiSteamFarm.Core.ASF.ArchiLogger.LogGenericException(ex); + } + + return false; + } + + private static bool SetExpectContinueProperty(HttpClient httpClient, bool value) { + try { + // Get the DefaultRequestHeaders property + PropertyInfo? defaultRequestHeadersProperty = httpClient.GetType().GetProperty("DefaultRequestHeaders", BindingFlags.Public | BindingFlags.Instance); + + if (defaultRequestHeadersProperty == null) { + throw new InvalidOperationException("HttpClient does not have DefaultRequestHeaders property."); + } + + if (defaultRequestHeadersProperty.GetValue(httpClient) is not HttpRequestHeaders defaultRequestHeaders) { + throw new InvalidOperationException("DefaultRequestHeaders is null."); + } + + // Get the ExpectContinue property + PropertyInfo? expectContinueProperty = defaultRequestHeaders.GetType().GetProperty("ExpectContinue", BindingFlags.Public | BindingFlags.Instance); + + if ((expectContinueProperty != null) && expectContinueProperty.CanWrite) { + expectContinueProperty.SetValue(defaultRequestHeaders, value); + + return true; + } + } + catch (Exception ex) { + ArchiSteamFarm.Core.ASF.ArchiLogger.LogGenericException(ex); + } + + return false; + } + #endregion } -public sealed class HttpStreamResponse(HttpResponseMessage response, Stream stream) : IAsyncDisposable { +public sealed class HttpStreamResponse(HttpResponseMessage response, Stream? stream) : IAsyncDisposable { public HttpResponseMessage Response { get; } = response; - public Stream Stream { get; } = stream; + public Stream Stream { get; } = stream ?? EmptyStreamLazy.Value; + + public bool HasValidStream => stream is not null && (!EmptyStreamLazy.IsValueCreated || !ReferenceEquals(EmptyStreamLazy.Value, Stream)); public async Task ReadAsStringAsync(CancellationToken cancellationToken) { using StreamReader reader = new(Stream, Encoding.UTF8); @@ -79,8 +154,10 @@ public async Task ReadAsStringAsync(CancellationToken cancellationToken) public HttpStatusCode StatusCode => Response.StatusCode; public async ValueTask DisposeAsync() { - ConfiguredValueTaskAwaitable task = Stream.DisposeAsync().ConfigureAwait(false); + ConfiguredValueTaskAwaitable task = HasValidStream ? Stream.DisposeAsync().ConfigureAwait(false) : ValueTask.CompletedTask.ConfigureAwait(false); Response.Dispose(); await task; } + + private static readonly Lazy EmptyStreamLazy = new(static () => new MemoryStream([], false)); }