Skip to content

Commit fd7c25b

Browse files
hyperblasthyperblast
committed
improve argument validation, introduce PlaylistRefConverter, fixes
1 parent eb4cdc5 commit fd7c25b

File tree

6 files changed

+83
-17
lines changed

6 files changed

+83
-17
lines changed

‎src/Client/Infrastructure/ArgumentValidator.cs‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
using System;
2+
using System.Runtime.CompilerServices;
3+
using System.Text.RegularExpressions;
4+
5+
namespace Beefweb.Client.Infrastructure;
6+
7+
internal static partial class ArgumentValidator
8+
{
9+
[GeneratedRegex("^[a-z0-9_]+$")]
10+
private static partial Regex IdMatcher();
11+
12+
public static void ValidatePlaylistId(string? id,
13+
[CallerArgumentExpression(nameof(id))] string parameterName = "")
14+
{
15+
if (id == null || !IdMatcher().IsMatch(id))
16+
throw new ArgumentException($"Invalid playlist identifier '{id ?? "null"}'.", parameterName);
17+
}
18+
19+
public static void ValidateConfigId(string id, [CallerArgumentExpression(nameof(id))] string parameterName = "")
20+
{
21+
if (!IdMatcher().IsMatch(id))
22+
throw new ArgumentException($"Invalid configuration identifier '{id}'.", parameterName);
23+
}
24+
}

‎src/Client/Infrastructure/PlaylistRefConverter.cs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
using System;
2+
using System.Text.Json;
3+
using System.Text.Json.Serialization;
4+
5+
namespace Beefweb.Client.Infrastructure;
6+
7+
internal sealed class PlaylistRefConverter : JsonConverter<PlaylistRef>
8+
{
9+
public override PlaylistRef Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
10+
{
11+
return reader.TokenType switch
12+
{
13+
JsonTokenType.Number => reader.GetInt32(),
14+
JsonTokenType.String => reader.GetString()!,
15+
_ => throw new JsonException()
16+
};
17+
}
18+
19+
public override void Write(Utf8JsonWriter writer, PlaylistRef value, JsonSerializerOptions options)
20+
{
21+
if (value.Id != null)
22+
writer.WriteStringValue(value.Id);
23+
else
24+
writer.WriteNumberValue(value.Index);
25+
}
26+
}

