From a6a79613587e63c79e57322303cffa148654183c Mon Sep 17 00:00:00 2001 From: Jake Fulmine Date: Wed, 7 Feb 2024 16:30:55 +0100 Subject: [PATCH] fix: include query params when validating discord media URLs --- .../CommandSystem/Context/ContextAvatarExt.cs | 10 +++++++--- PluralKit.Bot/Commands/Groups.cs | 4 ++-- PluralKit.Bot/Commands/Member.cs | 14 +++++++++----- PluralKit.Bot/Commands/MemberAvatar.cs | 3 ++- PluralKit.Bot/Commands/MemberEdit.cs | 2 +- PluralKit.Bot/Commands/SystemEdit.cs | 6 +++--- PluralKit.Bot/Utils/AvatarUtils.cs | 2 +- 7 files changed, 25 insertions(+), 16 deletions(-) diff --git a/PluralKit.Bot/CommandSystem/Context/ContextAvatarExt.cs b/PluralKit.Bot/CommandSystem/Context/ContextAvatarExt.cs index 825443da..a42877d0 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..eca6043d 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 }); 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 }); var msg = img.Source switch { diff --git a/PluralKit.Bot/Commands/Member.cs b/PluralKit.Bot/Commands/Member.cs index 95998f67..82d40611 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 }, 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..ba01dd06 100644 --- a/PluralKit.Bot/Commands/MemberAvatar.cs +++ b/PluralKit.Bot/Commands/MemberAvatar.cs @@ -3,6 +3,7 @@ using Myriad.Builders; using Myriad.Types; using PluralKit.Core; +using Serilog; namespace PluralKit.Bot; @@ -139,7 +140,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); await PrintResponse(location, ctx, target, avatarArg.Value, guildData); } diff --git a/PluralKit.Bot/Commands/MemberEdit.cs b/PluralKit.Bot/Commands/MemberEdit.cs index 84b50015..44b62c6e 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 }); var msg = img.Source switch { diff --git a/PluralKit.Bot/Commands/SystemEdit.cs b/PluralKit.Bot/Commands/SystemEdit.cs index e6d3ddcd..2da521b8 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 }); 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 }); 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 }); var msg = img.Source switch { diff --git a/PluralKit.Bot/Utils/AvatarUtils.cs b/PluralKit.Bot/Utils/AvatarUtils.cs index 1fd573c4..da2aad1c 100644 --- a/PluralKit.Bot/Utils/AvatarUtils.cs +++ b/PluralKit.Bot/Utils/AvatarUtils.cs @@ -66,7 +66,7 @@ public static class AvatarUtils 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"; + "https://media.discordapp.net/attachments/$1/$2/$3.$4$5width=256&height=256"; public static string? TryRewriteCdnUrl(string? url) => url == null ? null : DiscordCdnUrl.Replace(url, DiscordMediaUrlReplacement);