From 8befb1c857e1269ca3dd23282de28a5832b5368a Mon Sep 17 00:00:00 2001 From: Jake Fulmine Date: Fri, 9 Feb 2024 04:38:17 +0100 Subject: [PATCH] fix(bot): avoid 403 status codes on image upload (#611) --- .../CommandSystem/Context/ContextAvatarExt.cs | 10 +++++++--- PluralKit.Bot/Commands/Groups.cs | 4 ++-- PluralKit.Bot/Commands/Member.cs | 14 +++++++++----- PluralKit.Bot/Commands/MemberAvatar.cs | 2 +- PluralKit.Bot/Commands/MemberEdit.cs | 2 +- PluralKit.Bot/Commands/SystemEdit.cs | 6 +++--- PluralKit.Bot/Utils/AvatarUtils.cs | 16 +++++++++++++--- 7 files changed, 36 insertions(+), 18 deletions(-) diff --git a/PluralKit.Bot/CommandSystem/Context/ContextAvatarExt.cs b/PluralKit.Bot/CommandSystem/Context/ContextAvatarExt.cs index 825443da..83bb1028 100644 --- a/PluralKit.Bot/CommandSystem/Context/ContextAvatarExt.cs +++ b/PluralKit.Bot/CommandSystem/Context/ContextAvatarExt.cs @@ -33,11 +33,14 @@ public static class ContextAvatarExt // If we have an attachment, use that if (ctx.Message.Attachments.FirstOrDefault() is { } attachment) { - // XXX: strip query params from attachment URLs because of new Discord CDN shenanigans + // XXX: discord attachment URLs are unable to be validated without their query params + // keep both the URL with query (for validation) and the clean URL (for storage) around var uriBuilder = new UriBuilder(attachment.ProxyUrl); - uriBuilder.Query = ""; - return new ParsedImage { Url = uriBuilder.Uri.AbsoluteUri, Source = AvatarSource.Attachment }; + ParsedImage img = new ParsedImage { Url = uriBuilder.Uri.AbsoluteUri, Source = AvatarSource.Attachment }; + uriBuilder.Query = ""; + img.CleanUrl = uriBuilder.Uri.AbsoluteUri; + return img; } // We should only get here if there are no arguments (which would get parsed as URL + throw if error) @@ -49,6 +52,7 @@ public static class ContextAvatarExt public struct ParsedImage { public string Url; + public string? CleanUrl; public AvatarSource Source; public User? SourceUser; } diff --git a/PluralKit.Bot/Commands/Groups.cs b/PluralKit.Bot/Commands/Groups.cs index a16fe9e5..e3f539a2 100644 --- a/PluralKit.Bot/Commands/Groups.cs +++ b/PluralKit.Bot/Commands/Groups.cs @@ -263,7 +263,7 @@ public class Groups await AvatarUtils.VerifyAvatarOrThrow(_client, img.Url); - await ctx.Repository.UpdateGroup(target.Id, new GroupPatch { Icon = img.Url }); + await ctx.Repository.UpdateGroup(target.Id, new GroupPatch { Icon = img.CleanUrl ?? img.Url }); var msg = img.Source switch { @@ -328,7 +328,7 @@ public class Groups await AvatarUtils.VerifyAvatarOrThrow(_client, img.Url, true); - await ctx.Repository.UpdateGroup(target.Id, new GroupPatch { BannerImage = img.Url }); + await ctx.Repository.UpdateGroup(target.Id, new GroupPatch { BannerImage = img.CleanUrl ?? img.Url }); var msg = img.Source switch { diff --git a/PluralKit.Bot/Commands/Member.cs b/PluralKit.Bot/Commands/Member.cs index 95998f67..a34087c1 100644 --- a/PluralKit.Bot/Commands/Member.cs +++ b/PluralKit.Bot/Commands/Member.cs @@ -70,14 +70,18 @@ public class Member if (avatarArg != null) try { - // XXX: strip query params from attachment URLs because of new Discord CDN shenanigans - var uriBuilder = new UriBuilder(avatarArg.Url); + // XXX: discord attachment URLs are unable to be validated without their query params + // keep both the URL with query (for validation) and the clean URL (for storage) around + var uriBuilder = new UriBuilder(avatarArg.ProxyUrl); + ParsedImage img = new ParsedImage { Url = uriBuilder.Uri.AbsoluteUri, Source = AvatarSource.Attachment }; + uriBuilder.Query = ""; + img.CleanUrl = uriBuilder.Uri.AbsoluteUri; - await AvatarUtils.VerifyAvatarOrThrow(_client, uriBuilder.Uri.AbsoluteUri); - await ctx.Repository.UpdateMember(member.Id, new MemberPatch { AvatarUrl = uriBuilder.Uri.AbsoluteUri }, conn); + await AvatarUtils.VerifyAvatarOrThrow(_client, img.Url); + await ctx.Repository.UpdateMember(member.Id, new MemberPatch { AvatarUrl = img.CleanUrl ?? img.Url }, conn); - dispatchData.Add("avatar_url", avatarArg.Url); + dispatchData.Add("avatar_url", img.CleanUrl); } catch (Exception e) { diff --git a/PluralKit.Bot/Commands/MemberAvatar.cs b/PluralKit.Bot/Commands/MemberAvatar.cs index d5f5f544..7986e9d4 100644 --- a/PluralKit.Bot/Commands/MemberAvatar.cs +++ b/PluralKit.Bot/Commands/MemberAvatar.cs @@ -139,7 +139,7 @@ public class MemberAvatar ctx.CheckSystem().CheckOwnMember(target); await AvatarUtils.VerifyAvatarOrThrow(_client, avatarArg.Value.Url); - await UpdateAvatar(location, ctx, target, avatarArg.Value.Url); + await UpdateAvatar(location, ctx, target, avatarArg.Value.CleanUrl ?? avatarArg.Value.Url); await PrintResponse(location, ctx, target, avatarArg.Value, guildData); } diff --git a/PluralKit.Bot/Commands/MemberEdit.cs b/PluralKit.Bot/Commands/MemberEdit.cs index 84b50015..b6193249 100644 --- a/PluralKit.Bot/Commands/MemberEdit.cs +++ b/PluralKit.Bot/Commands/MemberEdit.cs @@ -182,7 +182,7 @@ public class MemberEdit { await AvatarUtils.VerifyAvatarOrThrow(_client, img.Url, true); - await ctx.Repository.UpdateMember(target.Id, new MemberPatch { BannerImage = img.Url }); + await ctx.Repository.UpdateMember(target.Id, new MemberPatch { BannerImage = img.CleanUrl ?? img.Url }); var msg = img.Source switch { diff --git a/PluralKit.Bot/Commands/SystemEdit.cs b/PluralKit.Bot/Commands/SystemEdit.cs index e6d3ddcd..b1a73e74 100644 --- a/PluralKit.Bot/Commands/SystemEdit.cs +++ b/PluralKit.Bot/Commands/SystemEdit.cs @@ -475,7 +475,7 @@ public class SystemEdit await AvatarUtils.VerifyAvatarOrThrow(_client, img.Url); - await ctx.Repository.UpdateSystem(target.Id, new SystemPatch { AvatarUrl = img.Url }); + await ctx.Repository.UpdateSystem(target.Id, new SystemPatch { AvatarUrl = img.CleanUrl ?? img.Url }); var msg = img.Source switch { @@ -543,7 +543,7 @@ public class SystemEdit await AvatarUtils.VerifyAvatarOrThrow(_client, img.Url); - await ctx.Repository.UpdateSystemGuild(target.Id, ctx.Guild.Id, new SystemGuildPatch { AvatarUrl = img.Url }); + await ctx.Repository.UpdateSystemGuild(target.Id, ctx.Guild.Id, new SystemGuildPatch { AvatarUrl = img.CleanUrl ?? img.Url }); var msg = img.Source switch { @@ -640,7 +640,7 @@ public class SystemEdit { await AvatarUtils.VerifyAvatarOrThrow(_client, img.Url, true); - await ctx.Repository.UpdateSystem(target.Id, new SystemPatch { BannerImage = img.Url }); + await ctx.Repository.UpdateSystem(target.Id, new SystemPatch { BannerImage = img.CleanUrl ?? img.Url }); var msg = img.Source switch { diff --git a/PluralKit.Bot/Utils/AvatarUtils.cs b/PluralKit.Bot/Utils/AvatarUtils.cs index 1fd573c4..00f3059f 100644 --- a/PluralKit.Bot/Utils/AvatarUtils.cs +++ b/PluralKit.Bot/Utils/AvatarUtils.cs @@ -63,11 +63,21 @@ public static class AvatarUtils // This lets us add resizing parameters to "borrow" their media proxy server to downsize the image // which in turn makes it more likely to be underneath the size limit! private static readonly Regex DiscordCdnUrl = - new(@"^https?://(?:cdn\.discordapp\.com|media\.discordapp\.net)/attachments/(\d{17,19})/(\d{17,19})/([^/\\&\?]+)\.(png|jpg|jpeg|webp)(\?.*)?$"); + new(@"^https?://(?:cdn\.discordapp\.com|media\.discordapp\.net)/attachments/(\d{17,19})/(\d{17,19})/([^/\\&\?]+)\.(png|jpg|jpeg|webp)(?:\?(?.*))?$"); private static readonly string DiscordMediaUrlReplacement = "https://media.discordapp.net/attachments/$1/$2/$3.$4?width=256&height=256"; - public static string? TryRewriteCdnUrl(string? url) => - url == null ? null : DiscordCdnUrl.Replace(url, DiscordMediaUrlReplacement); + public static string? TryRewriteCdnUrl(string? url) + { + if (url == null) + return null; + + var match = DiscordCdnUrl.Match(url); + var newUrl = DiscordCdnUrl.Replace(url, DiscordMediaUrlReplacement); + if (match.Groups["query"].Success) + newUrl += "&" + match.Groups["query"].Value; + + return newUrl; + } } \ No newline at end of file