From 625c538f244519700f3942b2b2969845db9a99b0 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Tue, 5 Jun 2018 20:22:46 -0400 Subject: move manifest parsing into toolkit (#532) --- src/SMAPI.Tests/Utilities/SemanticVersionTests.cs | 1 + 1 file changed, 1 insertion(+) (limited to 'src/SMAPI.Tests/Utilities') diff --git a/src/SMAPI.Tests/Utilities/SemanticVersionTests.cs b/src/SMAPI.Tests/Utilities/SemanticVersionTests.cs index f1a72012..b797393b 100644 --- a/src/SMAPI.Tests/Utilities/SemanticVersionTests.cs +++ b/src/SMAPI.Tests/Utilities/SemanticVersionTests.cs @@ -3,6 +3,7 @@ using System.Diagnostics.CodeAnalysis; using Newtonsoft.Json; using NUnit.Framework; using StardewModdingAPI.Framework; +using StardewModdingAPI.Toolkit.Serialisation.Models; namespace StardewModdingAPI.Tests.Utilities { -- cgit From 6eba10948bf39d5e05505ec060f6920f84610d58 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Tue, 5 Jun 2018 23:03:26 -0400 Subject: fix version parsing issues in new toolkit code (#532) --- src/SMAPI.Tests/Utilities/SemanticVersionTests.cs | 17 --------- .../Framework/Models/ManifestContentPackFor.cs | 2 +- .../Serialisation/SemanticVersionConverter.cs | 40 ++++++++++++++++++++++ src/SMAPI/Program.cs | 3 +- src/SMAPI/StardewModdingAPI.csproj | 1 + .../Converters/SemanticVersionConverter.cs | 5 ++- .../Serialisation/Models/LegacyManifestVersion.cs | 26 -------------- 7 files changed, 48 insertions(+), 46 deletions(-) create mode 100644 src/SMAPI/Framework/Serialisation/SemanticVersionConverter.cs delete mode 100644 src/StardewModdingAPI.Toolkit/Serialisation/Models/LegacyManifestVersion.cs (limited to 'src/SMAPI.Tests/Utilities') diff --git a/src/SMAPI.Tests/Utilities/SemanticVersionTests.cs b/src/SMAPI.Tests/Utilities/SemanticVersionTests.cs index b797393b..feab452a 100644 --- a/src/SMAPI.Tests/Utilities/SemanticVersionTests.cs +++ b/src/SMAPI.Tests/Utilities/SemanticVersionTests.cs @@ -3,7 +3,6 @@ using System.Diagnostics.CodeAnalysis; using Newtonsoft.Json; using NUnit.Framework; using StardewModdingAPI.Framework; -using StardewModdingAPI.Toolkit.Serialisation.Models; namespace StardewModdingAPI.Tests.Utilities { @@ -272,22 +271,6 @@ namespace StardewModdingAPI.Tests.Utilities Assert.IsTrue(version.IsOlderThan(new SemanticVersion("1.2.30")), "The game version should be considered older than the later semantic versions."); } - /**** - ** LegacyManifestVersion - ****/ - [Test(Description = "Assert that the LegacyManifestVersion subclass correctly parses legacy manifest versions.")] - [TestCase(1, 0, 0, null, ExpectedResult = "1.0")] - [TestCase(3000, 4000, 5000, null, ExpectedResult = "3000.4000.5000")] - [TestCase(1, 2, 3, "", ExpectedResult = "1.2.3")] - [TestCase(1, 2, 3, " ", ExpectedResult = "1.2.3")] - [TestCase(1, 2, 3, "0", ExpectedResult = "1.2.3")] // special case: drop '0' tag for legacy manifest versions - [TestCase(1, 2, 3, "some-tag.4", ExpectedResult = "1.2.3-some-tag.4")] - [TestCase(1, 2, 3, "some-tag.4 ", ExpectedResult = "1.2.3-some-tag.4")] - public string LegacyManifestVersion(int major, int minor, int patch, string tag) - { - return new LegacyManifestVersion(major, minor, patch, tag).ToString(); - } - /********* ** Private methods diff --git a/src/SMAPI/Framework/Models/ManifestContentPackFor.cs b/src/SMAPI/Framework/Models/ManifestContentPackFor.cs index cdad8893..90e20c6a 100644 --- a/src/SMAPI/Framework/Models/ManifestContentPackFor.cs +++ b/src/SMAPI/Framework/Models/ManifestContentPackFor.cs @@ -21,7 +21,7 @@ namespace StardewModdingAPI.Framework.Models public ManifestContentPackFor(Toolkit.Serialisation.Models.ManifestContentPackFor contentPackFor) { this.UniqueID = contentPackFor.UniqueID; - this.MinimumVersion = new SemanticVersion(contentPackFor.MinimumVersion); + this.MinimumVersion = contentPackFor.MinimumVersion != null ? new SemanticVersion(contentPackFor.MinimumVersion) : null; } /// Construct an instance. diff --git a/src/SMAPI/Framework/Serialisation/SemanticVersionConverter.cs b/src/SMAPI/Framework/Serialisation/SemanticVersionConverter.cs new file mode 100644 index 00000000..3e05a440 --- /dev/null +++ b/src/SMAPI/Framework/Serialisation/SemanticVersionConverter.cs @@ -0,0 +1,40 @@ +using Newtonsoft.Json.Linq; +using StardewModdingAPI.Toolkit.Serialisation; +using StardewModdingAPI.Toolkit.Serialisation.Converters; + +namespace StardewModdingAPI.Framework.Serialisation +{ + /// Handles deserialisation of . + internal class SemanticVersionConverter : SimpleReadOnlyConverter + { + /********* + ** Protected methods + *********/ + /// Read a JSON object. + /// The JSON object to read. + /// The path to the current JSON node. + protected override ISemanticVersion ReadObject(JObject obj, string path) + { + int major = obj.ValueIgnoreCase("MajorVersion"); + int minor = obj.ValueIgnoreCase("MinorVersion"); + int patch = obj.ValueIgnoreCase("PatchVersion"); + string build = obj.ValueIgnoreCase("Build"); + if (build == "0") + build = null; // '0' from incorrect examples in old SMAPI documentation + + return new SemanticVersion(major, minor, patch, build); + } + + /// Read a JSON string. + /// The JSON string value. + /// The path to the current JSON node. + protected override ISemanticVersion ReadString(string str, string path) + { + if (string.IsNullOrWhiteSpace(str)) + return null; + if (!SemanticVersion.TryParse(str, out ISemanticVersion version)) + throw new SParseException($"Can't parse semantic version from invalid value '{str}', should be formatted like 1.2, 1.2.30, or 1.2.30-beta (path: {path})."); + return version; + } + } +} diff --git a/src/SMAPI/Program.cs b/src/SMAPI/Program.cs index 05a3134e..cc59c0cd 100644 --- a/src/SMAPI/Program.cs +++ b/src/SMAPI/Program.cs @@ -159,7 +159,8 @@ namespace StardewModdingAPI new StringEnumConverter(), new ColorConverter(), new PointConverter(), - new RectangleConverter() + new RectangleConverter(), + new Framework.Serialisation.SemanticVersionConverter() }; foreach (JsonConverter converter in converters) this.JsonHelper.JsonSettings.Converters.Add(converter); diff --git a/src/SMAPI/StardewModdingAPI.csproj b/src/SMAPI/StardewModdingAPI.csproj index 3c953ec5..7eb5f95a 100644 --- a/src/SMAPI/StardewModdingAPI.csproj +++ b/src/SMAPI/StardewModdingAPI.csproj @@ -121,6 +121,7 @@ + diff --git a/src/StardewModdingAPI.Toolkit/Serialisation/Converters/SemanticVersionConverter.cs b/src/StardewModdingAPI.Toolkit/Serialisation/Converters/SemanticVersionConverter.cs index 2075d27e..2ddaa1bf 100644 --- a/src/StardewModdingAPI.Toolkit/Serialisation/Converters/SemanticVersionConverter.cs +++ b/src/StardewModdingAPI.Toolkit/Serialisation/Converters/SemanticVersionConverter.cs @@ -18,7 +18,10 @@ namespace StardewModdingAPI.Toolkit.Serialisation.Converters int minor = obj.ValueIgnoreCase("MinorVersion"); int patch = obj.ValueIgnoreCase("PatchVersion"); string build = obj.ValueIgnoreCase("Build"); - return new LegacyManifestVersion(major, minor, patch, build); + if (build == "0") + build = null; // '0' from incorrect examples in old SMAPI documentation + + return new SemanticVersion(major, minor, patch, build); } /// Read a JSON string. diff --git a/src/StardewModdingAPI.Toolkit/Serialisation/Models/LegacyManifestVersion.cs b/src/StardewModdingAPI.Toolkit/Serialisation/Models/LegacyManifestVersion.cs deleted file mode 100644 index 12f6755b..00000000 --- a/src/StardewModdingAPI.Toolkit/Serialisation/Models/LegacyManifestVersion.cs +++ /dev/null @@ -1,26 +0,0 @@ -using Newtonsoft.Json; - -namespace StardewModdingAPI.Toolkit.Serialisation.Models -{ - /// An implementation of that hamdles the legacy version format. - public class LegacyManifestVersion : SemanticVersion - { - /********* - ** Public methods - *********/ - /// Construct an instance. - /// The major version incremented for major API changes. - /// The minor version incremented for backwards-compatible changes. - /// The patch version for backwards-compatible bug fixes. - /// An optional build tag. - [JsonConstructor] - public LegacyManifestVersion(int majorVersion, int minorVersion, int patchVersion, string build = null) - : base( - majorVersion, - minorVersion, - patchVersion, - build != "0" ? build : null // '0' from incorrect examples in old SMAPI documentation - ) - { } - } -} -- cgit From 4b82b111e7392e695b0e021efac2385c1b9850af Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Sun, 10 Jun 2018 21:50:24 -0400 Subject: improve semantic version validation --- docs/release-notes.md | 1 + src/SMAPI.Tests/Utilities/SemanticVersionTests.cs | 14 ++++++++++++ src/StardewModdingAPI.Toolkit/SemanticVersion.cs | 27 ++++++++++++++++++++++- 3 files changed, 41 insertions(+), 1 deletion(-) (limited to 'src/SMAPI.Tests/Utilities') diff --git a/docs/release-notes.md b/docs/release-notes.md index a343cff3..152037f8 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -46,6 +46,7 @@ * Fixed some common non-mod build output being included in release zip. * Fixed mods able to intercept other mods' assets via the internal asset keys. * Fixed mods able to indirectly change other mods' data through shared content caches. + * Fixed `SemanticVersion` allowing invalid versions in some cases. * **Breaking changes** (see [migration guide](https://stardewvalleywiki.com/Modding:Migrate_to_Stardew_Valley_1.3)): * Dropped some deprecated APIs. * `LocationEvents` have been rewritten. diff --git a/src/SMAPI.Tests/Utilities/SemanticVersionTests.cs b/src/SMAPI.Tests/Utilities/SemanticVersionTests.cs index feab452a..9091ef09 100644 --- a/src/SMAPI.Tests/Utilities/SemanticVersionTests.cs +++ b/src/SMAPI.Tests/Utilities/SemanticVersionTests.cs @@ -49,6 +49,19 @@ namespace StardewModdingAPI.Tests.Utilities return version.ToString(); } + [Test(Description = "Assert that the constructor throws the expected exception for invalid versions when constructed from the individual numbers.")] + [TestCase(0, 0, 0, null)] + [TestCase(-1, 0, 0, null)] + [TestCase(0, -1, 0, null)] + [TestCase(0, 0, -1, null)] + [TestCase(1, 0, 0, "-tag")] + [TestCase(1, 0, 0, "tag spaces")] + [TestCase(1, 0, 0, "tag~")] + public void Constructor_FromParts_WithInvalidValues(int major, int minor, int patch, string tag) + { + this.AssertAndLogException(() => new SemanticVersion(major, minor, patch, tag)); + } + [Test(Description = "Assert that the constructor sets the expected values for all valid versions when constructed from an assembly version.")] [TestCase(1, 0, 0, ExpectedResult = "1.0")] [TestCase(1, 2, 3, ExpectedResult = "1.2.3")] @@ -79,6 +92,7 @@ namespace StardewModdingAPI.Tests.Utilities [TestCase("1.2.3.apple")] [TestCase("1..2..3")] [TestCase("1.2.3-")] + [TestCase("1.2.3--some-tag")] [TestCase("1.2.3-some-tag...")] [TestCase("1.2.3-some-tag...4")] [TestCase("apple")] diff --git a/src/StardewModdingAPI.Toolkit/SemanticVersion.cs b/src/StardewModdingAPI.Toolkit/SemanticVersion.cs index bd85f990..3008a4d6 100644 --- a/src/StardewModdingAPI.Toolkit/SemanticVersion.cs +++ b/src/StardewModdingAPI.Toolkit/SemanticVersion.cs @@ -10,8 +10,11 @@ namespace StardewModdingAPI.Toolkit /********* ** Properties *********/ + /// A regex pattern matching a valid prerelease tag. + internal const string TagPattern = @"(?>[a-z0-9]+[\-\.]?)+"; + /// A regex pattern matching a version within a larger string. - internal const string UnboundedVersionPattern = @"(?>(?0|[1-9]\d*))\.(?>(?0|[1-9]\d*))(?>(?:\.(?0|[1-9]\d*))?)(?:-(?(?>[a-z0-9]+[\-\.]?)+))?"; + internal const string UnboundedVersionPattern = @"(?>(?0|[1-9]\d*))\.(?>(?0|[1-9]\d*))(?>(?:\.(?0|[1-9]\d*))?)(?:-(?" + SemanticVersion.TagPattern + "))?"; /// A regular expression matching a semantic version string. /// @@ -54,6 +57,8 @@ namespace StardewModdingAPI.Toolkit this.Minor = minor; this.Patch = patch; this.Tag = this.GetNormalisedTag(tag); + + this.AssertValid(); } /// Construct an instance. @@ -67,6 +72,8 @@ namespace StardewModdingAPI.Toolkit this.Major = version.Major; this.Minor = version.Minor; this.Patch = version.Build; + + this.AssertValid(); } /// Construct an instance. @@ -87,6 +94,8 @@ namespace StardewModdingAPI.Toolkit this.Minor = match.Groups["minor"].Success ? int.Parse(match.Groups["minor"].Value) : 0; this.Patch = match.Groups["patch"].Success ? int.Parse(match.Groups["patch"].Value) : 0; this.Tag = match.Groups["prerelease"].Success ? this.GetNormalisedTag(match.Groups["prerelease"].Value) : null; + + this.AssertValid(); } /// Get an integer indicating whether this version precedes (less than 0), supercedes (more than 0), or is equivalent to (0) the specified version. @@ -235,5 +244,21 @@ namespace StardewModdingAPI.Toolkit // fallback (this should never happen) return string.Compare(this.ToString(), new SemanticVersion(otherMajor, otherMinor, otherPatch, otherTag).ToString(), StringComparison.InvariantCultureIgnoreCase); } + + /// Assert that the current version is valid. + private void AssertValid() + { + if (this.Major < 0 || this.Minor < 0 || this.Patch < 0) + throw new FormatException($"{this} isn't a valid semantic version. The major, minor, and patch numbers can't be negative."); + if (this.Major == 0 && this.Minor == 0 && this.Patch == 0) + throw new FormatException($"{this} isn't a valid semantic version. At least one of the major, minor, and patch numbers must be more than zero."); + if (this.Tag != null) + { + if (this.Tag.Trim() == "") + throw new FormatException($"{this} isn't a valid semantic version. The tag cannot be a blank string (but may be omitted)."); + if (!Regex.IsMatch(this.Tag, $"^{SemanticVersion.TagPattern}$")) + throw new FormatException($"{this} isn't a valid semantic version. The tag is invalid."); + } + } } } -- cgit From c41e1ed5c16d5402fa373267095e28121b4055f2 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Mon, 11 Jun 2018 01:03:27 -0400 Subject: fix new validation not allowing capitals in semver tags --- src/SMAPI.Tests/Utilities/SemanticVersionTests.cs | 2 ++ src/StardewModdingAPI.Toolkit/SemanticVersion.cs | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) (limited to 'src/SMAPI.Tests/Utilities') diff --git a/src/SMAPI.Tests/Utilities/SemanticVersionTests.cs b/src/SMAPI.Tests/Utilities/SemanticVersionTests.cs index 9091ef09..35d74b60 100644 --- a/src/SMAPI.Tests/Utilities/SemanticVersionTests.cs +++ b/src/SMAPI.Tests/Utilities/SemanticVersionTests.cs @@ -22,6 +22,7 @@ namespace StardewModdingAPI.Tests.Utilities [TestCase("3000.4000.5000", ExpectedResult = "3000.4000.5000")] [TestCase("1.2-some-tag.4", ExpectedResult = "1.2-some-tag.4")] [TestCase("1.2.3-some-tag.4", ExpectedResult = "1.2.3-some-tag.4")] + [TestCase("1.2.3-SoME-tAg.4", ExpectedResult = "1.2.3-SoME-tAg.4")] [TestCase("1.2.3-some-tag.4 ", ExpectedResult = "1.2.3-some-tag.4")] public string Constructor_FromString(string input) { @@ -35,6 +36,7 @@ namespace StardewModdingAPI.Tests.Utilities [TestCase(1, 2, 3, " ", ExpectedResult = "1.2.3")] [TestCase(1, 2, 3, "0", ExpectedResult = "1.2.3-0")] [TestCase(1, 2, 3, "some-tag.4", ExpectedResult = "1.2.3-some-tag.4")] + [TestCase(1, 2, 3, "sOMe-TaG.4", ExpectedResult = "1.2.3-sOMe-TaG.4")] [TestCase(1, 2, 3, "some-tag.4 ", ExpectedResult = "1.2.3-some-tag.4")] public string Constructor_FromParts(int major, int minor, int patch, string tag) { diff --git a/src/StardewModdingAPI.Toolkit/SemanticVersion.cs b/src/StardewModdingAPI.Toolkit/SemanticVersion.cs index 3008a4d6..de480416 100644 --- a/src/StardewModdingAPI.Toolkit/SemanticVersion.cs +++ b/src/StardewModdingAPI.Toolkit/SemanticVersion.cs @@ -256,7 +256,7 @@ namespace StardewModdingAPI.Toolkit { if (this.Tag.Trim() == "") throw new FormatException($"{this} isn't a valid semantic version. The tag cannot be a blank string (but may be omitted)."); - if (!Regex.IsMatch(this.Tag, $"^{SemanticVersion.TagPattern}$")) + if (!Regex.IsMatch(this.Tag, $"^{SemanticVersion.TagPattern}$", RegexOptions.IgnoreCase)) throw new FormatException($"{this} isn't a valid semantic version. The tag is invalid."); } } -- cgit