From 109738660f6b6b7f66d40876803f78132c2f8a00 Mon Sep 17 00:00:00 2001 From: Andrew Nekowitsch Date: Mon, 4 May 2026 16:52:15 -0500 Subject: [PATCH] Refactor Trakt settings validation and add unit tests for rating and year filters --- global.json | 2 +- .../Trakt/TraktValidationFixture.cs | 137 ++++++++++++++++++ .../Trakt/List/TraktListSettings.cs | 3 +- .../Trakt/Popular/TraktPopularSettings.cs | 10 +- .../ImportLists/Trakt/TraktQueryHelper.cs | 37 ++++- .../ImportLists/Trakt/TraktSettingsBase.cs | 66 ++++++++- .../Trakt/User/TraktUserSettings.cs | 8 +- 7 files changed, 236 insertions(+), 27 deletions(-) create mode 100644 src/NzbDrone.Core.Test/ImportListTests/Trakt/TraktValidationFixture.cs diff --git a/global.json b/global.json index c2af57a3f..058bafa62 100644 --- a/global.json +++ b/global.json @@ -1,5 +1,5 @@ { "sdk": { - "version": "10.0.102" + "version": "10.0.103" } } diff --git a/src/NzbDrone.Core.Test/ImportListTests/Trakt/TraktValidationFixture.cs b/src/NzbDrone.Core.Test/ImportListTests/Trakt/TraktValidationFixture.cs new file mode 100644 index 000000000..8a29e948e --- /dev/null +++ b/src/NzbDrone.Core.Test/ImportListTests/Trakt/TraktValidationFixture.cs @@ -0,0 +1,137 @@ +using System; +using FluentAssertions; +using NUnit.Framework; +using NzbDrone.Core.ImportLists.Trakt; +using NzbDrone.Core.ImportLists.Trakt.List; +using NzbDrone.Core.ImportLists.Trakt.Popular; +using NzbDrone.Core.ImportLists.Trakt.User; +using NzbDrone.Core.Test.Framework; + +namespace NzbDrone.Core.Test.ImportListTests.Trakt +{ + [TestFixture] + public class TraktValidationFixture : CoreTest + { + [TestCase("0-100")] + [TestCase("50-50")] + [TestCase("100-100")] + public void should_accept_supported_rating_ranges(string rating) + { + CreateValidListSettings(rating: rating).Validate().IsValid.Should().BeTrue(); + CreateValidPopularSettings(rating: rating).Validate().IsValid.Should().BeTrue(); + CreateValidUserSettings(rating: rating).Validate().IsValid.Should().BeTrue(); + } + + [TestCase("10")] + [TestCase("10-5")] + [TestCase("00-10")] + [TestCase("10-101")] + [TestCase("1000-1000")] + [TestCase("10 - 20")] + public void should_reject_invalid_rating_ranges(string rating) + { + CreateValidListSettings(rating: rating).Validate().IsValid.Should().BeFalse(); + CreateValidPopularSettings(rating: rating).Validate().IsValid.Should().BeFalse(); + CreateValidUserSettings(rating: rating).Validate().IsValid.Should().BeFalse(); + } + + [TestCase("1990")] + [TestCase("1990-2000")] + public void should_accept_supported_year_filters(string years) + { + CreateValidListSettings(years: years).Validate().IsValid.Should().BeTrue(); + CreateValidPopularSettings(years: years).Validate().IsValid.Should().BeTrue(); + CreateValidUserSettings(years: years).Validate().IsValid.Should().BeTrue(); + } + + [TestCase("234923498237423-234723477")] + [TestCase("199")] + [TestCase("1990-1980")] + [TestCase("1990 - 2000")] + public void should_reject_invalid_year_filters(string years) + { + CreateValidListSettings(years: years).Validate().IsValid.Should().BeFalse(); + CreateValidPopularSettings(years: years).Validate().IsValid.Should().BeFalse(); + CreateValidUserSettings(years: years).Validate().IsValid.Should().BeFalse(); + } + + [TestCase("genres=comedy")] + [TestCase("ratings=80-100")] + [TestCase("years=1990-2000")] + [TestCase("limit=10")] + public void should_reject_reserved_additional_parameters(string additionalParameters) + { + CreateValidListSettings(additionalParameters: additionalParameters).Validate().IsValid.Should().BeFalse(); + CreateValidPopularSettings(additionalParameters: additionalParameters).Validate().IsValid.Should().BeFalse(); + CreateValidUserSettings(additionalParameters: additionalParameters).Validate().IsValid.Should().BeFalse(); + } + + [Test] + public void should_allow_non_reserved_additional_parameters() + { + CreateValidListSettings(additionalParameters: "languages=en").Validate().IsValid.Should().BeTrue(); + CreateValidPopularSettings(additionalParameters: "languages=en").Validate().IsValid.Should().BeTrue(); + CreateValidUserSettings(additionalParameters: "languages=en").Validate().IsValid.Should().BeTrue(); + } + + [Test] + public void should_ignore_reserved_additional_parameters_when_building_filter_parameters() + { + var parameters = TraktQueryHelper.BuildFilterParameters( + "80-100", + "Drama", + "1990-2000", + 25, + "genres=comedy&ratings=10-20&years=2000-2010&limit=10&languages=en"); + + parameters.Should().ContainKey("genres").WhoseValue.Should().Be("drama"); + parameters.Should().ContainKey("ratings").WhoseValue.Should().Be("80-100"); + parameters.Should().ContainKey("years").WhoseValue.Should().Be("1990-2000"); + parameters.Should().ContainKey("limit").WhoseValue.Should().Be("25"); + parameters.Should().ContainKey("languages").WhoseValue.Should().Be("en"); + parameters.Should().HaveCount(5); + } + + private static TraktListSettings CreateValidListSettings(string rating = "80-100", string years = "1990-2000", string additionalParameters = "languages=en") + { + return new TraktListSettings + { + AccessToken = "access-token", + RefreshToken = "refresh-token", + Expires = DateTime.UtcNow.AddDays(1), + Username = "sonarr", + Listname = "watchlist", + Rating = rating, + Years = years, + TraktAdditionalParameters = additionalParameters + }; + } + + private static TraktPopularSettings CreateValidPopularSettings(string rating = "80-100", string years = "1990-2000", string additionalParameters = "languages=en") + { + return new TraktPopularSettings + { + AccessToken = "access-token", + RefreshToken = "refresh-token", + Expires = DateTime.UtcNow.AddDays(1), + Rating = rating, + Years = years, + TraktAdditionalParameters = additionalParameters + }; + } + + private static TraktUserSettings CreateValidUserSettings(string rating = "80-100", string years = "1990-2000", string additionalParameters = "languages=en") + { + return new TraktUserSettings + { + AccessToken = "access-token", + RefreshToken = "refresh-token", + Expires = DateTime.UtcNow.AddDays(1), + AuthUser = "sonarr-user", + Rating = rating, + Years = years, + TraktAdditionalParameters = additionalParameters + }; + } + } +} \ No newline at end of file diff --git a/src/NzbDrone.Core/ImportLists/Trakt/List/TraktListSettings.cs b/src/NzbDrone.Core/ImportLists/Trakt/List/TraktListSettings.cs index 34f0fa223..4d3100f24 100644 --- a/src/NzbDrone.Core/ImportLists/Trakt/List/TraktListSettings.cs +++ b/src/NzbDrone.Core/ImportLists/Trakt/List/TraktListSettings.cs @@ -1,4 +1,3 @@ -using System.Text.RegularExpressions; using FluentValidation; using NzbDrone.Common.Extensions; using NzbDrone.Core.Annotations; @@ -14,7 +13,7 @@ public TraktListSettingsValidator() RuleFor(c => c.Listname).NotEmpty(); RuleFor(c => c.Years) - .Matches(@"^\d+(\-\d+)?$", RegexOptions.IgnoreCase) + .Must(BeValidYearRange) .When(c => c.Years.IsNotNullOrWhiteSpace()) .WithMessage("Not a valid year or range of years"); } diff --git a/src/NzbDrone.Core/ImportLists/Trakt/Popular/TraktPopularSettings.cs b/src/NzbDrone.Core/ImportLists/Trakt/Popular/TraktPopularSettings.cs index 0dcc34b44..1a8b2f227 100644 --- a/src/NzbDrone.Core/ImportLists/Trakt/Popular/TraktPopularSettings.cs +++ b/src/NzbDrone.Core/ImportLists/Trakt/Popular/TraktPopularSettings.cs @@ -1,4 +1,3 @@ -using System.Text.RegularExpressions; using FluentValidation; using NzbDrone.Common.Extensions; using NzbDrone.Core.Annotations; @@ -19,15 +18,8 @@ public TraktPopularSettingsValidator() .WithMessage("Yearly lists are no longer supported"); #pragma warning restore CS0612 - // Loose validation @TODO - RuleFor(c => c.Rating) - .Matches(@"^\d+\-\d+$", RegexOptions.IgnoreCase) - .When(c => c.Rating.IsNotNullOrWhiteSpace()) - .WithMessage("Not a valid rating"); - - // Loose validation @TODO RuleFor(c => c.Years) - .Matches(@"^\d+(\-\d+)?$", RegexOptions.IgnoreCase) + .Must(BeValidYearRange) .When(c => c.Years.IsNotNullOrWhiteSpace()) .WithMessage("Not a valid year or range of years"); } diff --git a/src/NzbDrone.Core/ImportLists/Trakt/TraktQueryHelper.cs b/src/NzbDrone.Core/ImportLists/Trakt/TraktQueryHelper.cs index 003b2872d..133068365 100644 --- a/src/NzbDrone.Core/ImportLists/Trakt/TraktQueryHelper.cs +++ b/src/NzbDrone.Core/ImportLists/Trakt/TraktQueryHelper.cs @@ -7,11 +7,18 @@ namespace NzbDrone.Core.ImportLists.Trakt { public static class TraktQueryHelper { + private static readonly HashSet ReservedFilterParameters = new(StringComparer.OrdinalIgnoreCase) + { + "genres", + "ratings", + "years", + "limit" + }; + public static Dictionary BuildFilterParameters(string rating, string genres, string years, int limit, string additionalParameters) { var parameters = new Dictionary(StringComparer.OrdinalIgnoreCase); - // Parse additional parameters first (lower priority) if (additionalParameters.IsNotNullOrWhiteSpace()) { var trimmed = additionalParameters.TrimStart('?').TrimStart('&'); @@ -24,11 +31,7 @@ public static Dictionary BuildFilterParameters(string rating, st { var key = parts[0].Trim(); - // Skip explicitly handled parameters - if (key.Equals("genres", StringComparison.OrdinalIgnoreCase) || - key.Equals("ratings", StringComparison.OrdinalIgnoreCase) || - key.Equals("years", StringComparison.OrdinalIgnoreCase) || - key.Equals("limit", StringComparison.OrdinalIgnoreCase)) + if (ReservedFilterParameters.Contains(key)) { continue; } @@ -58,6 +61,28 @@ public static Dictionary BuildFilterParameters(string rating, st return parameters; } + public static bool ContainsReservedFilterParameters(string additionalParameters) + { + if (additionalParameters.IsNullOrWhiteSpace()) + { + return false; + } + + var trimmed = additionalParameters.TrimStart('?').TrimStart('&'); + + foreach (var param in trimmed.Split('&', StringSplitOptions.RemoveEmptyEntries)) + { + var parts = param.Split('=', 2); + + if (parts.Length == 2 && parts[0].IsNotNullOrWhiteSpace() && ReservedFilterParameters.Contains(parts[0].Trim())) + { + return true; + } + } + + return false; + } + public static string ToQueryString(this Dictionary parameters) { return string.Join("&", parameters.Where(p => p.Value.IsNotNullOrWhiteSpace()).Select(p => $"{p.Key}={p.Value}")); diff --git a/src/NzbDrone.Core/ImportLists/Trakt/TraktSettingsBase.cs b/src/NzbDrone.Core/ImportLists/Trakt/TraktSettingsBase.cs index 73aa8d977..c7757fbad 100644 --- a/src/NzbDrone.Core/ImportLists/Trakt/TraktSettingsBase.cs +++ b/src/NzbDrone.Core/ImportLists/Trakt/TraktSettingsBase.cs @@ -1,5 +1,5 @@ using System; -using System.Text.RegularExpressions; +using System.Globalization; using FluentValidation; using NzbDrone.Common.Extensions; using NzbDrone.Core.Annotations; @@ -34,9 +34,71 @@ public TraktSettingsBaseValidator() .WithMessage("Must be integer greater than 0"); RuleFor(c => c.Rating) - .Matches(@"^\d+\-\d+$", RegexOptions.IgnoreCase) + .Must(BeValidRatingRange) .When(c => c.Rating.IsNotNullOrWhiteSpace()) .WithMessage("Not a valid rating"); + + RuleFor(c => c.TraktAdditionalParameters) + .Must(additionalParameters => !TraktQueryHelper.ContainsReservedFilterParameters(additionalParameters)) + .When(c => c.TraktAdditionalParameters.IsNotNullOrWhiteSpace()) + .WithMessage("Additional parameters cannot include genres, ratings, years, or limit"); + } + + protected static bool BeValidYearRange(string years) + { + var parts = years.Split('-', StringSplitOptions.None); + + if (parts.Length == 1) + { + return TryParseYear(parts[0], out _); + } + + if (parts.Length != 2) + { + return false; + } + + return TryParseYear(parts[0], out var startYear) && + TryParseYear(parts[1], out var endYear) && + startYear <= endYear; + } + + private static bool BeValidRatingRange(string rating) + { + var parts = rating.Split('-', StringSplitOptions.None); + + if (parts.Length != 2) + { + return false; + } + + return TryParseRating(parts[0], out var minimumRating) && + TryParseRating(parts[1], out var maximumRating) && + minimumRating <= maximumRating; + } + + private static bool TryParseYear(string token, out int year) + { + year = default; + + return token.Length == 4 && + int.TryParse(token, NumberStyles.None, CultureInfo.InvariantCulture, out year) && + year >= 1000; + } + + private static bool TryParseRating(string token, out int rating) + { + if (!int.TryParse(token, NumberStyles.None, CultureInfo.InvariantCulture, out rating)) + { + return false; + } + + if (rating is < 0 or > 100) + { + return false; + } + + return token == rating.ToString(CultureInfo.InvariantCulture); } } diff --git a/src/NzbDrone.Core/ImportLists/Trakt/User/TraktUserSettings.cs b/src/NzbDrone.Core/ImportLists/Trakt/User/TraktUserSettings.cs index 1309a4e0e..32e55cf40 100644 --- a/src/NzbDrone.Core/ImportLists/Trakt/User/TraktUserSettings.cs +++ b/src/NzbDrone.Core/ImportLists/Trakt/User/TraktUserSettings.cs @@ -1,4 +1,3 @@ -using System.Text.RegularExpressions; using FluentValidation; using NzbDrone.Common.Extensions; using NzbDrone.Core.Annotations; @@ -14,13 +13,8 @@ public TraktUserSettingsValidator() RuleFor(c => c.TraktWatchedListType).NotNull(); RuleFor(c => c.AuthUser).NotEmpty(); - RuleFor(c => c.Rating) - .Matches(@"^\d+\-\d+$", RegexOptions.IgnoreCase) - .When(c => c.Rating.IsNotNullOrWhiteSpace()) - .WithMessage("Not a valid rating"); - RuleFor(c => c.Years) - .Matches(@"^\d+(\-\d+)?$", RegexOptions.IgnoreCase) + .Must(BeValidYearRange) .When(c => c.Years.IsNotNullOrWhiteSpace()) .WithMessage("Not a valid year or range of years"); }