From 5d68f3e5637d77553eb4b4650ce51df1888a35aa Mon Sep 17 00:00:00 2001
From: Rutherther <rutherther@ditigal.xyz>
Date: Tue, 17 Jun 2025 20:49:38 +0200
Subject: [PATCH 1/2] test: Add tests for invalid cases for TimeSpanParser

---
 .../Parsers/TimeSpanParserTests.cs            | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/Tests/Remora.Commands.Tests/Parsers/TimeSpanParserTests.cs b/Tests/Remora.Commands.Tests/Parsers/TimeSpanParserTests.cs
index 86f4239..868b419 100644
--- a/Tests/Remora.Commands.Tests/Parsers/TimeSpanParserTests.cs
+++ b/Tests/Remora.Commands.Tests/Parsers/TimeSpanParserTests.cs
@@ -24,6 +24,7 @@
 using System.Collections.Generic;
 using System.Threading.Tasks;
 using Remora.Commands.Parsers;
+using Remora.Commands.Results;
 using Xunit;
 
 namespace Remora.Commands.Tests.Parsers;
@@ -86,9 +87,25 @@ public class TimeSpanParserTests
 
                 return then - now + TimeSpan.FromMinutes(1);
             })
+        ],
+        [
+            "-1m",
+            new Func<TimeSpan>(() => -TimeSpan.FromMinutes(1))
         ]
     ];
 
+    /// <summary>
+    /// Gets a set of various test cases that aren't parseable.
+    /// </summary>
+    public static IEnumerable<object?[]> InvalidCases =>
+    [
+        ["x60sx"],
+        ["1m-60s"],
+        ["s"],
+        [string.Empty],
+        [null],
+    ];
+
     /// <summary>
     /// Tests whether the parser can successfully parse various inputs.
     /// </summary>
@@ -106,4 +123,21 @@ public async Task CanParse(string value, Func<TimeSpan> expected)
         Assert.True(result.IsSuccess);
         Assert.Equal(expected(), result.Entity);
     }
+
+    /// <summary>
+    /// Tests whether the parser outputs errors in case of malformed inputs.
+    /// </summary>
+    /// <param name="value">The value to parse.</param>
+    /// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
+    [Theory]
+    [MemberData(nameof(InvalidCases))]
+    public async Task DoesntAcceptInvalidInput(string? value)
+    {
+        var parser = new TimeSpanParser();
+
+        var result = await parser.TryParseAsync(value);
+
+        Assert.False(result.IsSuccess);
+        Assert.IsType<ParsingError<TimeSpan>>(result.Error);
+    }
 }

From 25b740d723a0e8bbdaf05a50f3505cbf2604bae9 Mon Sep 17 00:00:00 2001
From: Rutherther <rutherther@ditigal.xyz>
Date: Tue, 17 Jun 2025 20:49:59 +0200
Subject: [PATCH 2/2] fix: strings with excess characters are dropped by
 TimeSpanParser

TimeSpanParser currently ignores excess characters in front of or
after the matches. This means unexpected input like x60sx will lead to
a match of 60 s. The same, worse so, goes for - in front of the
TimeSpan.

This commit checks for excess characters, and returns a parsing error.
In addition it implements support for negative time spans. Negative
time spans are already supported in cases where TimeSpan.TryParse
parses the TimeSpan, but not with the custom format.
---
 .../Parsers/Builtin/TimeSpanParser.cs         | 20 +++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/Remora.Commands/Parsers/Builtin/TimeSpanParser.cs b/Remora.Commands/Parsers/Builtin/TimeSpanParser.cs
index e827a67..3ea686a 100644
--- a/Remora.Commands/Parsers/Builtin/TimeSpanParser.cs
+++ b/Remora.Commands/Parsers/Builtin/TimeSpanParser.cs
@@ -40,7 +40,7 @@ public class TimeSpanParser : AbstractTypeParser<TimeSpan>
 {
     private static readonly Regex _pattern = new
     (
-        "(?<Years>\\d+(?=y))|(?<Months>\\d+(?=mo))|(?<Weeks>\\d+(?=w))|(?<Days>\\d+(?=d))|(?<Hours>\\d+(?=h))|(?<Minutes>\\d+(?=m))|(?<Seconds>\\d+(?=s))",
+        "^-|(?<Years>\\d+y)|(?<Months>\\d+mo)|(?<Weeks>\\d+w)|(?<Days>\\d+d)|(?<Hours>\\d+h)|(?<Minutes>\\d+m)|(?<Seconds>\\d+s)",
         RegexOptions.Compiled
     );
 
@@ -65,6 +65,11 @@ public override ValueTask<Result<TimeSpan>> TryParseAsync(string? value, Cancell
             return new ValueTask<Result<TimeSpan>>(new ParsingError<TimeSpan>(value));
         }
 
+        if (value.Length != matches.Sum(x => x.Length))
+        {
+            return new ValueTask<Result<TimeSpan>>(new ParsingError<TimeSpan>(value));
+        }
+
         var timeSpan = TimeSpan.Zero;
         foreach (var match in matches.Cast<Match>())
         {
@@ -74,8 +79,14 @@ public override ValueTask<Result<TimeSpan>> TryParseAsync(string? value, Cancell
                 .Skip(1)
                 .Select(g => (g.Name, g.Value));
 
-            foreach (var (key, groupValue) in groups)
+            foreach (var (key, groupRawValue) in groups)
             {
+                var groupValue = groupRawValue[..^1];
+                if (key is "Months")
+                {
+                    groupValue = groupValue[..^1];
+                }
+
                 if (!double.TryParse(groupValue, out var parsedGroupValue))
                 {
                     return new ValueTask<Result<TimeSpan>>(new ParsingError<TimeSpan>(value));
@@ -123,6 +134,11 @@ public override ValueTask<Result<TimeSpan>> TryParseAsync(string? value, Cancell
             }
         }
 
+        if (value[0] == '-')
+        {
+            timeSpan = -timeSpan;
+        }
+
         return new ValueTask<Result<TimeSpan>>(timeSpan);
     }
 }