fix: tighten parsing of label formats

Author: robertcoltheart

src/GitVersion.Core.Tests/Formatting/LabelTokenizerTests.cs

@@ -39,6 +39,8 @@ public class LabelTokenizerTests
     [TestCase("Prop ?? literal ?? ? fallback")]
     [TestCase("Prop ? fallback")]
     [TestCase("Prop ?? fall?back")]
+    [TestCase("Prop ?? fallback ??")]
+    [TestCase("Prop ?? fallback ??   ")]
     public void ParseTokens_MalformedIdentifiers_Throws(string input) => AssertThrows(input);
 
     private static void AssertTokens(string input, string[] expected)

src/GitVersion.Core/Extensions/ConfigurationExtensions.cs

@@ -154,7 +154,10 @@ private static Dictionary<string, object> BuildLabelPlaceholders(string? regular
             if (!match.Success)
                 return placeholders;
 
-            foreach (var groupName in regex.GetGroupNames().Skip(1))
+            var namedGroups = regex.GetGroupNames()
+                .Where(name => !int.TryParse(name, out _));
+
+            foreach (var groupName in namedGroups)
             {
                 var groupValue = match.Groups[groupName].Value;
 

src/GitVersion.Core/Formatting/LabelTokenizer.cs

@@ -9,10 +9,11 @@ internal class LabelTokenizer(string input)
     public IEnumerable<LabelToken> ParseTokens()
     {
         var tokens = new List<LabelToken>();
+        var separatedParsed = false;
 
         SkipWhitespace();
 
-        while (index < input.Length)
+        while (this.index < input.Length)
         {
             var identifier = ParseIdentifier();
 
@@ -25,10 +26,16 @@ public IEnumerable<LabelToken> ParseTokens()
 
             tokens.Add(ParseToken(identifier));
 
-            ParseSeparator();
+            separatedParsed = ParseSeparator();
+
             SkipWhitespace();
         }
 
+        if (separatedParsed)
+        {
+            throw new FormatException("Invalid format sequence, expected identifier after '??' separator");
+        }
+
         return tokens;
     }
 
@@ -37,9 +44,9 @@ private static LabelToken ParseToken(string identifier)
         if (identifier.StartsWith("env:", StringComparison.OrdinalIgnoreCase))
         {
             var name = identifier[4..];
-            var (environmentName, environemntFormat) = ParseKeyAndFormat(name);
+            var (environmentName, environmentFormat) = ParseKeyAndFormat(name);
 
-            return new LabelToken(environmentName, LabelTokenType.Environment, environemntFormat);
+            return new LabelToken(environmentName, LabelTokenType.Environment, environmentFormat);
         }
 
         if (identifier.StartsWith('"') && identifier.EndsWith('"'))
@@ -79,9 +86,9 @@ private string ParseIdentifier()
             value.Append('"');
         }
 
-        while (index < input.Length)
+        while (this.index < input.Length)
         {
-            var c = input[index];
+            var c = input[this.index];
 
             if (!inQuotes && IsQuote())
             {
@@ -103,12 +110,12 @@ private string ParseIdentifier()
             if (IsEscapeQuote())
             {
                 value.Append('"');
-                index += 2;
+                this.index += 2;
             }
             else
             {
                 value.Append(c);
-                index++;
+                this.index++;
             }
         }
 
@@ -120,48 +127,50 @@ private string ParseIdentifier()
         return value.ToString();
     }
 
-    private void ParseSeparator()
+    private bool ParseSeparator()
     {
         var seen = 0;
 
         while (this.index < input.Length && seen < 2)
         {
-            if (input[index] != '?')
+            if (input[this.index] != '?')
             {
                 throw new FormatException("Expected '??' separator");
             }
 
             seen++;
-            index++;
+            this.index++;
         }
+
+        return seen == 2;
     }
 
     private void SkipWhitespace()
     {
-        while (index < input.Length)
+        while (this.index < input.Length)
         {
-            if (!char.IsWhiteSpace(input[index]))
+            if (!char.IsWhiteSpace(input[this.index]))
             {
                 break;
             }
 
-            index++;
+            this.index++;
         }
     }
 
     private bool ParseQuote()
     {
         if (IsQuote())
         {
-            index++;
+            this.index++;
 
             return true;
         }
 
         return false;
     }
 
-    private bool IsQuote() => index < input.Length && input[index] == '"';
+    private bool IsQuote() => this.index < input.Length && input[this.index] == '"';
 
-    private bool IsEscapeQuote() => this.index + 1 < input.Length && input[index] == '\\' && input[this.index + 1] == '"';
+    private bool IsEscapeQuote() => this.index + 1 < input.Length && input[this.index] == '\\' && input[this.index + 1] == '"';
 }

src/GitVersion.Core/Formatting/StringFormatWithExtension.cs

@@ -95,10 +95,10 @@ private static string EvaluateMatch(string input, Func<string, string?> memberEv
             }
 
             var value = token.Type == LabelTokenType.Environment
-                ? environment.GetEnvironmentVariable(token.Name)
+                ? environment.GetEnvironmentVariable(InputSanitizer.SanitizeEnvVarName(token.Name))
                 : memberEvaluator(token.Name);
 
-            if (!string.IsNullOrEmpty(value) && !string.IsNullOrEmpty(token.Format))
+            if (value is not null && !string.IsNullOrEmpty(token.Format))
             {
                 if (ValueFormatter.Default.TryFormat(value, InputSanitizer.SanitizeFormat(token.Format), out var formatted))
                 {
@@ -108,7 +108,7 @@ private static string EvaluateMatch(string input, Func<string, string?> memberEv
                 return value;
             }
 
-            if (!string.IsNullOrEmpty(value))
+            if (value is not null)
             {
                 return value;
             }

src/GitVersion.Core/VersionCalculation/VariableProvider.cs

@@ -84,7 +84,7 @@ public GitVersionVariables GetVariablesFor(
                 formattedString = formatString.FormatWith(source, this.environment)
                     .RegexReplace(RegexPatterns.Output.SanitizeAssemblyInfoRegexPattern, "-");
             }
-            catch (ArgumentException exception)
+            catch (Exception exception) when (exception is ArgumentException or FormatException)
             {
                 throw new WarningException($"Unable to format {formatVarName}.  Check your format string: {exception.Message}");
             }