Skip to content

Commit 0533ee7

Browse files
committed
Add user-facing validation for DB-enforced length limits of string properties
Closes #37. Similarly to #36, this won't make the submission not-fail, but errors won't get reported to sentry and users will get better feedback about why their map won't upload. This also moves the romanised validations from #24 to the built-in data annotations machinery as it's honestly just easier to not reinvent the wheel, and the declarativeness of the syntax is also nice.
1 parent 0a661db commit 0533ee7

File tree

5 files changed

+63
-10
lines changed

5 files changed

+63
-10
lines changed
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
2+
// See the LICENCE file in the repository root for full licence text.
3+
4+
using System.ComponentModel.DataAnnotations;
5+
using osu.Game.Beatmaps;
6+
7+
namespace osu.Server.BeatmapSubmission.Models.Database.Validation
8+
{
9+
public class RomanisedAttribute : ValidationAttribute
10+
{
11+
public override bool IsValid(object? value)
12+
=> value is string s && MetadataUtils.IsRomanised(s);
13+
}
14+
}

osu.Server.BeatmapSubmission/Models/Database/beatmapset_version_file.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,16 @@
33

44
// ReSharper disable InconsistentNaming
55

6+
using System.ComponentModel.DataAnnotations;
7+
68
namespace osu.Server.BeatmapSubmission.Models.Database
79
{
810
public class beatmapset_version_file
911
{
1012
public ulong file_id { get; set; }
1113
public ulong version_id { get; set; }
14+
15+
[MaxLength(500, ErrorMessage = "Filenames cannot exceed 500 characters.")]
1216
public string filename { get; set; } = string.Empty;
1317
}
1418
}

osu.Server.BeatmapSubmission/Models/Database/osu_beatmap.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33

44
// ReSharper disable InconsistentNaming
55

6+
using System.ComponentModel.DataAnnotations;
67
using osu.Game.Beatmaps;
8+
using osu.Server.BeatmapSubmission.Models.Database.Validation;
79

