Skip to content

Commit 703f149

Browse files
lilithlilith
committed
Matching: [/] for optional trailing slash, disallow /{var:?}/ (//), organize match tests and move junk to MatchOptionalExpressionTests.
1 parent 634a9bc commit 703f149

File tree

14 files changed

+618
-267
lines changed

14 files changed

+618
-267
lines changed

‎src/Imazen.Routing/HttpAbstractions/DictionaryExtensions.cs‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,20 @@ public static Dictionary<string,string> ToStringDictionary(this IReadOnlyQueryWr
1313
}
1414
return d;
1515
}
16+
17+
public static Dictionary<string,StringValues> ToStringValuesDictionary(this IReadOnlyQueryWrapper query)
18+
{
19+
#if NET6_0_OR_GREATER
20+
var d = new Dictionary<string,StringValues>(query);
21+
#else
22+
var d = new Dictionary<string,StringValues>(query.Count);
23+
foreach (var kvp in query)
24+
{
25+
d[kvp.Key] = kvp.Value;
26+
}
27+
#endif
28+
return d;
29+
}
1630
public static Dictionary<string,string> ToStringDictionary(this IDictionary<string,StringValues> query)
1731
{
1832
var d = new Dictionary<string,string>(query.Count);
@@ -34,4 +48,4 @@ public static Dictionary<string, StringValues> ToStringValuesDictionary(
3448
}
3549
return d;
3650
}
37-
}
51+
}

