From dcc15dc847f0bdcc35a7df9627fbe11726b8a5ae Mon Sep 17 00:00:00 2001 From: spiral Date: Sun, 1 Aug 2021 12:51:54 -0400 Subject: [PATCH] Move mediaproxy URL rewriting to ProxyService This shows full size avatars in API / cards. Also, rewrite URLs currently stored with media.discordapp.net "back" to cdn.discordapp.com before sending them to users. --- PluralKit.API/Controllers/v1/JsonModelExt.cs | 4 ++-- .../Commands/Avatars/ContextAvatarExt.cs | 14 ++----------- PluralKit.Bot/Commands/Groups.cs | 2 +- .../Commands/Lists/ContextListExt.cs | 2 +- PluralKit.Bot/Commands/MemberAvatar.cs | 2 +- PluralKit.Bot/Commands/SystemEdit.cs | 2 +- PluralKit.Bot/Proxy/ProxyService.cs | 2 +- PluralKit.Bot/Services/EmbedService.cs | 20 +++++++++---------- PluralKit.Bot/Utils/AvatarUtils.cs | 11 ++++++++++ PluralKit.Bot/Utils/ModelUtils.cs | 2 +- PluralKit.Core/Models/PKGroup.cs | 2 +- PluralKit.Core/Models/PKMember.cs | 2 +- PluralKit.Core/Services/DataFileService.cs | 4 ++-- PluralKit.Core/Utils/BulkImporter.cs | 2 +- PluralKit.Core/Utils/MiscUtils.cs | 8 ++++++++ 15 files changed, 44 insertions(+), 35 deletions(-) diff --git a/PluralKit.API/Controllers/v1/JsonModelExt.cs b/PluralKit.API/Controllers/v1/JsonModelExt.cs index b6d7934e..e65401a2 100644 --- a/PluralKit.API/Controllers/v1/JsonModelExt.cs +++ b/PluralKit.API/Controllers/v1/JsonModelExt.cs @@ -16,7 +16,7 @@ namespace PluralKit.API o.Add("name", system.Name); o.Add("description", system.DescriptionFor(ctx)); o.Add("tag", system.Tag); - o.Add("avatar_url", system.AvatarUrl); + o.Add("avatar_url", system.AvatarUrl.TryGetCleanCdnUrl()); o.Add("created", system.Created.FormatExport()); o.Add("tz", system.UiTz); o.Add("description_privacy", ctx == LookupContext.ByOwner ? system.DescriptionPrivacy.ToJsonString() : null); @@ -54,7 +54,7 @@ namespace PluralKit.API o.Add("display_name", member.NamePrivacy.CanAccess(ctx) ? member.DisplayName : null); o.Add("birthday", member.BirthdayFor(ctx)?.FormatExport()); o.Add("pronouns", member.PronounsFor(ctx)); - o.Add("avatar_url", member.AvatarFor(ctx)); + o.Add("avatar_url", member.AvatarFor(ctx).TryGetCleanCdnUrl()); o.Add("description", member.DescriptionFor(ctx)); var tagArray = new JArray(); diff --git a/PluralKit.Bot/Commands/Avatars/ContextAvatarExt.cs b/PluralKit.Bot/Commands/Avatars/ContextAvatarExt.cs index 43207639..850681e6 100644 --- a/PluralKit.Bot/Commands/Avatars/ContextAvatarExt.cs +++ b/PluralKit.Bot/Commands/Avatars/ContextAvatarExt.cs @@ -1,7 +1,6 @@ #nullable enable using System; using System.Linq; -using System.Text.RegularExpressions; using System.Threading.Tasks; using Myriad.Extensions; @@ -11,12 +10,6 @@ namespace PluralKit.Bot { public static class ContextAvatarExt { - // Rewrite cdn.discordapp.com URLs to media.discordapp.net for jpg/png files - // 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 Regex(@"^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 async Task MatchImage(this Context ctx) { // If we have a user @mention/ID, use their avatar @@ -41,13 +34,13 @@ namespace PluralKit.Bot throw Errors.InvalidUrl(arg); // ToString URL-decodes, which breaks URLs to spaces; AbsoluteUri doesn't - return new ParsedImage {Url = TryRewriteCdnUrl(uri.AbsoluteUri), Source = AvatarSource.Url}; + return new ParsedImage {Url = uri.AbsoluteUri, Source = AvatarSource.Url}; } // If we have an attachment, use that if (ctx.Message.Attachments.FirstOrDefault() is {} attachment) { - var url = TryRewriteCdnUrl(attachment.ProxyUrl); + var url = attachment.ProxyUrl; return new ParsedImage {Url = url, Source = AvatarSource.Attachment}; } @@ -55,9 +48,6 @@ namespace PluralKit.Bot // and if there are no attachments (which would have been caught just before) return null; } - - private static string TryRewriteCdnUrl(string url) => - DiscordCdnUrl.Replace(url, DiscordMediaUrlReplacement); } public struct ParsedImage diff --git a/PluralKit.Bot/Commands/Groups.cs b/PluralKit.Bot/Commands/Groups.cs index e82c9f41..66d9090b 100644 --- a/PluralKit.Bot/Commands/Groups.cs +++ b/PluralKit.Bot/Commands/Groups.cs @@ -227,7 +227,7 @@ namespace PluralKit.Bot { var eb = new EmbedBuilder() .Title("Group icon") - .Image(new(target.Icon)); + .Image(new(target.Icon.TryGetCleanCdnUrl())); if (target.System == ctx.System?.Id) { diff --git a/PluralKit.Bot/Commands/Lists/ContextListExt.cs b/PluralKit.Bot/Commands/Lists/ContextListExt.cs index fa990c4b..8e209708 100644 --- a/PluralKit.Bot/Commands/Lists/ContextListExt.cs +++ b/PluralKit.Bot/Commands/Lists/ContextListExt.cs @@ -154,7 +154,7 @@ namespace PluralKit.Bot profile.Append($"\n**Created on:** {created.FormatZoned(zone)}"); if (opts.IncludeAvatar && m.AvatarFor(lookupCtx) is {} avatar) - profile.Append($"\n**Avatar URL:** {avatar}"); + profile.Append($"\n**Avatar URL:** {avatar.TryGetCleanCdnUrl()}"); if (m.DescriptionFor(lookupCtx) is {} desc) profile.Append($"\n\n{desc}"); diff --git a/PluralKit.Bot/Commands/MemberAvatar.cs b/PluralKit.Bot/Commands/MemberAvatar.cs index 4afb6d15..ae156e93 100644 --- a/PluralKit.Bot/Commands/MemberAvatar.cs +++ b/PluralKit.Bot/Commands/MemberAvatar.cs @@ -60,7 +60,7 @@ namespace PluralKit.Bot var eb = new EmbedBuilder() .Title($"{target.NameFor(ctx)}'s {field}") - .Image(new(currentValue)); + .Image(new(currentValue?.TryGetCleanCdnUrl())); if (target.System == ctx.System?.Id) eb.Description($"To clear, use `pk;member {target.Reference()} {cmd} clear`."); await ctx.Reply(embed: eb.Build()); diff --git a/PluralKit.Bot/Commands/SystemEdit.cs b/PluralKit.Bot/Commands/SystemEdit.cs index a439041a..071f3b3f 100644 --- a/PluralKit.Bot/Commands/SystemEdit.cs +++ b/PluralKit.Bot/Commands/SystemEdit.cs @@ -203,7 +203,7 @@ namespace PluralKit.Bot { var eb = new EmbedBuilder() .Title("System icon") - .Image(new(ctx.System.AvatarUrl)) + .Image(new(ctx.System.AvatarUrl.TryGetCleanCdnUrl())) .Description("To clear, use `pk;system icon clear`."); await ctx.Reply(embed: eb.Build()); } diff --git a/PluralKit.Bot/Proxy/ProxyService.cs b/PluralKit.Bot/Proxy/ProxyService.cs index 1439cddb..3c2c9610 100644 --- a/PluralKit.Bot/Proxy/ProxyService.cs +++ b/PluralKit.Bot/Proxy/ProxyService.cs @@ -143,7 +143,7 @@ namespace PluralKit.Bot ChannelId = rootChannel.Id, ThreadId = threadId, Name = match.Member.ProxyName(ctx), - AvatarUrl = match.Member.ProxyAvatar(ctx), + AvatarUrl = AvatarUtils.TryRewriteCdnUrl(match.Member.ProxyAvatar(ctx)), Content = content, Attachments = trigger.Attachments, Embeds = embeds.ToArray(), diff --git a/PluralKit.Bot/Services/EmbedService.cs b/PluralKit.Bot/Services/EmbedService.cs index a2bb460e..55592930 100644 --- a/PluralKit.Bot/Services/EmbedService.cs +++ b/PluralKit.Bot/Services/EmbedService.cs @@ -66,7 +66,7 @@ namespace PluralKit.Bot { var eb = new EmbedBuilder() .Title(system.Name) - .Thumbnail(new(system.AvatarUrl)) + .Thumbnail(new(system.AvatarUrl.TryGetCleanCdnUrl())) .Footer(new($"System ID: {system.Hid} | Created on {system.Created.FormatZoned(system)}")) .Color(color); @@ -104,8 +104,8 @@ namespace PluralKit.Bot { var timestamp = DiscordUtils.SnowflakeToInstant(messageId); var name = member.NameFor(LookupContext.ByNonOwner); return new EmbedBuilder() - .Author(new($"#{channel.Name}: {name}", IconUrl: DiscordUtils.WorkaroundForUrlBug(member.AvatarFor(LookupContext.ByNonOwner)))) - .Thumbnail(new(member.AvatarFor(LookupContext.ByNonOwner))) + .Author(new($"#{channel.Name}: {name}", IconUrl: DiscordUtils.WorkaroundForUrlBug(member.AvatarFor(LookupContext.ByNonOwner).TryGetCleanCdnUrl()))) + .Thumbnail(new(member.AvatarFor(LookupContext.ByNonOwner).TryGetCleanCdnUrl())) .Description(content?.NormalizeLineEndSpacing()) .Footer(new($"System ID: {system.Hid} | Member ID: {member.Hid} | Sender: {sender.Username}#{sender.Discriminator} ({sender.Id}) | Message ID: {messageId} | Original Message ID: {originalMsgId}")) .Timestamp(timestamp.ToDateTimeOffset().ToString("O")) @@ -116,8 +116,8 @@ namespace PluralKit.Bot { var timestamp = DiscordUtils.SnowflakeToInstant(messageId); var name = member.NameFor(LookupContext.ByNonOwner); return new EmbedBuilder() - .Author(new($"[Edited] #{channel.Name}: {name}", IconUrl: DiscordUtils.WorkaroundForUrlBug(member.AvatarFor(LookupContext.ByNonOwner)))) - .Thumbnail(new(member.AvatarFor(LookupContext.ByNonOwner))) + .Author(new($"[Edited] #{channel.Name}: {name}", IconUrl: DiscordUtils.WorkaroundForUrlBug(member.AvatarFor(LookupContext.ByNonOwner).TryGetCleanCdnUrl()))) + .Thumbnail(new(member.AvatarFor(LookupContext.ByNonOwner).TryGetCleanCdnUrl())) .Field(new("Old message", oldContent?.NormalizeLineEndSpacing().Truncate(1000))) .Description(content?.NormalizeLineEndSpacing()) .Footer(new($"System ID: {system.Hid} | Member ID: {member.Hid} | Sender: {sender.Username}#{sender.Discriminator} ({sender.Id}) | Message ID: {messageId} | Original Message ID: {originalMsgId}")) @@ -159,7 +159,7 @@ namespace PluralKit.Bot { var eb = new EmbedBuilder() // TODO: add URL of website when that's up - .Author(new(name, IconUrl: DiscordUtils.WorkaroundForUrlBug(avatar))) + .Author(new(name, IconUrl: DiscordUtils.WorkaroundForUrlBug(avatar.TryGetCleanCdnUrl()))) // .WithColor(member.ColorPrivacy.CanAccess(ctx) ? color : DiscordUtils.Gray) .Color(color) .Footer(new( @@ -169,12 +169,12 @@ namespace PluralKit.Bot { if (member.MemberVisibility == PrivacyLevel.Private) description += "*(this member is hidden)*\n"; if (guildSettings?.AvatarUrl != null) if (member.AvatarFor(ctx) != null) - description += $"*(this member has a server-specific avatar set; [click here]({member.AvatarUrl}) to see the global avatar)*\n"; + description += $"*(this member has a server-specific avatar set; [click here]({member.AvatarUrl.TryGetCleanCdnUrl()}) to see the global avatar)*\n"; else description += "*(this member has a server-specific avatar set)*\n"; if (description != "") eb.Description(description); - if (avatar != null) eb.Thumbnail(new(avatar)); + if (avatar != null) eb.Thumbnail(new(avatar.TryGetCleanCdnUrl())); if (!member.DisplayName.EmptyOrNull() && member.NamePrivacy.CanAccess(ctx)) eb.Field(new("Display Name", member.DisplayName.Truncate(1024), true)); if (guild != null && guildDisplayName != null) eb.Field(new($"Server Nickname (for {guild.Name})", guildDisplayName.Truncate(1024), true)); @@ -248,7 +248,7 @@ namespace PluralKit.Bot { eb.Field(new("Description", desc)); if (target.IconFor(pctx) is {} icon) - eb.Thumbnail(new(icon)); + eb.Thumbnail(new(icon.TryGetCleanCdnUrl())); return eb.Build(); } @@ -316,7 +316,7 @@ namespace PluralKit.Bot { // Put it all together var eb = new EmbedBuilder() - .Author(new(msg.Member.NameFor(ctx), IconUrl: DiscordUtils.WorkaroundForUrlBug(msg.Member.AvatarFor(ctx)))) + .Author(new(msg.Member.NameFor(ctx), IconUrl: DiscordUtils.WorkaroundForUrlBug(msg.Member.AvatarFor(ctx).TryGetCleanCdnUrl()))) .Description(serverMsg?.Content?.NormalizeLineEndSpacing() ?? "*(message contents deleted or inaccessible)*") .Image(new(serverMsg?.Attachments?.FirstOrDefault()?.Url)) .Field(new("System", diff --git a/PluralKit.Bot/Utils/AvatarUtils.cs b/PluralKit.Bot/Utils/AvatarUtils.cs index 790c6bd3..1881d8e4 100644 --- a/PluralKit.Bot/Utils/AvatarUtils.cs +++ b/PluralKit.Bot/Utils/AvatarUtils.cs @@ -1,6 +1,7 @@ using System; using System.Linq; using System.Net.Http; +using System.Text.RegularExpressions; using System.Threading.Tasks; using PluralKit.Core; @@ -28,6 +29,8 @@ namespace PluralKit.Bot { if (!PluralKit.Core.MiscUtils.TryMatchUri(url, out var uri)) throw Errors.InvalidUrl(url); + url = TryRewriteCdnUrl(url); + var response = await client.GetAsync(uri); if (!response.IsSuccessStatusCode) // Check status code throw Errors.AvatarServerError(response.StatusCode); @@ -46,5 +49,13 @@ namespace PluralKit.Bot { throw Errors.AvatarDimensionsTooLarge(image.Width, image.Height); } } + + // Rewrite cdn.discordapp.com URLs to media.discordapp.net for jpg/png files + // 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 Regex(@"^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) => + DiscordCdnUrl.Replace(url, DiscordMediaUrlReplacement); } } \ No newline at end of file diff --git a/PluralKit.Bot/Utils/ModelUtils.cs b/PluralKit.Bot/Utils/ModelUtils.cs index 8c29093d..2429c09d 100644 --- a/PluralKit.Bot/Utils/ModelUtils.cs +++ b/PluralKit.Bot/Utils/ModelUtils.cs @@ -11,7 +11,7 @@ namespace PluralKit.Bot member.NameFor(ctx.LookupContextFor(member)); public static string AvatarFor(this PKMember member, Context ctx) => - member.AvatarFor(ctx.LookupContextFor(member)); + member.AvatarFor(ctx.LookupContextFor(member)).TryGetCleanCdnUrl(); public static string DisplayName(this PKMember member) => member.DisplayName ?? member.Name; diff --git a/PluralKit.Core/Models/PKGroup.cs b/PluralKit.Core/Models/PKGroup.cs index 21bea358..a76290c3 100644 --- a/PluralKit.Core/Models/PKGroup.cs +++ b/PluralKit.Core/Models/PKGroup.cs @@ -29,6 +29,6 @@ namespace PluralKit.Core group.DescriptionPrivacy.Get(ctx, group.Description); public static string? IconFor(this PKGroup group, LookupContext ctx) => - group.IconPrivacy.Get(ctx, group.Icon); + group.IconPrivacy.Get(ctx, group.Icon?.TryGetCleanCdnUrl()); } } \ No newline at end of file diff --git a/PluralKit.Core/Models/PKMember.cs b/PluralKit.Core/Models/PKMember.cs index 6976ff18..2d464ced 100644 --- a/PluralKit.Core/Models/PKMember.cs +++ b/PluralKit.Core/Models/PKMember.cs @@ -58,7 +58,7 @@ namespace PluralKit.Core { member.NamePrivacy.Get(ctx, member.Name, member.DisplayName ?? member.Name); public static string AvatarFor(this PKMember member, LookupContext ctx) => - member.AvatarPrivacy.Get(ctx, member.AvatarUrl); + member.AvatarPrivacy.Get(ctx, member.AvatarUrl.TryGetCleanCdnUrl()); public static string DescriptionFor(this PKMember member, LookupContext ctx) => member.DescriptionPrivacy.Get(ctx, member.Description); diff --git a/PluralKit.Core/Services/DataFileService.cs b/PluralKit.Core/Services/DataFileService.cs index cc259d3b..f4c06bb8 100644 --- a/PluralKit.Core/Services/DataFileService.cs +++ b/PluralKit.Core/Services/DataFileService.cs @@ -42,7 +42,7 @@ namespace PluralKit.Core Birthday = m.Birthday?.FormatExport(), Pronouns = m.Pronouns, Color = m.Color, - AvatarUrl = m.AvatarUrl, + AvatarUrl = m.AvatarUrl.TryGetCleanCdnUrl(), ProxyTags = m.ProxyTags, KeepProxy = m.KeepProxy, Created = m.Created.FormatExport(), @@ -127,7 +127,7 @@ namespace PluralKit.Core var patch = new SystemPatch {Name = data.Name}; if (data.Description != null) patch.Description = data.Description; if (data.Tag != null) patch.Tag = data.Tag; - if (data.AvatarUrl != null) patch.AvatarUrl = data.AvatarUrl; + if (data.AvatarUrl != null) patch.AvatarUrl = data.AvatarUrl.TryGetCleanCdnUrl(); if (data.TimeZone != null) patch.UiTz = data.TimeZone ?? "UTC"; await _repo.UpdateSystem(conn, system.Id, patch); diff --git a/PluralKit.Core/Utils/BulkImporter.cs b/PluralKit.Core/Utils/BulkImporter.cs index 1d409457..6055678e 100644 --- a/PluralKit.Core/Utils/BulkImporter.cs +++ b/PluralKit.Core/Utils/BulkImporter.cs @@ -101,7 +101,7 @@ namespace PluralKit.Core Description = patch.Description.Value, Pronouns = patch.Pronouns.Value, Color = patch.Color.Value, - AvatarUrl = patch.AvatarUrl.Value, + AvatarUrl = patch.AvatarUrl.Value?.TryGetCleanCdnUrl(), KeepProxy = patch.KeepProxy.Value, ProxyTags = patch.ProxyTags.Value, Birthday = patch.Birthday.Value, diff --git a/PluralKit.Core/Utils/MiscUtils.cs b/PluralKit.Core/Utils/MiscUtils.cs index bd21281a..e242b2ea 100644 --- a/PluralKit.Core/Utils/MiscUtils.cs +++ b/PluralKit.Core/Utils/MiscUtils.cs @@ -1,4 +1,5 @@ using System; +using System.Text.RegularExpressions; namespace PluralKit.Core { @@ -20,5 +21,12 @@ namespace PluralKit.Core return true; } + + // discord mediaproxy URLs used to be stored directly in the database, so now we cleanup image urls before using them outside of proxying + private static readonly Regex MediaProxyUrl = new Regex(@"^https?://media.discordapp.net/attachments/(\d{17,19})/(\d{17,19})/([^/\\&\?]+)\.(png|jpg|jpeg|webp)(\?.*)?$"); + private static readonly string DiscordCdnReplacement = "https://cdn.discordapp.com/attachments/$1/$2/$3.$4"; + public static string TryGetCleanCdnUrl(this string url) => + MediaProxyUrl.Replace(url, DiscordCdnReplacement); + } } \ No newline at end of file