From 41427db178b0adbd75ef5a03e3439bce821833d2 Mon Sep 17 00:00:00 2001 From: Ske Date: Mon, 23 Aug 2021 22:53:58 +0200 Subject: [PATCH] Use a proper user agent when fetching images --- Myriad/Rest/DiscordApiClient.cs | 2 +- PluralKit.Bot/Bot.cs | 14 +++- PluralKit.Bot/Commands/Groups.cs | 9 ++- PluralKit.Bot/Commands/ImportExport.cs | 91 +++++++++++++------------- PluralKit.Bot/Commands/Member.cs | 2 +- PluralKit.Bot/Commands/MemberAvatar.cs | 7 +- PluralKit.Bot/Commands/MemberEdit.cs | 7 +- PluralKit.Bot/Commands/SystemEdit.cs | 10 +-- PluralKit.Bot/Modules.cs | 4 +- PluralKit.Bot/Utils/AvatarUtils.cs | 47 +++++++------ 10 files changed, 106 insertions(+), 87 deletions(-) diff --git a/Myriad/Rest/DiscordApiClient.cs b/Myriad/Rest/DiscordApiClient.cs index 77b73a81..e68ec0e5 100644 --- a/Myriad/Rest/DiscordApiClient.cs +++ b/Myriad/Rest/DiscordApiClient.cs @@ -12,7 +12,7 @@ namespace Myriad.Rest { public class DiscordApiClient { - private const string UserAgent = "DiscordBot (https://github.com/xSke/PluralKit/tree/main/Myriad/, vMyriad)"; + public const string UserAgent = "PluralKit (https://github.com/xSke/PluralKit/tree/main/Myriad/, v1)"; private readonly BaseRestClient _client; public DiscordApiClient(string token, ILogger logger) diff --git a/PluralKit.Bot/Bot.cs b/PluralKit.Bot/Bot.cs index aa72688b..64e731b0 100644 --- a/PluralKit.Bot/Bot.cs +++ b/PluralKit.Bot/Bot.cs @@ -180,11 +180,21 @@ namespace PluralKit.Bot await using var serviceScope = _services.BeginLifetimeScope(); // Find an event handler that can handle the type of event () we're given - var handler = serviceScope.Resolve>(); - var queue = serviceScope.ResolveOptional>(); + IEventHandler handler; + try + { + handler = serviceScope.Resolve>(); + } + catch (Exception e) + { + _logger.Error(e, "Error instantiating handler class"); + return; + } try { + var queue = serviceScope.ResolveOptional>(); + using var _ = LogContext.PushProperty("EventId", Guid.NewGuid()); using var __ = LogContext.Push(serviceScope.Resolve().GetEnricher(shard, evt)); _logger.Verbose("Received gateway event: {@Event}", evt); diff --git a/PluralKit.Bot/Commands/Groups.cs b/PluralKit.Bot/Commands/Groups.cs index b727d567..f3404004 100644 --- a/PluralKit.Bot/Commands/Groups.cs +++ b/PluralKit.Bot/Commands/Groups.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Net.Http; using System.Text; using System.Text.RegularExpressions; using System.Threading.Tasks; @@ -22,12 +23,14 @@ namespace PluralKit.Bot private readonly IDatabase _db; private readonly ModelRepository _repo; private readonly EmbedService _embeds; + private readonly HttpClient _client; - public Groups(IDatabase db, ModelRepository repo, EmbedService embeds) + public Groups(IDatabase db, ModelRepository repo, EmbedService embeds, HttpClient client) { _db = db; _repo = repo; _embeds = embeds; + _client = client; } public async Task CreateGroup(Context ctx) @@ -200,7 +203,7 @@ namespace PluralKit.Bot { ctx.CheckOwnGroup(target); - await AvatarUtils.VerifyAvatarOrThrow(img.Url); + await AvatarUtils.VerifyAvatarOrThrow(_client, img.Url); await _db.Execute(c => _repo.UpdateGroup(c, target.Id, new GroupPatch {Icon = img.Url})); @@ -262,7 +265,7 @@ namespace PluralKit.Bot { ctx.CheckOwnGroup(target); - await AvatarUtils.VerifyAvatarOrThrow(img.Url, isFullSizeImage: true); + await AvatarUtils.VerifyAvatarOrThrow(_client, img.Url, isFullSizeImage: true); await _db.Execute(c => _repo.UpdateGroup(c, target.Id, new GroupPatch {BannerImage = img.Url})); diff --git a/PluralKit.Bot/Commands/ImportExport.cs b/PluralKit.Bot/Commands/ImportExport.cs index 17d757d5..fd2f50e2 100644 --- a/PluralKit.Bot/Commands/ImportExport.cs +++ b/PluralKit.Bot/Commands/ImportExport.cs @@ -22,15 +22,17 @@ namespace PluralKit.Bot public class ImportExport { private readonly DataFileService _dataFiles; + private readonly HttpClient _client; private readonly JsonSerializerSettings _settings = new() { // Otherwise it'll mess up/reformat the ISO strings for ???some??? reason >.> DateParseHandling = DateParseHandling.None }; - public ImportExport(DataFileService dataFiles) + public ImportExport(DataFileService dataFiles, HttpClient client) { _dataFiles = dataFiles; + _client = client; } public async Task Import(Context ctx) @@ -40,57 +42,54 @@ namespace PluralKit.Bot await ctx.BusyIndicator(async () => { - using (var client = new HttpClient()) + HttpResponseMessage response; + try { - HttpResponseMessage response; - try - { - response = await client.GetAsync(url); - } - catch (InvalidOperationException) - { - // Invalid URL throws this, we just error back out - throw Errors.InvalidImportFile; - } + response = await _client.GetAsync(url); + } + catch (InvalidOperationException) + { + // Invalid URL throws this, we just error back out + throw Errors.InvalidImportFile; + } - if (!response.IsSuccessStatusCode) - throw Errors.InvalidImportFile; + if (!response.IsSuccessStatusCode) + throw Errors.InvalidImportFile; - DataFileSystem data; - try - { - var json = JsonConvert.DeserializeObject(await response.Content.ReadAsStringAsync(), _settings); - data = await LoadSystem(ctx, json); - } - catch (JsonException) - { - throw Errors.InvalidImportFile; - } + DataFileSystem data; + try + { + var json = JsonConvert.DeserializeObject(await response.Content.ReadAsStringAsync(), _settings); + data = await LoadSystem(ctx, json); + } + catch (JsonException) + { + throw Errors.InvalidImportFile; + } - if (!data.Valid) - throw Errors.InvalidImportFile; + if (!data.Valid) + throw Errors.InvalidImportFile; - if (data.LinkedAccounts != null && !data.LinkedAccounts.Contains(ctx.Author.Id)) - { - var msg = $"{Emojis.Warn} You seem to importing a system profile belonging to another account. Are you sure you want to proceed?"; - if (!await ctx.PromptYesNo(msg, "Import")) throw Errors.ImportCancelled; - } + if (data.LinkedAccounts != null && !data.LinkedAccounts.Contains(ctx.Author.Id)) + { + var msg = $"{Emojis.Warn} You seem to importing a system profile belonging to another account. Are you sure you want to proceed?"; + if (!await ctx.PromptYesNo(msg, "Import")) throw Errors.ImportCancelled; + } - // If passed system is null, it'll create a new one - // (and that's okay!) - var result = await _dataFiles.ImportSystem(data, ctx.System, ctx.Author.Id); - if (!result.Success) - await ctx.Reply($"{Emojis.Error} The provided system profile could not be imported. {result.Message}"); - else if (ctx.System == null) - { - // We didn't have a system prior to importing, so give them the new system's ID - await ctx.Reply($"{Emojis.Success} PluralKit has created a system for you based on the given file. Your system ID is `{result.System.Hid}`. Type `pk;system` for more information."); - } - else - { - // We already had a system, so show them what changed - await ctx.Reply($"{Emojis.Success} Updated {result.ModifiedNames.Count} members, created {result.AddedNames.Count} members. Type `pk;system list` to check!"); - } + // If passed system is null, it'll create a new one + // (and that's okay!) + var result = await _dataFiles.ImportSystem(data, ctx.System, ctx.Author.Id); + if (!result.Success) + await ctx.Reply($"{Emojis.Error} The provided system profile could not be imported. {result.Message}"); + else if (ctx.System == null) + { + // We didn't have a system prior to importing, so give them the new system's ID + await ctx.Reply($"{Emojis.Success} PluralKit has created a system for you based on the given file. Your system ID is `{result.System.Hid}`. Type `pk;system` for more information."); + } + else + { + // We already had a system, so show them what changed + await ctx.Reply($"{Emojis.Success} Updated {result.ModifiedNames.Count} members, created {result.AddedNames.Count} members. Type `pk;system list` to check!"); } }); } diff --git a/PluralKit.Bot/Commands/Member.cs b/PluralKit.Bot/Commands/Member.cs index eb88e589..620463a2 100644 --- a/PluralKit.Bot/Commands/Member.cs +++ b/PluralKit.Bot/Commands/Member.cs @@ -62,7 +62,7 @@ namespace PluralKit.Bot if (avatarArg != null) { try { - await AvatarUtils.VerifyAvatarOrThrow(avatarArg.Url); + await AvatarUtils.VerifyAvatarOrThrow(_client, avatarArg.Url); await _db.Execute(conn => _repo.UpdateMember(conn, member.Id, new MemberPatch { AvatarUrl = avatarArg.Url })); } catch (Exception e) { imageMatchError = e; diff --git a/PluralKit.Bot/Commands/MemberAvatar.cs b/PluralKit.Bot/Commands/MemberAvatar.cs index ae156e93..094d04a2 100644 --- a/PluralKit.Bot/Commands/MemberAvatar.cs +++ b/PluralKit.Bot/Commands/MemberAvatar.cs @@ -1,5 +1,6 @@ #nullable enable using System; +using System.Net.Http; using System.Threading.Tasks; using Myriad.Builders; @@ -12,11 +13,13 @@ namespace PluralKit.Bot { private readonly IDatabase _db; private readonly ModelRepository _repo; + private readonly HttpClient _client; - public MemberAvatar(IDatabase db, ModelRepository repo) + public MemberAvatar(IDatabase db, ModelRepository repo, HttpClient client) { _db = db; _repo = repo; + _client = client; } private async Task AvatarClear(AvatarLocation location, Context ctx, PKMember target, MemberGuildSettings? mgs) @@ -102,7 +105,7 @@ namespace PluralKit.Bot } ctx.CheckSystem().CheckOwnMember(target); - await AvatarUtils.VerifyAvatarOrThrow(avatarArg.Value.Url); + await AvatarUtils.VerifyAvatarOrThrow(_client, avatarArg.Value.Url); await UpdateAvatar(location, ctx, target, 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 ef31ad8e..53de9d75 100644 --- a/PluralKit.Bot/Commands/MemberEdit.cs +++ b/PluralKit.Bot/Commands/MemberEdit.cs @@ -1,6 +1,7 @@ using System.Text.RegularExpressions; using System.Threading.Tasks; using System; +using System.Net.Http; using Myriad.Builders; @@ -14,11 +15,13 @@ namespace PluralKit.Bot { private readonly IDatabase _db; private readonly ModelRepository _repo; + private readonly HttpClient _client; - public MemberEdit(IDatabase db, ModelRepository repo) + public MemberEdit(IDatabase db, ModelRepository repo, HttpClient client) { _db = db; _repo = repo; + _client = client; } public async Task Name(Context ctx, PKMember target) @@ -148,7 +151,7 @@ namespace PluralKit.Bot async Task SetBannerImage(ParsedImage img) { - await AvatarUtils.VerifyAvatarOrThrow(img.Url, isFullSizeImage: true); + await AvatarUtils.VerifyAvatarOrThrow(_client, img.Url, isFullSizeImage: true); await _db.Execute(c => _repo.UpdateMember(c, target.Id, new MemberPatch {BannerImage = img.Url})); diff --git a/PluralKit.Bot/Commands/SystemEdit.cs b/PluralKit.Bot/Commands/SystemEdit.cs index 0589a9e0..3a337f3e 100644 --- a/PluralKit.Bot/Commands/SystemEdit.cs +++ b/PluralKit.Bot/Commands/SystemEdit.cs @@ -1,10 +1,10 @@ using System; using System.Linq; +using System.Net.Http; using System.Text.RegularExpressions; using System.Threading.Tasks; using Myriad.Builders; -using Myriad.Types; using NodaTime; using NodaTime.Text; @@ -18,11 +18,13 @@ namespace PluralKit.Bot { private readonly IDatabase _db; private readonly ModelRepository _repo; + private readonly HttpClient _client; - public SystemEdit(IDatabase db, ModelRepository repo) + public SystemEdit(IDatabase db, ModelRepository repo, HttpClient client) { _db = db; _repo = repo; + _client = client; } public async Task Name(Context ctx) @@ -279,7 +281,7 @@ namespace PluralKit.Bot async Task SetIcon(ParsedImage img) { - await AvatarUtils.VerifyAvatarOrThrow(img.Url); + await AvatarUtils.VerifyAvatarOrThrow(_client, img.Url); await _db.Execute(c => _repo.UpdateSystem(c, ctx.System.Id, new SystemPatch {AvatarUrl = img.Url})); @@ -332,7 +334,7 @@ namespace PluralKit.Bot async Task SetImage(ParsedImage img) { - await AvatarUtils.VerifyAvatarOrThrow(img.Url, isFullSizeImage: true); + await AvatarUtils.VerifyAvatarOrThrow(_client, img.Url, isFullSizeImage: true); await _db.Execute(c => _repo.UpdateSystem(c, ctx.System.Id, new SystemPatch {BannerImage = img.Url})); diff --git a/PluralKit.Bot/Modules.cs b/PluralKit.Bot/Modules.cs index beca477f..47d54994 100644 --- a/PluralKit.Bot/Modules.cs +++ b/PluralKit.Bot/Modules.cs @@ -5,6 +5,7 @@ using Autofac; using Myriad.Cache; using Myriad.Gateway; +using Myriad.Rest; using NodaTime; @@ -113,7 +114,8 @@ namespace PluralKit.Bot // Utils builder.Register(c => new HttpClient { - Timeout = TimeSpan.FromSeconds(5) + Timeout = TimeSpan.FromSeconds(5), + DefaultRequestHeaders = {{"User-Agent", DiscordApiClient.UserAgent}} }).AsSelf().SingleInstance(); builder.RegisterInstance(SystemClock.Instance).As(); builder.RegisterType().AsSelf().SingleInstance(); diff --git a/PluralKit.Bot/Utils/AvatarUtils.cs b/PluralKit.Bot/Utils/AvatarUtils.cs index 6bad7f7c..f644a6b2 100644 --- a/PluralKit.Bot/Utils/AvatarUtils.cs +++ b/PluralKit.Bot/Utils/AvatarUtils.cs @@ -10,7 +10,7 @@ using SixLabors.ImageSharp; namespace PluralKit.Bot { public static class AvatarUtils { - public static async Task VerifyAvatarOrThrow(string url, bool isFullSizeImage = false) + public static async Task VerifyAvatarOrThrow(HttpClient client, string url, bool isFullSizeImage = false) { if (url.Length > Limits.MaxUriLength) throw Errors.UrlTooLong(url); @@ -24,35 +24,32 @@ namespace PluralKit.Bot { // TODO: add image/webp once ImageSharp supports this }; - using (var client = new HttpClient()) - { - if (!PluralKit.Core.MiscUtils.TryMatchUri(url, out var uri)) - throw Errors.InvalidUrl(url); + if (!PluralKit.Core.MiscUtils.TryMatchUri(url, out var uri)) + throw Errors.InvalidUrl(url); - url = TryRewriteCdnUrl(url); + url = TryRewriteCdnUrl(url); - var response = await client.GetAsync(url); - if (!response.IsSuccessStatusCode) // Check status code - throw Errors.AvatarServerError(response.StatusCode); - if (response.Content.Headers.ContentLength == null) // Check presence of content length - throw Errors.AvatarNotAnImage(null); - if (!acceptableMimeTypes.Contains(response.Content.Headers.ContentType.MediaType)) // Check MIME type - throw Errors.AvatarNotAnImage(response.Content.Headers.ContentType.MediaType); + var response = await client.GetAsync(url); + if (!response.IsSuccessStatusCode) // Check status code + throw Errors.AvatarServerError(response.StatusCode); + if (response.Content.Headers.ContentLength == null) // Check presence of content length + throw Errors.AvatarNotAnImage(null); + if (!acceptableMimeTypes.Contains(response.Content.Headers.ContentType.MediaType)) // Check MIME type + throw Errors.AvatarNotAnImage(response.Content.Headers.ContentType.MediaType); - if (isFullSizeImage) - // no need to do size checking on banners - return; + if (isFullSizeImage) + // no need to do size checking on banners + return; - if (response.Content.Headers.ContentLength > Limits.AvatarFileSizeLimit) // Check content length - throw Errors.AvatarFileSizeLimit(response.Content.Headers.ContentLength.Value); + if (response.Content.Headers.ContentLength > Limits.AvatarFileSizeLimit) // Check content length + throw Errors.AvatarFileSizeLimit(response.Content.Headers.ContentLength.Value); - // Parse the image header in a worker - var stream = await response.Content.ReadAsStreamAsync(); - var image = await Task.Run(() => Image.Identify(stream)); - if (image == null) throw Errors.AvatarInvalid; - if (image.Width > Limits.AvatarDimensionLimit || image.Height > Limits.AvatarDimensionLimit) // Check image size - throw Errors.AvatarDimensionsTooLarge(image.Width, image.Height); - } + // Parse the image header in a worker + var stream = await response.Content.ReadAsStreamAsync(); + var image = await Task.Run(() => Image.Identify(stream)); + if (image == null) throw Errors.AvatarInvalid; + if (image.Width > Limits.AvatarDimensionLimit || image.Height > Limits.AvatarDimensionLimit) // Check image size + throw Errors.AvatarDimensionsTooLarge(image.Width, image.Height); } // Rewrite cdn.discordapp.com URLs to media.discordapp.net for jpg/png files