From 05f1ee92ea9790217c2b86cef141ce5a71b17281 Mon Sep 17 00:00:00 2001 From: spiral Date: Sun, 22 Nov 2020 15:43:38 -0500 Subject: [PATCH 1/2] DRY-ify checking URL length for avatars --- PluralKit.Bot/Commands/Groups.cs | 2 -- PluralKit.Bot/Commands/MemberAvatar.cs | 9 +-------- PluralKit.Bot/Commands/SystemEdit.cs | 2 -- PluralKit.Bot/Utils/AvatarUtils.cs | 3 +++ 4 files changed, 4 insertions(+), 12 deletions(-) diff --git a/PluralKit.Bot/Commands/Groups.cs b/PluralKit.Bot/Commands/Groups.cs index 09ef467b..b0e45b9e 100644 --- a/PluralKit.Bot/Commands/Groups.cs +++ b/PluralKit.Bot/Commands/Groups.cs @@ -177,8 +177,6 @@ namespace PluralKit.Bot { ctx.CheckOwnGroup(target); - if (img.Url.Length > Limits.MaxUriLength) - throw Errors.InvalidUrl(img.Url); await AvatarUtils.VerifyAvatarOrThrow(img.Url); await _db.Execute(c => _repo.UpdateGroup(c, target.Id, new GroupPatch {Icon = img.Url})); diff --git a/PluralKit.Bot/Commands/MemberAvatar.cs b/PluralKit.Bot/Commands/MemberAvatar.cs index 6164b82c..786779b6 100644 --- a/PluralKit.Bot/Commands/MemberAvatar.cs +++ b/PluralKit.Bot/Commands/MemberAvatar.cs @@ -102,18 +102,11 @@ namespace PluralKit.Bot } ctx.CheckSystem().CheckOwnMember(target); - await ValidateUrl(avatarArg.Value.Url); + await AvatarUtils.VerifyAvatarOrThrow(avatarArg.Value.Url); await UpdateAvatar(location, ctx, target, avatarArg.Value.Url); await PrintResponse(location, ctx, target, avatarArg.Value, guildData); } - private static Task ValidateUrl(string url) - { - if (url.Length > Limits.MaxUriLength) - throw Errors.InvalidUrl(url); - return AvatarUtils.VerifyAvatarOrThrow(url); - } - private Task PrintResponse(AvatarLocation location, Context ctx, PKMember target, ParsedImage avatar, MemberGuildSettings? targetGuildData) { diff --git a/PluralKit.Bot/Commands/SystemEdit.cs b/PluralKit.Bot/Commands/SystemEdit.cs index 39676ebd..a4f641af 100644 --- a/PluralKit.Bot/Commands/SystemEdit.cs +++ b/PluralKit.Bot/Commands/SystemEdit.cs @@ -140,8 +140,6 @@ namespace PluralKit.Bot async Task SetIcon(ParsedImage img) { - if (img.Url.Length > Limits.MaxUriLength) - throw Errors.InvalidUrl(img.Url); await AvatarUtils.VerifyAvatarOrThrow(img.Url); await _db.Execute(c => _repo.UpdateSystem(c, ctx.System.Id, new SystemPatch {AvatarUrl = img.Url})); diff --git a/PluralKit.Bot/Utils/AvatarUtils.cs b/PluralKit.Bot/Utils/AvatarUtils.cs index 837fc2fc..df4881ab 100644 --- a/PluralKit.Bot/Utils/AvatarUtils.cs +++ b/PluralKit.Bot/Utils/AvatarUtils.cs @@ -11,6 +11,9 @@ namespace PluralKit.Bot { public static class AvatarUtils { public static async Task VerifyAvatarOrThrow(string url) { + if (url.Length > Limits.MaxUriLength) + throw Errors.UrlTooLong(url); + // List of MIME types we consider acceptable var acceptableMimeTypes = new[] { From 837b0a457d758f57c1cfa664b262d3f0b3d0833b Mon Sep 17 00:00:00 2001 From: spiral Date: Sun, 22 Nov 2020 16:15:26 -0500 Subject: [PATCH 2/2] Remove duplicates of CheckSystem/CheckOwnMember --- PluralKit.Bot/Commands/MemberEdit.cs | 46 +++++++++++---------------- PluralKit.Bot/Commands/MemberProxy.cs | 3 +- 2 files changed, 20 insertions(+), 29 deletions(-) diff --git a/PluralKit.Bot/Commands/MemberEdit.cs b/PluralKit.Bot/Commands/MemberEdit.cs index af8a2eec..52f49a4b 100644 --- a/PluralKit.Bot/Commands/MemberEdit.cs +++ b/PluralKit.Bot/Commands/MemberEdit.cs @@ -24,10 +24,9 @@ namespace PluralKit.Bot _repo = repo; } - public async Task Name(Context ctx, PKMember target) { - // TODO: this method is pretty much a 1:1 copy/paste of the above creation method, find a way to clean? - if (ctx.System == null) throw Errors.NoSystemError; - if (target.System != ctx.System.Id) throw Errors.NotOwnMemberError; + public async Task Name(Context ctx, PKMember target) + { + ctx.CheckSystem().CheckOwnMember(target); var newName = ctx.RemainderOrNull() ?? throw new PKSyntaxError("You must pass a new name for the member."); @@ -58,15 +57,10 @@ namespace PluralKit.Bot } } - private void CheckEditMemberPermission(Context ctx, PKMember target) - { - if (target.System != ctx.System?.Id) throw Errors.NotOwnMemberError; - } - public async Task Description(Context ctx, PKMember target) { if (await ctx.MatchClear("this member's description")) { - CheckEditMemberPermission(ctx, target); + ctx.CheckOwnMember(target); var patch = new MemberPatch {Description = Partial.Null()}; await _db.Execute(conn => _repo.UpdateMember(conn, target.Id, patch)); @@ -93,7 +87,7 @@ namespace PluralKit.Bot } else { - CheckEditMemberPermission(ctx, target); + ctx.CheckOwnMember(target); var description = ctx.RemainderOrNull().NormalizeLineEndSpacing(); if (description.IsLongerThan(Limits.MaxDescriptionLength)) @@ -109,7 +103,8 @@ namespace PluralKit.Bot public async Task Pronouns(Context ctx, PKMember target) { if (await ctx.MatchClear("this member's pronouns")) { - CheckEditMemberPermission(ctx, target); + ctx.CheckOwnMember(target); + var patch = new MemberPatch {Pronouns = Partial.Null()}; await _db.Execute(conn => _repo.UpdateMember(conn, target.Id, patch)); await ctx.Reply($"{Emojis.Success} Member pronouns cleared."); @@ -129,7 +124,7 @@ namespace PluralKit.Bot } else { - CheckEditMemberPermission(ctx, target); + ctx.CheckOwnMember(target); var pronouns = ctx.RemainderOrNull().NormalizeLineEndSpacing(); if (pronouns.IsLongerThan(Limits.MaxPronounsLength)) @@ -147,7 +142,7 @@ namespace PluralKit.Bot var color = ctx.RemainderOrNull(); if (await ctx.MatchClear()) { - CheckEditMemberPermission(ctx, target); + ctx.CheckOwnMember(target); var patch = new MemberPatch {Color = Partial.Null()}; await _db.Execute(conn => _repo.UpdateMember(conn, target.Id, patch)); @@ -176,7 +171,7 @@ namespace PluralKit.Bot } else { - CheckEditMemberPermission(ctx, target); + ctx.CheckOwnMember(target); if (color.StartsWith("#")) color = color.Substring(1); if (!Regex.IsMatch(color, "^[0-9a-fA-F]{6}$")) throw Errors.InvalidColorError(color); @@ -195,7 +190,7 @@ namespace PluralKit.Bot { if (await ctx.MatchClear("this member's birthday")) { - CheckEditMemberPermission(ctx, target); + ctx.CheckOwnMember(target); var patch = new MemberPatch {Birthday = Partial.Null()}; await _db.Execute(conn => _repo.UpdateMember(conn, target.Id, patch)); @@ -216,7 +211,7 @@ namespace PluralKit.Bot } else { - CheckEditMemberPermission(ctx, target); + ctx.CheckOwnMember(target); var birthdayStr = ctx.RemainderOrNull(); var birthday = DateUtils.ParseDate(birthdayStr, true); @@ -281,7 +276,7 @@ namespace PluralKit.Bot if (await ctx.MatchClear("this member's display name")) { - CheckEditMemberPermission(ctx, target); + ctx.CheckOwnMember(target); var patch = new MemberPatch {DisplayName = Partial.Null()}; await _db.Execute(conn => _repo.UpdateMember(conn, target.Id, patch)); @@ -298,7 +293,7 @@ namespace PluralKit.Bot } else { - CheckEditMemberPermission(ctx, target); + ctx.CheckOwnMember(target); var newDisplayName = ctx.RemainderOrNull(); @@ -315,7 +310,7 @@ namespace PluralKit.Bot if (await ctx.MatchClear("this member's server name")) { - CheckEditMemberPermission(ctx, target); + ctx.CheckOwnMember(target); var patch = new MemberGuildPatch {DisplayName = null}; await _db.Execute(conn => _repo.UpsertMemberGuild(conn, target.Id, ctx.Guild.Id, patch)); @@ -335,7 +330,7 @@ namespace PluralKit.Bot } else { - CheckEditMemberPermission(ctx, target); + ctx.CheckOwnMember(target); var newServerName = ctx.RemainderOrNull(); @@ -348,8 +343,7 @@ namespace PluralKit.Bot public async Task KeepProxy(Context ctx, PKMember target) { - if (ctx.System == null) throw Errors.NoSystemError; - if (target.System != ctx.System.Id) throw Errors.NotOwnMemberError; + ctx.CheckSystem().CheckOwnMember(target); bool newValue; if (ctx.Match("on", "enabled", "true", "yes")) newValue = true; @@ -375,8 +369,7 @@ namespace PluralKit.Bot public async Task Privacy(Context ctx, PKMember target, PrivacyLevel? newValueFromCommand) { - if (ctx.System == null) throw Errors.NoSystemError; - if (target.System != ctx.System.Id) throw Errors.NotOwnMemberError; + ctx.CheckSystem().CheckOwnMember(target); // Display privacy settings if (!ctx.HasNext() && newValueFromCommand == null) @@ -466,8 +459,7 @@ namespace PluralKit.Bot public async Task Delete(Context ctx, PKMember target) { - if (ctx.System == null) throw Errors.NoSystemError; - if (target.System != ctx.System.Id) throw Errors.NotOwnMemberError; + ctx.CheckSystem().CheckOwnMember(target); await ctx.Reply($"{Emojis.Warn} Are you sure you want to delete \"{target.NameFor(ctx)}\"? If so, reply to this message with the member's ID (`{target.Hid}`). __***This cannot be undone!***__"); if (!await ctx.ConfirmWithReply(target.Hid)) throw Errors.MemberDeleteCancelled; diff --git a/PluralKit.Bot/Commands/MemberProxy.cs b/PluralKit.Bot/Commands/MemberProxy.cs index fb9ef0fd..d09ea4cf 100644 --- a/PluralKit.Bot/Commands/MemberProxy.cs +++ b/PluralKit.Bot/Commands/MemberProxy.cs @@ -20,8 +20,7 @@ namespace PluralKit.Bot public async Task Proxy(Context ctx, PKMember target) { - if (ctx.System == null) throw Errors.NoSystemError; - if (target.System != ctx.System.Id) throw Errors.NotOwnMemberError; + ctx.CheckSystem().CheckOwnMember(target); ProxyTag ParseProxyTags(string exampleProxy) {