‎src/Imazen.Routing/Matching/ExpressionFlags.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,12 +139,12 @@ public static bool TryParseFromEnd(ReadOnlyMemory<char> expression, out ReadOnly
139139

140140
// Allow k/v pairs as well, [a-zA-Z-][a-zA-Z0-9-]*([=][a-zA-Z0-9-]+)?
141141
#if NET8_0_OR_GREATER
142-
[GeneratedRegex(@"^[a-zA-Z-_][a-zA-Z0-9-]*([=][a-zA-Z0-9-_]+)?$")]
142+
[GeneratedRegex(@"^([a-zA-Z-_][a-zA-Z0-9-\.]*([=][a-zA-Z0-9-_\.]+)?|/)$")]
143143
public static partial Regex LatestPermissiveFlagSyntax();
144144
#else
145145

146146
public static readonly Regex LatestPermissiveFlagSyntaxVar =
147-
new(@"^[a-zA-Z-_][a-zA-Z0-9-]*([=][a-zA-Z0-9-_]+)?$");
147+
new(@"^([a-zA-Z-_][a-zA-Z0-9-\.]*([=][a-zA-Z0-9-_\.]+)?|/)$");
148148

149149
public static Regex LatestPermissiveFlagSyntax() => LatestPermissiveFlagSyntaxVar;
150150
#endif

‎src/Imazen.Routing/Matching/ExpressionParsingOptions.cs‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ public record ExpressionParsingOptions
1212
public bool AllowStarLiteral { get; init; } = false;
1313

1414
public bool AllowQuestionLiteral { get; init; } = false;
15+
16+
public bool AllowOptionalSegmentBetweenSlashes { get; init; } = false;
17+
18+
public bool MatchOptionalTrailingSlash { get; init; } = false;
1519
// /// <summary>
1620
// /// If true, all segments will capture the / character by default. If false, segments must specify {:**} to capture slashes.
1721
// /// </summary>
@@ -31,10 +35,14 @@ internal static ExpressionParsingOptions SubtractFromFlags(List<string> flags, E
3135
{
3236
defaults = defaults with { AllowQuestionLiteral = true };
3337
}
34-
// if (flags.Remove("capture-slashes"))
35-
// {
36-
// defaults = defaults with { CaptureSlashesByDefault = true };
37-
// }
38+
if (flags.Remove("allow-optional-segment-between-slashes"))
39+
{
40+
defaults = defaults with { AllowOptionalSegmentBetweenSlashes = true };
41+
}
42+
if (flags.Remove("/"))
43+
{
44+
defaults = defaults with { MatchOptionalTrailingSlash = true };
45+
}
3846
return defaults;
3947
}
4048
public static ExpressionParsingOptions ParseComplete(ReadOnlyMemory<char> expressionWithFlags, out ReadOnlyMemory<char> remainingExpression)

‎src/Imazen.Routing/Matching/MatchExpression.cs‎

Lines changed: 77 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,17 +27,89 @@ public override string ToString()
2727
{
2828
return string.Join("", Segments);
2929
}
30+
31+
private static bool EndsWithChar(string? s, char c)
32+
{
33+
if (s == null || s.Length == 0) return false;
34+
return s[^1] == c;
35+
}
36+
private static bool StartsWithChar(string? s, char c)
37+
{
38+
if (s == null || s.Length == 0) return false;
39+
return s[0] == c;
40+
}
3041

31-
private static bool TryCreate(IReadOnlyCollection<MatchSegment> segments, [NotNullWhen(true)] out MatchExpression? result, [NotNullWhen(false)]out string? error)
42+
private static bool TryCreate(ExpressionParsingOptions options, IReadOnlyCollection<MatchSegment> segments, [NotNullWhen(true)] out MatchExpression? result, [NotNullWhen(false)]out string? error)
3243
{
33-
if (segments.Count == 0)
44+
45+
var segmentArray = segments.ToArray();
46+
47+
48+
// A shortcut to {:?:eq(/)} is [/]
49+
if (options.MatchOptionalTrailingSlash)
50+
{
51+
var addSlashSegment = true;
52+
if (segmentArray.Length > 0)
53+
{
54+
// Check if the the last segment ends in a forward slash, OR if it is an optional segment that ends in a forward slash
55+
var lastSegment = segmentArray[^1];
56+
if (EndsWithChar(lastSegment.StartsOn.MatchString, '/') || EndsWithChar(lastSegment.EndsOn.MatchString, '/'))
57+
{
58+
addSlashSegment = false;
59+
error = $"Flag [/] would add {{:?:eq(/)}} after {lastSegment}, but that would be confusing.";
60+
result = null;
61+
return false;
62+
}
63+
}
64+
if (addSlashSegment)
65+
{
66+
if (!MatchSegment.TryParseSegmentExpression(options, "{:?:eq(/)}".AsMemory(), new Stack<MatchSegment>(), out _, out var optionalSlashSegment, out error)){
67+
throw new InvalidOperationException($"Failed to parse optional slash segment: {error}");
68+
}
69+
segmentArray = [.. segmentArray, optionalSlashSegment.Value];
70+
}
71+
}
72+
73+
74+
if (segmentArray.Length == 0)
3475
{
3576
result = null;
3677
error = "Zero segments found in expression";
3778
return false;
3879
}
3980

40-
result = new MatchExpression(segments.ToArray());
81+
82+
// Check for bookended optional segments (literals ending with '/' followed by optional segment followed by literals starting with '/')
83+
for (int i = 1; i < segmentArray.Length - 1; i++)
84+
{
85+
var currentSegment = segmentArray[i];
86+
var previousSegment = segmentArray[i - 1];
87+
var nextSegment = segmentArray[i + 1];
88+
89+
// Check if current segment is optional
90+
if (currentSegment.IsOptional)
91+
{
92+
if (!options.AllowOptionalSegmentBetweenSlashes)
93+
{
94+
95+
// Check if previous segment is a literal ending with '/'
96+
var prevLiteral = previousSegment.StartsOn.AsCaseSensitiveLiteral;
97+
if (prevLiteral != null && EndsWithChar(prevLiteral, '/'))
98+
{
99+
// Check if next segment is a literal starting with '/'
100+
var nextLiteral = nextSegment.StartsOn.AsCaseSensitiveLiteral;
101+
if (nextLiteral != null && StartsWithChar(nextLiteral, '/'))
102+
{
103+
result = null;
104+
error = $"Expressions like '/{{var:?}}/' are disallowed; they would match '//'. Found '/{currentSegment}/'. Use {{var:?:suffix(/)}} to make the segment optional - it won't capture there are no more slashes. ";
105+
return false;
106+
}
107+
}
108+
}
109+
}
110+
}
111+
112+
result = new MatchExpression(segmentArray);
41113
error = null;
42114
return true;
43115
}
@@ -201,7 +273,7 @@ private static bool TryParseInternal(ExpressionParsingOptions options, ReadOnlyM
201273
}
202274
segments.Push(parsedSegment.Value);
203275
}
204-
return TryCreate(segments, out result, out error);
276+
return TryCreate(options, segments, out result, out error);
205277
}
206278
public static bool TryParseWithSmartQuery(ParsingOptions parsingDefaults, ReadOnlyMemory<char> expression,
207279
out MatchExpression? pathMatcherResult,
@@ -574,4 +646,4 @@ public IReadOnlyDictionary<string, MatcherVariableInfo> GetMatcherVariableInfo()
574646
}
575647
return new System.Collections.ObjectModel.ReadOnlyDictionary<string, MatcherVariableInfo>(dict);
576648
}
577-
}
649+
}

