From 98042f80404107bfa24dfc21e84a70eb85b8119a Mon Sep 17 00:00:00 2001 From: MentalBlank Date: Sun, 25 May 2025 21:50:26 +1000 Subject: [PATCH] Improved enrichment validation, logging, and test coverage. Strengthened URL Validation. Improved HTTP Client Configuration. Logged total rejected tracker count at the information level. Logged individual rejected tracker details at the debug level. Debug logs for bytes and enriched bytes are now conditional. Converted some tests from Fact to Theory. Used Assert.Collection in EnricherTest.cs. Standardized URL Encoding in Enricher.cs. --- .../Services/EnricherTest.cs | 89 +++++++++++++------ server/RdtClient.Service/Services/Enricher.cs | 3 +- server/RdtClient.Service/Services/Torrents.cs | 21 +++-- .../Services/TrackerListGrabber.cs | 47 +++++++--- 4 files changed, 114 insertions(+), 46 deletions(-) diff --git a/server/RdtClient.Service.Test/Services/EnricherTest.cs b/server/RdtClient.Service.Test/Services/EnricherTest.cs index ebdd84e..dfd1933 100644 --- a/server/RdtClient.Service.Test/Services/EnricherTest.cs +++ b/server/RdtClient.Service.Test/Services/EnricherTest.cs @@ -41,19 +41,30 @@ public class EnricherTest : IDisposable Assert.Equal(TestMagnetLink, enriched); } - [Fact] - public async Task EnrichMagnetLink_AddsTrackers_WhenTrackersFromTrackerGrabber() + [Theory] + [InlineData("magnet:?xt=urn:btih:HASH", + new[] + { + "http://new-tracker.com/announce" + }, + "magnet:?xt=urn:btih:HASH&tr=http%3A%2F%2Fnew-tracker.com%2Fannounce")] + [InlineData("magnet:?xt=urn:btih:HASH", + new[] + { + "http://tracker1.com/announce", "http://tracker2.com/announce" + }, + "magnet:?xt=urn:btih:HASH&tr=http%3A%2F%2Ftracker1.com%2Fannounce&tr=http%3A%2F%2Ftracker2.com%2Fannounce")] + public async Task EnrichMagnetLink_AddsTrackers_WhenTrackersFromTrackerGrabber_Theory(String magnetLink, String[] newTrackers, String expectedResult) { // Arrange - SetupTrackerListGrabber(["http://new-tracker.com/announce"]); - + SetupTrackerListGrabber(newTrackers); var enricher = new Enricher(_loggerMock.Object, _trackerListGrabberMock.Object); // Act - var enriched = await enricher.EnrichMagnetLink(TestMagnetLink); + var enriched = await enricher.EnrichMagnetLink(magnetLink); // Assert - Assert.Equal(TestMagnetLink + $"&tr={Uri.EscapeDataString("http://new-tracker.com/announce")}", enriched); + Assert.Equal(expectedResult, enriched); } [Fact] @@ -281,17 +292,27 @@ public class EnricherTest : IDisposable Assert.Equal(2, trValues.Count); } - [Fact] - public async Task EnrichMagnetLink_BasicMagnetWithHashOnly() + [Theory] + [InlineData("magnet:?xt=urn:btih:HASH", + new[] + { + "udp://tracker1" + }, + "magnet:?xt=urn:btih:HASH&tr=udp%3A%2F%2Ftracker1")] + [InlineData("magnet:?xt=urn:btih:HASH", + new[] + { + "udp://tracker1", "udp://tracker2" + }, + "magnet:?xt=urn:btih:HASH&tr=udp%3A%2F%2Ftracker1&tr=udp%3A%2F%2Ftracker2")] + [InlineData("magnet:?xt=urn:btih:HASH", + new String[] + { + }, + "magnet:?xt=urn:btih:HASH")] + public async Task EnrichMagnetLink_BasicMagnetWithHashOnly_Theory(String magnetLink, String[] newTrackers, String expectedResult) { // Arrange - var magnetLink = "magnet:?xt=urn:btih:HASH"; - - var newTrackers = new[] - { - "udp://tracker1" - }; - SetupTrackerListGrabber(newTrackers); var enricher = new Enricher(_loggerMock.Object, _trackerListGrabberMock.Object); @@ -299,20 +320,30 @@ public class EnricherTest : IDisposable var result = await enricher.EnrichMagnetLink(magnetLink); // Assert - Assert.Equal($"{magnetLink}&tr={Uri.EscapeDataString("udp://tracker1")}", result); + Assert.Equal(expectedResult, result); } - [Fact] - public async Task EnrichMagnetLink_MagnetEndsWithQuestionMarkOnly() + [Theory] + [InlineData("magnet:?", + new[] + { + "udp://tracker1" + }, + "magnet:?tr=udp%3A%2F%2Ftracker1")] + [InlineData("magnet:?", + new[] + { + "udp://tracker1", "udp://tracker2" + }, + "magnet:?tr=udp%3A%2F%2Ftracker1&tr=udp%3A%2F%2Ftracker2")] + [InlineData("magnet:?", + new String[] + { + }, + "magnet:?")] + public async Task EnrichMagnetLink_MagnetEndsWithQuestionMarkOnly_Theory(String magnetLink, String[] newTrackers, String expectedResult) { // Arrange - var magnetLink = "magnet:?"; - - var newTrackers = new[] - { - "udp://tracker1" - }; - SetupTrackerListGrabber(newTrackers); var enricher = new Enricher(_loggerMock.Object, _trackerListGrabberMock.Object); @@ -320,7 +351,7 @@ public class EnricherTest : IDisposable var result = await enricher.EnrichMagnetLink(magnetLink); // Assert - Assert.Equal($"{magnetLink}tr={Uri.EscapeDataString("udp://tracker1")}", result); + Assert.Equal(expectedResult, result); } [Fact] @@ -365,8 +396,10 @@ public class EnricherTest : IDisposable Assert.Equal(newTrackers[0], ((BEncodedString)enrichedDict["announce"]).Text); var announceList = (BEncodedList)enrichedDict["announce-list"]; Assert.Equal(2, announceList.Count); - Assert.Equal(newTrackers[0], ((BEncodedString)((BEncodedList)announceList[0])[0]).Text); - Assert.Equal(newTrackers[1], ((BEncodedString)((BEncodedList)announceList[1])[0]).Text); + + Assert.Collection(announceList, + tier => Assert.Equal(newTrackers[0], ((BEncodedString)((BEncodedList)tier)[0]).Text), + tier => Assert.Equal(newTrackers[1], ((BEncodedString)((BEncodedList)tier)[0]).Text)); } [Fact] diff --git a/server/RdtClient.Service/Services/Enricher.cs b/server/RdtClient.Service/Services/Enricher.cs index 222cdca..64a2965 100644 --- a/server/RdtClient.Service/Services/Enricher.cs +++ b/server/RdtClient.Service/Services/Enricher.cs @@ -1,4 +1,3 @@ - using System.Text; using System.Web; using Microsoft.Extensions.Logging; @@ -108,7 +107,7 @@ public sealed class Enricher(ILogger logger, ITrackerListGrabber track } else { - outParams.Add($"{kv.Key}={HttpUtility.UrlEncode(v)}"); + outParams.Add($"{kv.Key}={Uri.EscapeDataString(v)}"); } } } diff --git a/server/RdtClient.Service/Services/Torrents.cs b/server/RdtClient.Service/Services/Torrents.cs index 3ff2e69..7b77350 100644 --- a/server/RdtClient.Service/Services/Torrents.cs +++ b/server/RdtClient.Service/Services/Torrents.cs @@ -115,7 +115,7 @@ public class Torrents( MagnetLink magnet; try { - magnet = MagnetLink.Parse(enriched); + magnet = MagnetLink.Parse(magnetLink); } catch (Exception ex) { @@ -139,15 +139,24 @@ public class Torrents( { MonoTorrent.Torrent monoTorrent; - var fileAsBase64 = Convert.ToBase64String(bytes); - logger.LogDebug($"bytes {bytes}"); - var enriched = await enricher.EnrichTorrentBytes(bytes); - logger.LogDebug($"enriched bytes {enriched}"); + + String fileAsBase64; + + if (enriched.SequenceEqual(bytes)) + { + fileAsBase64 = Convert.ToBase64String(bytes); + logger.LogDebug($"bytes {bytes}"); + } + else + { + fileAsBase64 = Convert.ToBase64String(enriched); + logger.LogDebug($"enriched bytes {enriched}"); + } try { - monoTorrent = await MonoTorrent.Torrent.LoadAsync(enriched); + monoTorrent = await MonoTorrent.Torrent.LoadAsync(bytes); } catch (Exception ex) { diff --git a/server/RdtClient.Service/Services/TrackerListGrabber.cs b/server/RdtClient.Service/Services/TrackerListGrabber.cs index 3ccc049..7e49b32 100644 --- a/server/RdtClient.Service/Services/TrackerListGrabber.cs +++ b/server/RdtClient.Service/Services/TrackerListGrabber.cs @@ -1,5 +1,7 @@ +using System.Net.Http.Headers; using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Logging; +using System.Reflection; namespace RdtClient.Service.Services; @@ -107,9 +109,17 @@ public class TrackerListGrabber(IHttpClientFactory httpClientFactory, IMemoryCac logger.LogDebug("Fetching tracker list from URL: {TrackerUrl}", trackerUri); var httpClient = httpClientFactory.CreateClient(); + + var version = Assembly.GetEntryAssembly()?.GetName().Version?.ToString(); + + var currentVersion = version != null && version.LastIndexOf('.') > 0 + ? $"v{version[..version.LastIndexOf('.')]}" + : ""; + + httpClient.DefaultRequestHeaders.UserAgent.Add(new ProductInfoHeaderValue("RdtClient", currentVersion)); using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(15)); var token = cts.Token; - var response = await httpClient.GetAsync(trackerUri, token).ConfigureAwait(false); + var response = await httpClient.GetAsync(trackerUri, HttpCompletionOption.ResponseHeadersRead, token).ConfigureAwait(false); using (response) { @@ -123,6 +133,8 @@ public class TrackerListGrabber(IHttpClientFactory httpClientFactory, IMemoryCac try { + var trackerRejectionCount = 0; + trackers = result .Split([ "\r\n", "\n" @@ -135,21 +147,36 @@ public class TrackerListGrabber(IHttpClientFactory httpClientFactory, IMemoryCac { if (!Uri.TryCreate(t, UriKind.Absolute, out var uri)) { + logger.LogDebug("Rejected tracker: {TrackerUrl} - Reason: Invalid format or unsupported scheme.", t); + trackerRejectionCount++; return false; } - return (uri.Scheme == Uri.UriSchemeHttp || - uri.Scheme == Uri.UriSchemeHttps || - uri.Scheme.Equals("udp", StringComparison.OrdinalIgnoreCase) || - uri.Scheme.Equals("wss", StringComparison.OrdinalIgnoreCase)) && - !t.Contains("..") && - !t.Contains("\\") && - !t.Any(Char.IsControl) && - uri.Host.All(c => Char.IsLetterOrDigit(c) || c == '.' || c == '-' || c == '_' || c == ':') && - uri.Host.Length > 0; + var isIpv6 = uri.Host.StartsWith('[') && uri.Host.Contains(']'); + + var valid = ((isIpv6 && uri.Host.All(c => Char.IsLetterOrDigit(c) || c == '.' || c == ':' || c == '[' || c == ']')) || + (!isIpv6 && uri.Host.All(c => Char.IsLetterOrDigit(c) || c == '.' || c == '-' || c == '_'))) && + (uri.Scheme == Uri.UriSchemeHttp || + uri.Scheme == Uri.UriSchemeHttps || + uri.Scheme.Equals("udp", StringComparison.OrdinalIgnoreCase) || + uri.Scheme.Equals("wss", StringComparison.OrdinalIgnoreCase)) && + !t.Contains("..") && + !t.Contains("\\") && + !t.Any(Char.IsControl) && + uri.Host.Length > 0; + + if (!valid) + { + logger.LogDebug("Enrichment tracker rejected: {TrackerUrl} - Reason: Invalid format or unsupported scheme.", t); + trackerRejectionCount++; + } + + return valid; }) .Distinct(StringComparer.OrdinalIgnoreCase) .ToArray(); + + logger.LogInformation("{TrackerRejectionCount} trackers were rejected during enrichment.", trackerRejectionCount); } catch (Exception ex)