From fafaf0dc8647a1e57c0886ad0f12d6d5bfa6dd04 Mon Sep 17 00:00:00 2001 From: Octavia Togami Date: Sat, 19 Sep 2026 21:06:06 -0700 Subject: [PATCH] Consolidate all HTTP to run over OkHttp The only reason for using it is that JDA does, if JDA changes we should follow suit. --- .../org/enginehub/discord/EngineHubBot.java | 2 + .../enginehub/discord/module/RoryFetch.java | 17 ++- .../module/errorhelper/ErrorHelper.java | 48 ++++----- .../resolver/RawSubdirectoryUrlResolver.java | 8 +- .../resolver/RawSubdomainUrlResolver.java | 8 +- .../enginehub/discord/util/HttpResult.java | 26 +++++ .../org/enginehub/discord/util/HttpUtil.java | 102 ++++++++++++++++++ .../org/enginehub/discord/util/PasteUtil.java | 57 +++++----- .../module/errorhelper/ErrorHelperTest.java | 13 +++ .../enginehub/discord/util/PasteUtilTest.java | 10 +- 10 files changed, 195 insertions(+), 96 deletions(-) create mode 100644 src/main/java/org/enginehub/discord/util/HttpResult.java create mode 100644 src/main/java/org/enginehub/discord/util/HttpUtil.java diff --git a/src/main/java/org/enginehub/discord/EngineHubBot.java b/src/main/java/org/enginehub/discord/EngineHubBot.java index 3953361..2764834 100644 --- a/src/main/java/org/enginehub/discord/EngineHubBot.java +++ b/src/main/java/org/enginehub/discord/EngineHubBot.java @@ -52,6 +52,7 @@ import org.enginehub.discord.module.RoryFetch; import org.enginehub.discord.module.SetProfilePicture; import org.enginehub.discord.module.errorhelper.ErrorHelper; +import org.enginehub.discord.util.HttpUtil; import org.enginehub.discord.util.PermissionRole; import org.enginehub.discord.util.command.CommandArgParser; import org.enginehub.discord.util.command.CommandRegistrationHandler; @@ -158,6 +159,7 @@ private EngineHubBot() throws LoginException, InterruptedException { bot = this; LOGGER.info("Connecting..."); api = JDABuilder.create(Settings.token, intents) + .setHttpClient(HttpUtil.getClient()) .setAutoReconnect(true) .addEventListeners(this) .enableCache(CacheFlag.EMOJI) diff --git a/src/main/java/org/enginehub/discord/module/RoryFetch.java b/src/main/java/org/enginehub/discord/module/RoryFetch.java index d803713..b83b787 100644 --- a/src/main/java/org/enginehub/discord/module/RoryFetch.java +++ b/src/main/java/org/enginehub/discord/module/RoryFetch.java @@ -29,6 +29,9 @@ import net.dv8tion.jda.api.EmbedBuilder; import net.dv8tion.jda.api.entities.Message; import net.dv8tion.jda.api.entities.MessageEmbed; +import okhttp3.Request; +import org.enginehub.discord.util.HttpResult; +import org.enginehub.discord.util.HttpUtil; import org.enginehub.discord.util.command.CommandRegistrationHandler; import org.enginehub.piston.CommandManager; import org.enginehub.piston.annotation.Command; @@ -40,9 +43,6 @@ import java.net.MalformedURLException; import java.net.URI; import java.net.URISyntaxException; -import java.net.http.HttpClient; -import java.net.http.HttpRequest; -import java.net.http.HttpResponse; import java.time.Instant; import java.util.HashMap; import java.util.Map; @@ -58,8 +58,6 @@ public class RoryFetch implements Module { new TypeReference<>() { }; - private final HttpClient client = HttpClient.newHttpClient(); - static { roryOverrides.put("gilmore", "https://i.pinimg.com/736x/25/ee/b7/25eeb71bb71aeee5574c50f96205d871.jpg"); } @@ -89,12 +87,9 @@ public void rory(Message message, @Arg(desc = "The ID of the rory image", def = url = url + '/' + roryId; } try { - HttpResponse response = client.send( - HttpRequest.newBuilder(new URI(url)).build(), - HttpResponse.BodyHandlers.ofString() - ); + HttpResult response = HttpUtil.send(new Request.Builder().url(new URI(url).toURL()).build()); - if (response.statusCode() == 404) { + if (response.code() == 404) { message.getChannel().sendMessage(message.getAuthor().getEffectiveName() + ", that's not a valid rory pic").queue(); return; } @@ -103,7 +98,7 @@ public void rory(Message message, @Arg(desc = "The ID of the rory image", def = message.getChannel().sendMessageEmbeds(createRoryEmbed(parsedResponse.get("id"), parsedResponse.get("url"))).queue(); } catch (MalformedURLException | URISyntaxException _) { message.getChannel().sendMessage(message.getAuthor().getEffectiveName() + ", that's an invalid URL!").queue(); - } catch (InterruptedException | IOException _) { + } catch (IOException _) { message.getChannel().sendMessage(message.getAuthor().getEffectiveName() + ", failed to lookup rory pic!").queue(); } } diff --git a/src/main/java/org/enginehub/discord/module/errorhelper/ErrorHelper.java b/src/main/java/org/enginehub/discord/module/errorhelper/ErrorHelper.java index dc4d08d..b132ba5 100644 --- a/src/main/java/org/enginehub/discord/module/errorhelper/ErrorHelper.java +++ b/src/main/java/org/enginehub/discord/module/errorhelper/ErrorHelper.java @@ -32,6 +32,8 @@ import net.sourceforge.tess4j.Tesseract; import ninja.leaping.configurate.ConfigurationNode; import ninja.leaping.configurate.objectmapping.ObjectMappingException; +import okhttp3.Request; +import okhttp3.Response; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.enginehub.discord.EngineHubBot; @@ -44,6 +46,7 @@ import org.enginehub.discord.module.errorhelper.resolver.MCLogsResolver; import org.enginehub.discord.module.errorhelper.resolver.RawSubdirectoryUrlResolver; import org.enginehub.discord.module.errorhelper.resolver.RawSubdomainUrlResolver; +import org.enginehub.discord.util.HttpUtil; import org.enginehub.discord.util.PasteUtil; import java.awt.image.BufferedImage; @@ -51,11 +54,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; -import java.net.URI; -import java.net.URISyntaxException; -import java.net.http.HttpClient; -import java.net.http.HttpRequest; -import java.net.http.HttpResponse; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; @@ -76,9 +74,6 @@ public class ErrorHelper extends ListenerAdapter implements Module { private static final Logger LOGGER = LogManager.getLogger(); - private static final HttpClient HTTP_CLIENT = HttpClient.newBuilder() - .followRedirects(HttpClient.Redirect.NORMAL) - .build(); private static final int LOG_SCAN_LIMIT = 1024 * 1024 * 50; // 50MB private final List resolvers = List.of( @@ -86,12 +81,11 @@ public class ErrorHelper extends ListenerAdapter implements Module { new RawSubdirectoryUrlResolver("pastebin.com", "raw"), // PastebinResolver new RawSubdirectoryUrlResolver("hastebin.com", "raw"), // HastebinResolver new RawSubdirectoryUrlResolver("paste.helpch.at", "raw"), // PasteHelpchatResolver - new RawSubdirectoryUrlResolver("paste.md-5.net", "raw"), // md-5.net new RawSubdomainUrlResolver("pastes.dev", "api"), // Pastes.dev new GhostbinResolver(), new GistResolver(), new MCLogsResolver(), - new RawSubdirectoryUrlResolver("paste.enginehub.org", "documents", true), // EngineHubResolver + new RawSubdirectoryUrlResolver("paste.enginehub.org", "documents"), // EngineHubResolver new IncompatibleResolver("mcpaste.io") ); @@ -141,17 +135,13 @@ public void scanMessage(Message message, User author, MessageChannel channel) { LOGGER.error("Failed to read attachment", e); } - try { - var _ = PasteUtil.sendToPastebin(messageText.toString()).thenAccept(url -> { - String responseUrl = url.toString(); - if (attachment.getFileName().equals("report.txt")) { - responseUrl = responseUrl + ".report"; - } - channel.sendMessage("[AutoReply] Here's a pasted version, " + responseUrl).queue(); - }); - } catch (IOException | URISyntaxException | InterruptedException e) { - LOGGER.error("Failed to send to EH Paste Service", e); - } + var _ = PasteUtil.sendToPastebin(messageText.toString()).thenAccept(url -> { + String responseUrl = url.toString(); + if (attachment.getFileName().equals("report.txt")) { + responseUrl = responseUrl + ".report"; + } + channel.sendMessage("[AutoReply] Here's a pasted version, " + responseUrl).queue(); + }); } } resolvers.parallelStream() @@ -196,15 +186,15 @@ private static String getStringFromUrl0(String url, int tries) { StringBuilder main = new StringBuilder(); try { - HttpResponse> response = HTTP_CLIENT.send( - HttpRequest.newBuilder(new URI(url)).build(), - HttpResponse.BodyHandlers.ofLines() - ); - try (Stream lines = response.body()) { - if (response.statusCode() >= 400) { - throw new IOException("HTTP " + response.statusCode()); + Request request = new Request.Builder().url(url).build(); + try (Response response = HttpUtil.getClient().newCall(request).execute()) { + if (response.code() >= 400) { + throw new IOException("HTTP " + response.code()); + } + + try (BufferedReader bodyStream = new BufferedReader(response.body().charStream())) { + bodyStream.lines().forEach(main::append); } - lines.forEach(main::append); } } catch (Throwable e) { LOGGER.warn("Failed to load URL " + url + " (Tries " + tries + ')', e); diff --git a/src/main/java/org/enginehub/discord/module/errorhelper/resolver/RawSubdirectoryUrlResolver.java b/src/main/java/org/enginehub/discord/module/errorhelper/resolver/RawSubdirectoryUrlResolver.java index ada3bd5..4ef5047 100644 --- a/src/main/java/org/enginehub/discord/module/errorhelper/resolver/RawSubdirectoryUrlResolver.java +++ b/src/main/java/org/enginehub/discord/module/errorhelper/resolver/RawSubdirectoryUrlResolver.java @@ -34,17 +34,11 @@ public class RawSubdirectoryUrlResolver implements ErrorResolver { private final Pattern urlPattern; private final String baseUrl; private final String subUrl; - private final boolean secure; public RawSubdirectoryUrlResolver(String baseUrl, String subUrl) { - this(baseUrl, subUrl, false); - } - - public RawSubdirectoryUrlResolver(String baseUrl, String subUrl, boolean secure) { this.baseUrl = baseUrl; this.subUrl = subUrl; this.urlPattern = Pattern.compile(baseUrl + "/([A-Za-z0-9._-]*)"); - this.secure = secure; } @Override @@ -52,7 +46,7 @@ public List foundText(String message) { List foundText = new ArrayList<>(); Matcher matcher = urlPattern.matcher(message); while (matcher.find()) { - foundText.add(ErrorHelper.getStringFromUrl((secure ? "https" : "http") + "://" + baseUrl + '/' + subUrl + '/' + matcher.group(1))); + foundText.add(ErrorHelper.getStringFromUrl("https://" + baseUrl + '/' + subUrl + '/' + matcher.group(1))); } return foundText; diff --git a/src/main/java/org/enginehub/discord/module/errorhelper/resolver/RawSubdomainUrlResolver.java b/src/main/java/org/enginehub/discord/module/errorhelper/resolver/RawSubdomainUrlResolver.java index 1b49430..ee88849 100644 --- a/src/main/java/org/enginehub/discord/module/errorhelper/resolver/RawSubdomainUrlResolver.java +++ b/src/main/java/org/enginehub/discord/module/errorhelper/resolver/RawSubdomainUrlResolver.java @@ -34,17 +34,11 @@ public class RawSubdomainUrlResolver implements ErrorResolver { private final Pattern urlPattern; private final String baseUrl; private final String subDomain; - private final boolean secure; public RawSubdomainUrlResolver(String baseUrl, String subDomain) { - this(baseUrl, subDomain, false); - } - - public RawSubdomainUrlResolver(String baseUrl, String subDomain, boolean secure) { this.baseUrl = baseUrl; this.subDomain = subDomain; this.urlPattern = Pattern.compile(baseUrl + "/([A-Za-z0-9._-]*)"); - this.secure = secure; } @Override @@ -52,7 +46,7 @@ public List foundText(String message) { List foundText = new ArrayList<>(); Matcher matcher = urlPattern.matcher(message); while (matcher.find()) { - foundText.add(ErrorHelper.getStringFromUrl((secure ? "https" : "http") + "://" + subDomain + "." + baseUrl + '/' + matcher.group(1))); + foundText.add(ErrorHelper.getStringFromUrl("https://" + subDomain + "." + baseUrl + '/' + matcher.group(1))); } return foundText; diff --git a/src/main/java/org/enginehub/discord/util/HttpResult.java b/src/main/java/org/enginehub/discord/util/HttpResult.java new file mode 100644 index 0000000..03464dd --- /dev/null +++ b/src/main/java/org/enginehub/discord/util/HttpResult.java @@ -0,0 +1,26 @@ +/* + * Copyright (c) EngineHub and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package org.enginehub.discord.util; + +public record HttpResult(int code, String body) { +} diff --git a/src/main/java/org/enginehub/discord/util/HttpUtil.java b/src/main/java/org/enginehub/discord/util/HttpUtil.java new file mode 100644 index 0000000..76c28eb --- /dev/null +++ b/src/main/java/org/enginehub/discord/util/HttpUtil.java @@ -0,0 +1,102 @@ +/* + * Copyright (c) EngineHub and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package org.enginehub.discord.util; + +import okhttp3.Call; +import okhttp3.Callback; +import okhttp3.ConnectionPool; +import okhttp3.Dispatcher; +import okhttp3.OkHttpClient; +import okhttp3.Request; +import okhttp3.Response; + +import java.io.IOException; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; + +public final class HttpUtil { + + private static final String USER_AGENT = "EngineHub-Bot (https://github.com/EngineHub/EngineHub-Bot)"; + private static final OkHttpClient CLIENT = createClient(); + + /** + * Get an HTTP client, using JDA's settings for connection pooling and request dispatching. + * We share this client across all requests to avoid creating too many connections and threads. + * + *

+ * Prefer using {@link #send(Request)} or {@link #sendAsync(Request)} for sending requests if possible in order to + * keep things simple. + *

+ * + * @return the client + */ + public static OkHttpClient getClient() { + return CLIENT; + } + + private static OkHttpClient createClient() { + Dispatcher dispatcher = new Dispatcher(); + dispatcher.setMaxRequestsPerHost(25); + return new OkHttpClient.Builder() + .addInterceptor(chain -> { + Request request = chain.request(); + if (request.header("User-Agent") == null) { + request = request.newBuilder().header("User-Agent", USER_AGENT).build(); + } + return chain.proceed(request); + }) + .dispatcher(dispatcher) + .connectionPool(new ConnectionPool(5, 10, TimeUnit.SECONDS)) + .followSslRedirects(false) + .build(); + } + + public static HttpResult send(Request request) throws IOException { + try (Response response = CLIENT.newCall(request).execute()) { + return new HttpResult(response.code(), response.body().string()); + } + } + + public static CompletableFuture sendAsync(Request request) { + var future = new CompletableFuture(); + CLIENT.newCall(request).enqueue(new Callback() { + @Override + public void onResponse(Call call, Response response) { + try (response) { + future.complete(new HttpResult(response.code(), response.body().string())); + } catch (Throwable t) { + future.completeExceptionally(t); + } + } + + @Override + public void onFailure(Call call, IOException e) { + future.completeExceptionally(e); + } + }); + return future; + } + + private HttpUtil() { + } +} diff --git a/src/main/java/org/enginehub/discord/util/PasteUtil.java b/src/main/java/org/enginehub/discord/util/PasteUtil.java index 242dd03..a3bc9c7 100644 --- a/src/main/java/org/enginehub/discord/util/PasteUtil.java +++ b/src/main/java/org/enginehub/discord/util/PasteUtil.java @@ -26,13 +26,11 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.datatype.jdk8.Jdk8Module; import com.fasterxml.jackson.module.paramnames.ParameterNamesModule; +import okhttp3.Request; +import okhttp3.RequestBody; -import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; -import java.net.http.HttpClient; -import java.net.http.HttpRequest; -import java.net.http.HttpResponse; import java.util.Map; import java.util.concurrent.CompletableFuture; @@ -41,16 +39,13 @@ public final class PasteUtil { public static final ObjectMapper OBJECT_MAPPER = new ObjectMapper() .registerModules(new Jdk8Module(), new ParameterNamesModule()); - private static HttpClient client = HttpClient.newHttpClient(); - - public static CompletableFuture sendToPastebin(String content) throws IOException, URISyntaxException, InterruptedException { - HttpRequest request = HttpRequest.newBuilder() - .uri(new URI("https://paste.enginehub.org/signed_paste_v2")) - .GET() + public static CompletableFuture sendToPastebin(String content) { + Request request = new Request.Builder() + .url("https://paste.enginehub.org/signed_paste_v2") .build(); - return client.sendAsync(request, HttpResponse.BodyHandlers.ofString()).thenApply(response -> { - if (response.statusCode() != 200) { + return HttpUtil.sendAsync(request).thenApply(response -> { + if (response.code() != 200) { throw new RuntimeException("Failed start paste signing: " + response.body()); } @@ -60,29 +55,25 @@ public static CompletableFuture sendToPastebin(String content) throws IOExc throw new RuntimeException(e); } }).thenCompose(signedPasteData -> { - try { - HttpRequest.Builder uploadRequestBuilder = HttpRequest.newBuilder() - .uri(new URI(signedPasteData.uploadUrl)) - .PUT(HttpRequest.BodyPublishers.ofString(content)); - - for (Map.Entry header : signedPasteData.headers.entrySet()) { - uploadRequestBuilder = uploadRequestBuilder.header(header.getKey(), header.getValue()); - } + Request.Builder uploadRequestBuilder = new Request.Builder() + .url(signedPasteData.uploadUrl) + .put(RequestBody.create(content, null)); - return client.sendAsync(uploadRequestBuilder.build(), HttpResponse.BodyHandlers.ofString()).thenApply(uploadResponse -> { - // If this succeeds, it will not return any data aside from a 204 status. - if (uploadResponse.statusCode() != 200 && uploadResponse.statusCode() != 204) { - throw new RuntimeException("Failed to upload paste: " + uploadResponse.body()); - } - try { - return new URI(signedPasteData.viewUrl); - } catch (URISyntaxException e) { - throw new RuntimeException(e); - } - }); - } catch (URISyntaxException e) { - throw new RuntimeException(e); + for (Map.Entry header : signedPasteData.headers.entrySet()) { + uploadRequestBuilder = uploadRequestBuilder.header(header.getKey(), header.getValue()); } + + return HttpUtil.sendAsync(uploadRequestBuilder.build()).thenApply(uploadResponse -> { + // If this succeeds, it will not return any data aside from a 204 status. + if (uploadResponse.code() != 200 && uploadResponse.code() != 204) { + throw new RuntimeException("Failed to upload paste: " + uploadResponse.body()); + } + try { + return new URI(signedPasteData.viewUrl); + } catch (URISyntaxException e) { + throw new RuntimeException(e); + } + }); }); } diff --git a/src/test/java/org/enginehub/discord/module/errorhelper/ErrorHelperTest.java b/src/test/java/org/enginehub/discord/module/errorhelper/ErrorHelperTest.java index debf60c..d8b07ad 100644 --- a/src/test/java/org/enginehub/discord/module/errorhelper/ErrorHelperTest.java +++ b/src/test/java/org/enginehub/discord/module/errorhelper/ErrorHelperTest.java @@ -82,6 +82,19 @@ public void testFollowsRedirect() { assertEquals("moved", ErrorHelper.getStringFromUrl(url("/old"))); } + @Test + public void testDoesNotFollowCrossSchemeRedirect() { + AtomicInteger requests = new AtomicInteger(); + server.createContext("/cross", exchange -> { + requests.getAndIncrement(); + exchange.getResponseHeaders().add("Location", "https://127.0.0.1:1/y"); + respond(exchange, 302, "blocked"); + }); + + assertEquals("blocked", ErrorHelper.getStringFromUrl(url("/cross"))); + assertEquals(1, requests.get()); + } + @Test public void testRetriesServerError() { AtomicInteger requests = new AtomicInteger(); diff --git a/src/test/java/org/enginehub/discord/util/PasteUtilTest.java b/src/test/java/org/enginehub/discord/util/PasteUtilTest.java index d421ff2..3f1ac7e 100644 --- a/src/test/java/org/enginehub/discord/util/PasteUtilTest.java +++ b/src/test/java/org/enginehub/discord/util/PasteUtilTest.java @@ -25,20 +25,12 @@ import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; -import java.io.IOException; -import java.net.URISyntaxException; -import java.util.concurrent.ExecutionException; - public class PasteUtilTest { // This test is ignored as it actually creates a paste on paste.enginehub.org @Disabled @Test public void testCreatesPaste() { - try { - System.out.println(PasteUtil.sendToPastebin("test").get()); - } catch (IOException | URISyntaxException | InterruptedException | ExecutionException e) { - throw new RuntimeException(e); - } + System.out.println(PasteUtil.sendToPastebin("test").join()); } }