‎src/Imazen.Routing/Matching/MultiValueMatcher.cs‎

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public static bool TryParse(ReadOnlyMemory<char> expressionWithFlags,
7373
[NotNullWhen(true)] out MultiValueMatcher? result, [NotNullWhen(false)] out string? error)
7474
{
7575
if (!ExpressionFlags.TryParseFromEnd(expressionWithFlags, out var expression, out var flags, out error,
76-
ExpressionFlagParsingOptions.LowercaseDash))
76+
ExpressionFlagParsingOptions.Permissive))
7777
{
7878
result = null;
7979
return false;
@@ -125,17 +125,43 @@ internal MultiMatchResult Match(in MatchingContext context, in string pathAndQue
125125
return Match(context, path.AsMemory(), queryWrapper, headers, ref rawQuery, ref rawPathAndQuery, ref sorted);
126126
}
127127

128+
internal static Tuple<string,string>[] AcceptHeaderQueryPairs = new Tuple<string,string>[]
129+
{
130+
Tuple.Create("image/webp", "accept.webp"),
131+
Tuple.Create("image/avif", "accept.avif"),
132+
Tuple.Create("image/jxl", "accept.jxl"),
133+
};
134+
128135
internal MultiMatchResult Match(in MatchingContext context, in ReadOnlyMemory<char> path,
129136
IReadOnlyQueryWrapper? query, IDictionary<string, StringValues>? headers,
130137
ref ReadOnlyMemory<char>? rawQuery, ref ReadOnlyMemory<char>? rawPathAndQuery, ref ReadOnlyMemory<char>? pathAndSortedQuery)
131138
{
132-
if (ParsingOptions.RequireAcceptWebP &&
133-
(headers == null
134-
|| !headers.TryGetValue(HttpHeaderNames.Accept, out var accept)
135-
|| !accept.Any(s => s != null && s.Contains("image/webp"))))
139+
// TODO: Importing the accept header might happen at the CDN
140+
// And needs to modify the cache. And if we use the CDN, we don't want to
141+
// populate the cache based on a single user's header... or another cache layer.
142+
// So, we may want to work with this a little more.
143+
Dictionary<string, StringValues>? mutableQuery = null;
144+
if (ParsingOptions.ImportAcceptHeader &&
145+
headers != null && query != null
146+
&& headers.TryGetValue(HttpHeaderNames.Accept, out var accept))
147+
{
148+
foreach (var pair in AcceptHeaderQueryPairs)
149+
{
150+
if (accept.Any(s => s != null && s.Contains(pair.Item1)))
151+
{
152+
if (!query.TryGetValue(pair.Item2, out string? acceptWebp)
153+
|| acceptWebp != "1")
154+
{
155+
mutableQuery ??= query.ToStringValuesDictionary();
156+
mutableQuery.Add(pair.Item2, "1");
157+
}
158+
// We don't currently override existing values
159+
}
160+
}
161+
}
162+
if (mutableQuery != null)
136163
{
137-
return new MultiMatchResult()
138-
{ Success = false, Error = "'image/webp' was not found in the HTTP Accept header string" };
164+
query = new DictionaryQueryWrapper(mutableQuery);
139165
}
140166

141167
var pathInput = ParsingOptions.IgnorePath ? "".AsMemory() : path;

‎src/Imazen.Routing/Matching/ParsingOptions.cs‎

Lines changed: 5 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -16,21 +16,7 @@ public record ParsingOptions
1616
/// </summary>
1717
public bool IgnorePath { get; init; } = false;
1818

19-
/// <summary>
20-
/// If true, matching will only succeed if the Accept HTTP header is present and contains 'image/webp'
21-
/// </summary>
22-
public bool RequireAcceptWebP { get; init; }
23-
24-
/// <summary>
25-
/// If true, matching will only succeed if the Accept HTTP header is present and contains 'image/avif'
26-
/// </summary>
27-
public bool RequireAcceptAvif { get; init; }
28-
29-
/// <summary>
30-
/// If true, matching will only succeed if the Accept HTTP header is present and contains 'image/jxl'
31-
/// </summary>
32-
public bool RequireAcceptJxl { get; init; }
33-
19+
3420
/// <summary>
3521
/// * `import-accept-header` Searches the accept header for image/webp, image/avif, and image/jxl and translates them to &accept.webp=1, &accept.avif=1, &accept.jxl=1
3622
/// </summary>
@@ -60,7 +46,7 @@ public record ParsingOptions
6046
public static ParsingOptions SubtractFromFlags(List<string> flags)
6147
{
6248
var context = new ParsingOptions();
63-
if (flags.Remove("ignore-case"))
49+
if (flags.Remove("ignore-case") || flags.Remove("i"))
6450
{
6551
context = context with { QueryParsingOptions = QueryParsingOptions.DefaultCaseInsensitive, PathParsingOptions = PathParsingOptions.DefaultCaseInsensitive };
6652
}
@@ -72,32 +58,17 @@ public static ParsingOptions SubtractFromFlags(List<string> flags)
7258
{
7359
context = context with { RawQueryAndPath = true };
7460
}
75-
if (flags.Remove("sort-raw-query-first"))
61+
if (flags.Remove("sort-raw"))
7662
{
77-
context = context with { SortRawQueryValuesFirst = true };
63+
context = context with { SortRawQueryValuesFirst = true, RawQueryAndPath = true };
7864
}
7965

8066
if (flags.Remove("ignore-path"))
8167
{
8268
context = context with { IgnorePath = true };
8369
}
8470

85-
if (flags.Remove("require-accept-webp"))
86-
{
87-
context = context with { RequireAcceptWebP = true };
88-
}
89-
90-
if (flags.Remove("require-accept-avif"))
91-
{
92-
context = context with { RequireAcceptAvif = true };
93-
}
94-
95-
if (flags.Remove("require-accept-jxl"))
96-
{
97-
context = context with { RequireAcceptJxl = true };
98-
}
99-
100-
if (flags.Remove("import-accept-header"))
71+
if (flags.Contains("accept.format"))
10172
{
10273
context = context with { ImportAcceptHeader = true };
10374
}

‎src/Imazen.Routing/Matching/PathParsingOptions.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ internal ExpressionParsingOptions ToExpressionParsingOptions()
1010
OrdinalIgnoreCase = OrdinalIgnoreCase,
1111
AllowStarLiteral = AllowStarLiteral,
1212
AllowQuestionLiteral = false,
13+
MatchOptionalTrailingSlash = MatchOptionalTrailingSlash,
1314
//CaptureSlashesByDefault = CaptureSlashesByDefault
1415
};
1516
}
@@ -19,6 +20,7 @@ internal ExpressionParsingOptions ToExpressionParsingOptions()
1920
public bool OrdinalIgnoreCase { get; init; } = false;
2021

2122
public bool AllowStarLiteral { get; init; } = false;
23+
public bool MatchOptionalTrailingSlash { get; init; } = false;
2224
/// <summary>
2325
/// If true, all segments will capture the / character by default. If false, segments must specify {:**} to capture slashes.
2426
/// </summary>
@@ -34,6 +36,10 @@ public static PathParsingOptions SubtractFromFlags(List<string> flags, PathParsi
3436
{
3537
defaults = defaults with { AllowStarLiteral = true };
3638
}
39+
if (flags.Remove("/"))
40+
{
41+
defaults = defaults with { MatchOptionalTrailingSlash = true };
42+
}
3743
return defaults;
3844
}
3945

‎src/Imazen.Routing/Matching/SegmentBoundary.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,7 @@ public bool TryScan(ReadOnlySpan<char> text, out int start, out int end)
381381

382382

383383
}
384-
private string? MatchString => On switch
384+
public string? MatchString => On switch
385385
{
386386
When.AtChar or When.EqualsChar => Char.ToString(),
387387
When.AtString or When.AtStringIgnoreCase or

‎src/Imazen.Routing/Matching/Templating/StringTemplate.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ private static bool TryParseVariableContent(
222222
bool isHandled = transformations.Any(t => t is OrTransform || t is DefaultTransform || t is OptionalMarkerTransform);
223223
if (!isHandled)
224224
{
225-
error = $"Template uses optional variable '{variableName}' without providing a fallback (:or-var, :default) or marking as ignorable (:optional, :?).";
225+
error = $"Template uses optional variable '{variableName}' without providing a fallback such as (:or-var(name), :default(value)) or marking as ignorable (:optional, :?).";
226226
segment = null;
227227
return false;
228228
}

0 commit comments

Comments
 (0)