From 81341c40180813abecdfe5a87c467abe322cab54 Mon Sep 17 00:00:00 2001 From: Geoff Bourne Date: Mon, 13 Feb 2023 08:32:04 -0600 Subject: [PATCH] http: configurable timeout for response and TLS handshake --- .../curseforge/CurseForgeInstaller.java | 11 ++++++-- .../curseforge/InstallCurseForgeCommand.java | 8 +++++- .../fabric/FabricLauncherInstaller.java | 3 +- src/main/java/me/itzg/helpers/http/Fetch.java | 5 ++-- .../itzg/helpers/http/FetchBuilderBase.java | 3 +- .../me/itzg/helpers/http/SharedFetch.java | 28 ++++++++++++++++++- .../me/itzg/helpers/http/SharedFetchArgs.java | 26 +++++++++++++++++ .../me/itzg/helpers/env/InterpolatorTest.java | 2 +- 8 files changed, 77 insertions(+), 9 deletions(-) create mode 100644 src/main/java/me/itzg/helpers/http/SharedFetchArgs.java diff --git a/src/main/java/me/itzg/helpers/curseforge/CurseForgeInstaller.java b/src/main/java/me/itzg/helpers/curseforge/CurseForgeInstaller.java index ea951212..c0b14024 100644 --- a/src/main/java/me/itzg/helpers/curseforge/CurseForgeInstaller.java +++ b/src/main/java/me/itzg/helpers/curseforge/CurseForgeInstaller.java @@ -47,6 +47,7 @@ import me.itzg.helpers.forge.ForgeInstaller; import me.itzg.helpers.http.Fetch; import me.itzg.helpers.http.SharedFetch; +import me.itzg.helpers.http.SharedFetch.Options; import me.itzg.helpers.http.UriBuilder; import me.itzg.helpers.json.ObjectMappers; import reactor.core.publisher.Flux; @@ -94,6 +95,9 @@ public class CurseForgeInstaller { @Getter @Setter private boolean overridesSkipExisting; + @Getter @Setter + private SharedFetch.Options sharedFetchOptions; + private final Set applicableClassIdSlugs = new HashSet<>(Arrays.asList( "mc-mods", "bukkit-plugins", @@ -106,14 +110,17 @@ public void install(String slug, String fileMatcher, Integer fileId) throws IOEx final UriBuilder uriBuilder = UriBuilder.withBaseUrl(apiBaseUrl); - try (SharedFetch preparedFetch = Fetch.sharedFetch("install-curseforge")) { + try (SharedFetch preparedFetch = Fetch.sharedFetch("install-curseforge", + sharedFetchOptions != null ? sharedFetchOptions : Options.builder().build() + )) { // TODO encapsulate preparedFetch and uriBuilder to avoid passing deep into call tree final CategoryInfo categoryInfo = loadCategoryInfo(preparedFetch, uriBuilder); final ModsSearchResponse searchResponse = preparedFetch.fetch( uriBuilder.resolve("/mods/search?gameId={gameId}&slug={slug}&classId={classId}", - MINECRAFT_GAME_ID, slug, categoryInfo.modpackClassId) + MINECRAFT_GAME_ID, slug, categoryInfo.modpackClassId + ) ) .toObject(ModsSearchResponse.class) .execute(); diff --git a/src/main/java/me/itzg/helpers/curseforge/InstallCurseForgeCommand.java b/src/main/java/me/itzg/helpers/curseforge/InstallCurseForgeCommand.java index 77ba08cf..ca806bc5 100644 --- a/src/main/java/me/itzg/helpers/curseforge/InstallCurseForgeCommand.java +++ b/src/main/java/me/itzg/helpers/curseforge/InstallCurseForgeCommand.java @@ -11,6 +11,7 @@ import me.itzg.helpers.files.ResultsFileWriter; import me.itzg.helpers.http.PathOrUri; import me.itzg.helpers.http.PathOrUriConverter; +import me.itzg.helpers.http.SharedFetchArgs; import me.itzg.helpers.json.ObjectMappers; import picocli.CommandLine.ArgGroup; import picocli.CommandLine.Command; @@ -98,6 +99,9 @@ static class Listed { ) boolean overridesSkipExisting; + @ArgGroup(exclusive = false) + SharedFetchArgs sharedFetchArgs = new SharedFetchArgs(); + private static final Pattern PAGE_URL_PATTERN = Pattern.compile( "https://www.curseforge.com/minecraft/modpacks/(?.+?)(/files(/(?\\d+)?)?)?"); @@ -133,7 +137,9 @@ public Integer call() throws Exception { .setForceSynchronize(forceSynchronize) .setParallelism(parallelDownloads) .setLevelFrom(levelFrom) - .setOverridesSkipExisting(overridesSkipExisting); + .setOverridesSkipExisting(overridesSkipExisting) + .setSharedFetchOptions(sharedFetchArgs.options()); + installer.install(slug, filenameMatcher, fileId); return ExitCode.OK; diff --git a/src/main/java/me/itzg/helpers/fabric/FabricLauncherInstaller.java b/src/main/java/me/itzg/helpers/fabric/FabricLauncherInstaller.java index e3427ea9..7320d622 100644 --- a/src/main/java/me/itzg/helpers/fabric/FabricLauncherInstaller.java +++ b/src/main/java/me/itzg/helpers/fabric/FabricLauncherInstaller.java @@ -25,6 +25,7 @@ import me.itzg.helpers.files.ResultsFileWriter; import me.itzg.helpers.http.FailedRequestException; import me.itzg.helpers.http.SharedFetch; +import me.itzg.helpers.http.SharedFetch.Options; import me.itzg.helpers.http.UriBuilder; import reactor.core.publisher.Mono; import reactor.core.scheduler.Schedulers; @@ -56,7 +57,7 @@ public Path installUsingVersions(@NonNull String minecraftVersion, String loader final UriBuilder uriBuilder = UriBuilder.withBaseUrl(fabricMetaBaseUrl); - try (SharedFetch sharedFetch = sharedFetch("fabric")) { + try (SharedFetch sharedFetch = sharedFetch("fabric", Options.builder().build())) { loaderVersion = resolveLoaderVersion(uriBuilder, sharedFetch, minecraftVersion, loaderVersion); installerVersion = resolveInstallerVersion(uriBuilder, sharedFetch, installerVersion); diff --git a/src/main/java/me/itzg/helpers/http/Fetch.java b/src/main/java/me/itzg/helpers/http/Fetch.java index 6fa09652..c11e1fe3 100644 --- a/src/main/java/me/itzg/helpers/http/Fetch.java +++ b/src/main/java/me/itzg/helpers/http/Fetch.java @@ -1,6 +1,7 @@ package me.itzg.helpers.http; import java.net.URI; +import me.itzg.helpers.http.SharedFetch.Options; public class Fetch { @@ -15,8 +16,8 @@ public static FetchBuilderBase fetch(URI uri) { * Provides an efficient way to make multiple web requests since a single client * is shared. */ - public static SharedFetch sharedFetch(String forCommand) { - return new SharedFetch(forCommand); + public static SharedFetch sharedFetch(String forCommand, Options options) { + return new SharedFetch(forCommand, options); } private Fetch() { diff --git a/src/main/java/me/itzg/helpers/http/FetchBuilderBase.java b/src/main/java/me/itzg/helpers/http/FetchBuilderBase.java index a6baa09d..d9d0ff73 100644 --- a/src/main/java/me/itzg/helpers/http/FetchBuilderBase.java +++ b/src/main/java/me/itzg/helpers/http/FetchBuilderBase.java @@ -17,6 +17,7 @@ import java.util.function.BiConsumer; import lombok.extern.slf4j.Slf4j; import me.itzg.helpers.errors.GenericException; +import me.itzg.helpers.http.SharedFetch.Options; import me.itzg.helpers.json.ObjectMappers; import org.slf4j.Logger; import reactor.core.publisher.Mono; @@ -128,7 +129,7 @@ protected R useReactiveClient(ReactiveClientUser user) { return user.use(state.sharedFetch.getReactiveClient()); } else { - try (SharedFetch sharedFetch = new SharedFetch(state.userAgentCommand)) { + try (SharedFetch sharedFetch = new SharedFetch(state.userAgentCommand, Options.builder().build())) { return user.use(sharedFetch.getReactiveClient()); } } diff --git a/src/main/java/me/itzg/helpers/http/SharedFetch.java b/src/main/java/me/itzg/helpers/http/SharedFetch.java index 40f5fd5d..f1027155 100644 --- a/src/main/java/me/itzg/helpers/http/SharedFetch.java +++ b/src/main/java/me/itzg/helpers/http/SharedFetch.java @@ -2,12 +2,16 @@ import io.netty.handler.codec.http.HttpHeaderNames; import java.net.URI; +import java.time.Duration; import java.util.HashMap; import java.util.Map; import java.util.UUID; +import lombok.Builder; +import lombok.Builder.Default; import lombok.Getter; import lombok.extern.slf4j.Slf4j; import me.itzg.helpers.McImageHelper; +import reactor.netty.http.Http11SslContextSpec; import reactor.netty.http.client.HttpClient; /** @@ -29,7 +33,7 @@ public class SharedFetch implements AutoCloseable { @Getter private final HttpClient reactiveClient; - public SharedFetch(String forCommand) { + public SharedFetch(String forCommand, Options options) { final String userAgent = String.format("%s/%s (cmd=%s)", "mc-image-helper", McImageHelper.getVersion(), @@ -43,6 +47,13 @@ public SharedFetch(String forCommand) { headers .set(HttpHeaderNames.USER_AGENT.toString(), userAgent) .set("x-fetch-session", fetchSessionId) + ) + // Reference https://projectreactor.io/docs/netty/release/reference/index.html#response-timeout + .responseTimeout(options.getResponseTimeout()) + // Reference https://projectreactor.io/docs/netty/release/reference/index.html#ssl-tls-timeout + .secure(spec -> + spec.sslContext(Http11SslContextSpec.forClient()) + .handshakeTimeout(options.getTlsHandshakeTimeout()) ); headers.put("x-fetch-session", fetchSessionId); @@ -61,4 +72,19 @@ public SharedFetch addHeader(String name, String value) { @Override public void close() { } + + @Builder + @Getter + public static class Options { + + @Default + private final Duration responseTimeout + // not set by default + = Duration.ofSeconds(5); + + @Default + private final Duration tlsHandshakeTimeout + // double the Netty default + = Duration.ofSeconds(20); + } } diff --git a/src/main/java/me/itzg/helpers/http/SharedFetchArgs.java b/src/main/java/me/itzg/helpers/http/SharedFetchArgs.java new file mode 100644 index 00000000..c0acd5b6 --- /dev/null +++ b/src/main/java/me/itzg/helpers/http/SharedFetchArgs.java @@ -0,0 +1,26 @@ +package me.itzg.helpers.http; + +import java.time.Duration; +import me.itzg.helpers.http.SharedFetch.Options; +import picocli.CommandLine.Option; + +public class SharedFetchArgs { + + private final Options.OptionsBuilder optionsBuilder = Options.builder(); + + @Option(names = "--http-response-timeout", defaultValue = "${env:FETCH_RESPONSE_TIMEOUT}", + description = "The response timeout to apply to HTTP operations. Parsed from ISO-8601 format." + ) + public void setResponseTimeout(Duration timeout) { + optionsBuilder.responseTimeout(timeout); + } + + @Option(names = "--tls-handshake-timeout", defaultValue = "${env:FETCH_TLS_HANDSHAKE_TIMEOUT}") + public void setTlsHandshakeTimeout(Duration timeout) { + optionsBuilder.tlsHandshakeTimeout(timeout); + } + + public Options options() { + return optionsBuilder.build(); + } +} diff --git a/src/test/java/me/itzg/helpers/env/InterpolatorTest.java b/src/test/java/me/itzg/helpers/env/InterpolatorTest.java index b94ec80f..3b83f4e2 100644 --- a/src/test/java/me/itzg/helpers/env/InterpolatorTest.java +++ b/src/test/java/me/itzg/helpers/env/InterpolatorTest.java @@ -31,7 +31,7 @@ void typicalReplacements() throws IOException { } @Test - void interpolateToValueWithDollarSignInValue() throws IOException { + void interpolateToValueWithDollarSign() throws IOException { when(varProvider.get("CFG_HAS_DOLLAR_VALUE_FILE")) .thenReturn(null); when(varProvider.get("CFG_HAS_DOLLAR_VALUE"))