Skip to content

Commit 61fec61

Browse files
committed
refactor: express both Link parsing and the parameter check as the queries they are
CodeQL flagged two loops that filter their sequence implicitly (cs/linq/missed-where, alerts 293 and 294). Both are mine: 294 is this PR's parameter check, and 293 is the Link-header loop on master — #264 cleared cs/linq/missed-select there and CodeQL immediately raised missed-where on the same loop, so the guard-on-the-way-in shape traded one alert for another rather than fixing the cause. Splitting the parse of a single RFC 8288 link-value into its own method leaves ParseLinkHeaderNext reading as what it actually is — the first rel="next" target, if any — and removes the whole class of finding instead of moving it. The parameter check becomes the FirstOrDefault it always was; a default KeyValuePair has a null Key, which is precisely the "no complex parameter" answer. Behaviour is unchanged: Http 60/60, AspNetCore 159/159. The explanatory comment moved from between `=>` and the expression into the XML doc — dotnet format rejects a comment in that position, and it belongs with the rest of the method's documentation anyway.
1 parent 582e355 commit 61fec61

2 files changed

Lines changed: 42 additions & 42 deletions

File tree

src/Integrations/NeoReports.AspNetCore/NeoReportsEndpointRouteBuilderExtensions.cs

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1299,22 +1299,18 @@ private static IResult SchemaProblem(HttpContext http, Exception ex, string sour
12991299
/// they are left alone here.
13001300
/// </para>
13011301
/// </summary>
1302+
/// <para>
1303+
/// <see cref="PrimitiveObjectConverter"/> turns every scalar into a CLR primitive and leaves
1304+
/// exactly the structured values as a <see cref="JsonElement"/>, so the value-kind check below is
1305+
/// the whole test. <c>FirstOrDefault</c> over a <see cref="KeyValuePair{TKey,TValue}"/> yields a
1306+
/// default pair — <c>Key</c> null — when nothing matches, which is the "no complex parameter"
1307+
/// answer.
1308+
/// </para>
13021309
/// <param name="parameters">The normalized parameter bag.</param>
1303-
private static string? FirstComplexParameter(IReadOnlyDictionary<string, object?>? parameters)
1304-
{
1305-
if (parameters is null)
1306-
return null;
1307-
1308-
foreach (KeyValuePair<string, object?> pair in parameters)
1309-
{
1310-
// PrimitiveObjectConverter turns every scalar into a CLR primitive and leaves exactly the
1311-
// structured values as a JsonElement, so this is the whole test.
1312-
if (pair.Value is JsonElement { ValueKind: JsonValueKind.Array or JsonValueKind.Object })
1313-
return pair.Key;
1314-
}
1315-
1316-
return null;
1317-
}
1310+
private static string? FirstComplexParameter(IReadOnlyDictionary<string, object?>? parameters) =>
1311+
parameters?
1312+
.FirstOrDefault(pair => pair.Value is JsonElement { ValueKind: JsonValueKind.Array or JsonValueKind.Object })
1313+
.Key;
13181314

13191315
private static IReadOnlyDictionary<string, object?>? NormalizeJsonValues(
13201316
IReadOnlyDictionary<string, object?>? parameters)

src/Sources/NeoReports.Sources.Http/HttpBatchSource.cs

Lines changed: 31 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -207,35 +207,39 @@ private static IEnumerable<string> SplitLinkValues(string headerValue)
207207
if (!response.Headers.TryGetValues("Link", out IEnumerable<string>? values))
208208
return null;
209209

210-
foreach (string headerValue in values)
210+
// Parsing one link-value is a separate concern from picking the one we want, so it lives in
211+
// its own method and this reads as what it is: the first rel="next" target, if any.
212+
return values
213+
.SelectMany(SplitLinkValues)
214+
.Select(ParseLinkValue)
215+
.FirstOrDefault(parsed => parsed.IsNext)
216+
.Target;
217+
}
218+
219+
/// <summary>
220+
/// Parses one RFC 8288 link-value into its target URI and whether it is the <c>next</c> relation.
221+
/// A value that does not match the grammar yields <c>(null, false)</c> rather than throwing —
222+
/// a <c>Link</c> header may legitimately carry relations this source does not understand.
223+
/// </summary>
224+
private static (string? Target, bool IsNext) ParseLinkValue(string link)
225+
{
226+
string[] parts = link.Split(';');
227+
if (parts.Length < 2)
228+
return (null, false);
229+
230+
string urlPart = parts[0].Trim();
231+
if (urlPart.Length < 2 || urlPart[0] != '<' || urlPart[^1] != '>')
232+
return (null, false);
233+
234+
bool isNext = parts.Skip(1).Any(p =>
211235
{
212-
foreach (string link in SplitLinkValues(headerValue))
213-
{
214-
// Guarded on the way in rather than assigned then checked: a bare `var parts =
215-
// link.Split(...)` as the loop's first statement reads to CodeQL as a map that should
216-
// have been a .Select, which this loop cannot be — it has two guards and an early
217-
// return (alert cs/linq/missed-select, opened by the Link-parsing fix in #262).
218-
if (link.Split(';') is not { Length: >= 2 } parts)
219-
continue;
220-
221-
string urlPart = parts[0].Trim();
222-
if (urlPart.Length < 2 || urlPart[0] != '<' || urlPart[^1] != '>')
223-
continue;
224-
225-
bool isNext = parts.Skip(1).Any(p =>
226-
{
227-
string[] kv = p.Trim().Split('=', 2);
228-
return kv.Length == 2
229-
&& kv[0].Trim().Equals("rel", StringComparison.OrdinalIgnoreCase)
230-
&& kv[1].Trim().Trim('"').Equals("next", StringComparison.OrdinalIgnoreCase);
231-
});
232-
233-
if (isNext)
234-
return urlPart[1..^1];
235-
}
236-
}
236+
string[] kv = p.Trim().Split('=', 2);
237+
return kv.Length == 2
238+
&& kv[0].Trim().Equals("rel", StringComparison.OrdinalIgnoreCase)
239+
&& kv[1].Trim().Trim('"').Equals("next", StringComparison.OrdinalIgnoreCase);
240+
});
237241

238-
return null;
242+
return (urlPart[1..^1], isNext);
239243
}
240244

241245
private BatchResult<T> BuildCursorResult(List<T> records, JsonElement responseRoot, HttpCursorState state)

0 commit comments

Comments
 (0)