810
namespace osu.Server.BeatmapSubmission.Models.Database
911
{
@@ -12,9 +14,16 @@ public class osu_beatmap
1214
public uint beatmap_id { get; set; }
1315
public uint? beatmapset_id { get; set; }
1416
public uint user_id { get; set; }
17+
18+
[MaxLength(150, ErrorMessage = "Beatmap difficulty filenames must not exceed 150 characters.")]
1519
public string? filename { get; set; }
20+
1621
public string? checksum { get; set; }
22+
23+
[MaxLength(80, ErrorMessage = "Beatmap difficulty names must not exceed 80 characters.")]
24+
[Romanised(ErrorMessage = "Difficulty name contains disallowed characters.")]
1725
public string version { get; set; } = string.Empty;
26+
1827
public uint total_length { get; set; }
1928
public uint hit_length { get; set; }
2029
public uint countTotal { get; set; }

osu.Server.BeatmapSubmission/Models/Database/osu_beatmapset.cs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
// ReSharper disable InconsistentNaming
55
// ReSharper disable CollectionNeverQueried
66

7+
using System.ComponentModel.DataAnnotations;
78
using osu.Game.Beatmaps;
9+
using osu.Server.BeatmapSubmission.Models.Database.Validation;
810

911
namespace osu.Server.BeatmapSubmission.Models.Database
1012
{
@@ -13,13 +15,30 @@ public class osu_beatmapset
1315
public uint beatmapset_id { get; set; }
1416
public uint user_id { get; set; }
1517
public uint thread_id { get; set; }
18+
19+
[MaxLength(80, ErrorMessage = "Romanised artist cannot exceed 80 characters.")]
20+
[Romanised(ErrorMessage = "Romanised artist contains disallowed characters.")]
1621
public string artist { get; set; } = string.Empty;
22+
23+
[MaxLength(80, ErrorMessage = "Artist cannot exceed 80 characters.")]
1724
public string? artist_unicode { get; set; }
25+
26+
[MaxLength(80, ErrorMessage = "Romanised title cannot exceed 80 characters.")]
27+
[Romanised(ErrorMessage = "Romanised title contains disallowed characters.")]
1828
public string title { get; set; } = string.Empty;
29+
30+
[MaxLength(80, ErrorMessage = "Title cannot exceed 80 characters.")]
1931
public string? title_unicode { get; set; }
32+
33+
[MaxLength(80, ErrorMessage = "Author username cannot exceed 80 characters.")]
2034
public string creator { get; set; } = string.Empty;
35+
36+
[MaxLength(200, ErrorMessage = "Source cannot exceed 200 characters.")]
2137
public string source { get; set; } = string.Empty;
38+
39+
[MaxLength(1000, ErrorMessage = "Tags cannot exceed 1000 characters.")]
2240
public string tags { get; set; } = string.Empty;
41+
2342
public bool video { get; set; }
2443
public bool storyboard { get; set; }
2544
public bool epilepsy { get; set; }

osu.Server.BeatmapSubmission/Services/BeatmapPackageParser.cs

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
22
// See the LICENCE file in the repository root for full licence text.
33

4+
using System.ComponentModel.DataAnnotations;
45
using System.Security.Cryptography;
56
using osu.Framework.Extensions;
67
using osu.Game.Beatmaps;
@@ -57,12 +58,9 @@ public BeatmapPackageParseResult Parse(uint beatmapSetId, ArchiveReader archiveR
5758

5859
if (beatmapContent.Beatmap.BeatmapInfo.BeatmapSet.OnlineID != beatmapSetId)
5960
throw new InvariantException($"Beatmap has invalid beatmap set ID inside ({filename})");
60-
61-
if (!MetadataUtils.IsRomanised(beatmapContent.Beatmap.BeatmapInfo.DifficultyName))
62-
throw new InvariantException($"Difficulty name \"{beatmapContent.Beatmap.BeatmapInfo.DifficultyName}\" contains disallowed characters.");
6361
}
6462

65-
files.Add(new PackageFile(
63+
var file = new PackageFile(
6664
new beatmapset_file
6765
{
6866
sha2_hash = SHA256.HashData(stream),
@@ -73,7 +71,13 @@ public BeatmapPackageParseResult Parse(uint beatmapSetId, ArchiveReader archiveR
7371
filename = filename,
7472
},
7573
beatmapContent
76-
));
74+
);
75+
76+
var errors = new List<ValidationResult>();
77+
if (!Validator.TryValidateObject(file.VersionFile, new ValidationContext(file.VersionFile), errors, validateAllProperties: true))
78+
throw new InvariantException(string.Join(Environment.NewLine, errors.Select(r => r.ErrorMessage)));
79+
80+
files.Add(file);
7781
}
7882

7983
var beatmapSetRow = constructDatabaseRowForBeatmapset(beatmapSetId, archiveReader,
@@ -118,11 +122,9 @@ private osu_beatmapset constructDatabaseRowForBeatmapset(uint beatmapSetId, Arch
118122
if (creator != expectedCreator)
119123
throw new InvariantException("At least one difficulty has a specified creator that isn't the beatmap host's username.");
120124

121-
if (!MetadataUtils.IsRomanised(result.artist))
122-
throw new InvariantException("Romanised artist contains disallowed characters.");
123-
124-
if (!MetadataUtils.IsRomanised(result.title))
125-
throw new InvariantException("Romanised title contains disallowed characters.");
125+
var errors = new List<ValidationResult>();
126+
if (!Validator.TryValidateObject(result, new ValidationContext(result), errors, validateAllProperties: true))
127+
throw new InvariantException(string.Join(Environment.NewLine, errors.Select(r => r.ErrorMessage)));
126128

127129
// TODO: maybe unnecessary?
128130
result.displaytitle = $"[bold:0,size:20]{result.artist_unicode}|{result.title_unicode}";
@@ -186,6 +188,11 @@ public osu_beatmap GetDatabaseRow()
186188
};
187189

188190
countObjectsByType(Beatmap, result);
191+
192+
var errors = new List<ValidationResult>();
193+
if (!Validator.TryValidateObject(result, new ValidationContext(result), errors, validateAllProperties: true))
194+
throw new InvariantException(string.Join(Environment.NewLine, errors.Select(r => r.ErrorMessage)));
195+
189196
return result;
190197
}
191198

0 commit comments

Comments
 (0)