‎src/Client/Infrastructure/RequestHandler.cs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ private static JsonSerializerOptions CreateSerializerOptions()
4646
new UnixTimestampConverter(),
4747
new TimeSpanAsSecondsConverter(),
4848
new FileSystemEntryTypeConverter(),
49-
new JsonStringEnumConverter(namingPolicy)
49+
new PlaylistRefConverter(),
50+
new JsonStringEnumConverter(namingPolicy),
5051
},
5152
};
5253
}
@@ -160,10 +161,15 @@ public async IAsyncEnumerable<object> GetEvents(
160161
HttpContent GetContent()
161162
{
162163
if (body is RawJson rawJson)
163-
return new StringContent(rawJson.Value, Utf8, new MediaTypeHeaderValue(ContentTypes.Json, "utf-8"));
164+
{
165+
return new StringContent(rawJson.Value ?? "null", Utf8,
166+
new MediaTypeHeaderValue(ContentTypes.Json, "utf-8"));
167+
}
164168

165169
if (body != null)
170+
{
166171
return JsonContent.Create(body, body.GetType(), options: serializerOptions ?? DefaultSerializerOptions);
172+
}
167173

168174
return new ByteArrayContent([]);
169175
}

‎src/Client/PlayerClient.cs‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
using System.Threading;
77
using System.Threading.Tasks;
88
using Beefweb.Client.Infrastructure;
9+
using static System.FormattableString;
10+
using static Beefweb.Client.Infrastructure.ArgumentValidator;
911

1012
namespace Beefweb.Client;
1113

@@ -109,7 +111,7 @@ public async ValueTask PlayPreviousBy(string expression, CancellationToken cance
109111
public async ValueTask Play(PlaylistRef playlist, int itemIndex, CancellationToken cancellationToken = default)
110112
{
111113
await _handler
112-
.Post(FormattableString.Invariant($"api/player/play/{playlist}/{itemIndex}"), null, cancellationToken)
114+
.Post(Invariant($"api/player/play/{playlist}/{itemIndex}"), null, cancellationToken)
113115
.ConfigureAwait(false);
114116
}
115117

@@ -203,8 +205,8 @@ public async ValueTask AddToPlayQueue(PlaylistRef playlist, int itemIndex, int?
203205
CancellationToken cancellationToken = default)
204206
{
205207
var body = queueIndex != null
206-
? new { plref = playlist.GetValue(), itemIndex, queueIndex }
207-
: (object) new { plref = playlist.GetValue(), itemIndex };
208+
? new { plref = playlist, itemIndex, queueIndex }
209+
: (object) new { plref = playlist, itemIndex };
208210

209211
await _handler.Post("api/playqueue/add", body, cancellationToken).ConfigureAwait(false);
210212
}
@@ -220,7 +222,7 @@ public async ValueTask RemoveFromPlayQueue(
220222
PlaylistRef playlist, int itemIndex, CancellationToken cancellationToken = default)
221223
{
222224
await _handler
223-
.Post("api/playqueue/remove", new { plref = playlist.GetValue(), itemIndex }, cancellationToken)
225+
.Post("api/playqueue/remove", new { plref = playlist, itemIndex }, cancellationToken)
224226
.ConfigureAwait(false);
225227
}
226228

@@ -270,7 +272,7 @@ public async ValueTask<PlaylistItemsResult> GetPlaylistItems(
270272
public async ValueTask SetCurrentPlaylist(PlaylistRef playlist, CancellationToken cancellationToken = default)
271273
{
272274
await _handler
273-
.Post("api/playlists", new { current = playlist.GetValue() }, cancellationToken)
275+
.Post("api/playlists", new { current = playlist }, cancellationToken)
274276
.ConfigureAwait(false);
275277
}
276278

@@ -289,7 +291,7 @@ public async ValueTask MovePlaylist(
289291
PlaylistRef playlist, int newPosition, CancellationToken cancellationToken = default)
290292
{
291293
await _handler
292-
.Post(FormattableString.Invariant($"api/playlists/move/{playlist}/{newPosition}"), null, cancellationToken)
294+
.Post(Invariant($"api/playlists/move/{playlist}/{newPosition}"), null, cancellationToken)
293295
.ConfigureAwait(false);
294296
}
295297

@@ -422,23 +424,23 @@ public async ValueTask<FileSystemEntriesResult> GetFileSystemEntries(string path
422424
PlaylistRef playlist, int itemIndex, CancellationToken cancellationToken = default)
423425
{
424426
return await _handler
425-
.GetStream(FormattableString.Invariant($"api/artwork/{playlist}/{itemIndex}"), null, cancellationToken)
427+
.GetStream(Invariant($"api/artwork/{playlist}/{itemIndex}"), null, cancellationToken)
426428
.ConfigureAwait(false);
427429
}
428430

429431
/// <inheritdoc />
430432
public async ValueTask<string?> GetClientConfig(string id, CancellationToken cancellationToken = default)
431433
{
432-
var result = (RawJson?)await GetClientConfig(id, typeof(RawJson), cancellationToken: cancellationToken)
433-
.ConfigureAwait(false);
434-
434+
var result = await this.GetClientConfig<RawJson?>(id, cancellationToken).ConfigureAwait(false);
435435
return result?.Value == "null" ? null : result?.Value;
436436
}
437437

438438
/// <inheritdoc />
439439
public async ValueTask<object?> GetClientConfig(
440440
string id, Type configType, CancellationToken cancellationToken = default)
441441
{
442+
ValidateConfigId(id);
443+
442444
return await _handler
443445
.Get(configType,
444446
$"api/clientconfig/{id}",
@@ -451,12 +453,16 @@ public async ValueTask<FileSystemEntriesResult> GetFileSystemEntries(string path
451453
/// <inheritdoc />
452454
public async ValueTask SetClientConfig(string id, string value, CancellationToken cancellationToken = default)
453455
{
456+
ValidateConfigId(id);
457+
454458
await SetClientConfig(id, new RawJson(value), cancellationToken).ConfigureAwait(false);
455459
}
456460

457461
/// <inheritdoc />
458462
public async ValueTask SetClientConfig(string id, object value, CancellationToken cancellationToken = default)
459463
{
464+
ValidateConfigId(id);
465+
460466
await _handler
461467
.Post(
462468
null,

‎src/Client/PlaylistRef.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.Globalization;
3+
using Beefweb.Client.Infrastructure;
34

45
namespace Beefweb.Client;
56

@@ -34,9 +35,7 @@ namespace Beefweb.Client;
3435
/// <exception cref="ArgumentException"><paramref name="id"/> is null or contains only whitespace characters.</exception>
3536
public PlaylistRef(string id)
3637
{
37-
if (string.IsNullOrWhiteSpace(id))
38-
throw new ArgumentException("Invalid playlist identifier.", nameof(id));
39-
38+
ArgumentValidator.ValidatePlaylistId(id);
4039
Id = id;
4140
Index = -1;
4241
}

‎src/CommandLineTool/Services/Extensions.cs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,20 @@ public static async ValueTask<PlaylistInfo> GetPlaylist(
2525
return playlists.Get(playlistRef, zeroBasedIndexing);
2626
}
2727

28-
public static PlaylistInfo Get(this IList<PlaylistInfo> playlists, string playlistRef, bool zeroBasedIndexes)
28+
public static PlaylistInfo Get(this IList<PlaylistInfo> playlists, string playlistRef, bool zeroBasedIndices)
2929
{
3030
if (string.Equals(playlistRef, Constants.CurrentPlaylist, StringComparison.OrdinalIgnoreCase))
3131
{
3232
return playlists.FirstOrDefault(p => p.IsCurrent) ?? playlists.First();
3333
}
3434

35-
if (!IndexParser.TryParse(playlistRef, zeroBasedIndexes, out var index))
35+
if (!IndexParser.TryParse(playlistRef, zeroBasedIndices, out var index))
3636
{
37+
if (!zeroBasedIndices && playlistRef == "0")
38+
{
39+
throw new InvalidRequestException("Playlist index (0) is out of range.");
40+
}
41+
3742
return playlists.FirstOrDefault(p => p.Id == playlistRef) ??
3843
throw new InvalidRequestException($"Unable to find playlist with id '{playlistRef}'.");
3944
}

0 commit comments

Comments
 (0)