From c377b67b59346a08bcd80cb8cdbef996885b926d Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 22 Jul 2026 13:23:22 +0200 Subject: [PATCH 1/3] fix(core): resolve Bedrock identity graph config lazily to avoid startup crash The 0.12.0 Bedrock identity work (#57) wired BedrockIdentityEnforcer and BedrockIdentityKeyProvider with a direct dependency on ConnectConfig. Those components are resolved by the platform injector while the platform is constructed, i.e. before ConnectPlatform.init() loads the config. ConnectConfig is only bound in the child injector that init() creates via ConfigLoadedModule, so resolving the Bedrock graph from the parent injector forced Guice to create a just-in-time ConnectConfig binding on the parent, which then collided with the child binding: [Guice/JitBindingAlreadySet]: A just-in-time binding to ConnectConfig was already configured on a parent injector at ConfigLoadedModule.config(...) The plugin crashed in SpigotPlugin.onLoad() before token initialization, so plugins/Connect/token.json was never created. Fix: the injected Bedrock components read ConnectConfig lazily through the parent-bound ConfigHolder (populated by init() before the config is used), so no ConnectConfig binding is ever established on the parent injector. The existing (ConnectConfig, ...) constructors are preserved for tests and direct use, and token precedence (CONNECT_TOKEN, existing token.json, else generate + persist) is unchanged. Verified end-to-end on Paper 26.2 (build 60) + Java 25 with a single plugin jar: - unfixed: JitBindingAlreadySet in onLoad, no token.json (reproduces the report with exactly one jar on a clean data dir, so the duplicate-jar theory is false) - fixed: reaches enabled state and creates token.json; restart reuses the token; an explicit CONNECT_TOKEN does not overwrite token.json Regression guard: core BedrockParentInjectorStartupTest. --- AGENTS.md | 20 +++++ .../bedrock/BedrockIdentityEnforcer.java | 35 +++++++-- .../bedrock/BedrockIdentityKeyProvider.java | 49 +++++++++--- .../minekube/connect/module/CommonModule.java | 14 ++-- .../BedrockParentInjectorStartupTest.java | 75 +++++++++++++++++++ 5 files changed, 170 insertions(+), 23 deletions(-) create mode 100644 core/src/test/java/com/minekube/connect/module/BedrockParentInjectorStartupTest.java diff --git a/AGENTS.md b/AGENTS.md index 86afc988..0a6d7343 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -36,6 +36,19 @@ gh -R minekube/connect-java release view --json tagName,targetCommitis curl -I -L --fail https://github.com/minekube/connect-java/releases/download//connect-velocity.jar ``` +## Injector Scoping (config availability) + +- `ConnectConfig` is bound only in the child injector that `ConnectPlatform.init()` + creates via `ConfigLoadedModule`, after the config file is loaded. The parent + injector (built in each platform's `onLoad`) is config-agnostic. +- Do NOT make anything reachable from the parent/platform injector inject + `ConnectConfig` directly. Guice resolves it as a just-in-time binding on the + parent, which then collides with the child binding + (`JitBindingAlreadySet`, crash before token init). Instead depend on the + parent-bound `ConfigHolder` and read `configHolder.get()` lazily (post-init). +- Regression guard: `core/.../module/BedrockParentInjectorStartupTest` + (introduced for the 0.12.0 Paper startup regression). + ## Velocity Join Bugs - For Velocity proxy issues, test both `CONFIGURATION` and `PLAY` state packet @@ -57,3 +70,10 @@ Run targeted module tests for packet/session fixes, then the broader build: Do not call production fixed from a Connect Java release alone. Confirm the released jar is in the hub image, the hub pod logs the expected plugin version, and Moxy accepts a tunnel with the same `connectorVersion`. + +## Maintaining this file + +Keep this file for knowledge useful to almost every future agent session in this project. +Do not repeat what the codebase already shows; point to the authoritative file or command instead. +Prefer rewriting or pruning existing entries over appending new ones. +When updating this file, preserve this bar for all agents and keep entries concise. diff --git a/core/src/main/java/com/minekube/connect/bedrock/BedrockIdentityEnforcer.java b/core/src/main/java/com/minekube/connect/bedrock/BedrockIdentityEnforcer.java index 35e92934..a2fff147 100644 --- a/core/src/main/java/com/minekube/connect/bedrock/BedrockIdentityEnforcer.java +++ b/core/src/main/java/com/minekube/connect/bedrock/BedrockIdentityEnforcer.java @@ -10,6 +10,7 @@ import com.minekube.connect.api.player.bedrock.BedrockIdentityReplayCache; import com.minekube.connect.api.player.bedrock.BedrockIdentityVerificationException; import com.minekube.connect.api.player.bedrock.BedrockIdentityVerifier; +import com.minekube.connect.config.ConfigHolder; import com.minekube.connect.config.ConnectConfig; import com.minekube.connect.config.ConnectConfig.BedrockIdentityConfig; import com.minekube.connect.network.netty.LocalSession; @@ -28,20 +29,26 @@ public final class BedrockIdentityEnforcer { private static final String PROTOCOL_BEDROCK = "bedrock"; private static final String REJECT_MESSAGE = "Bedrock identity verification failed"; - private final ConnectConfig config; + private final Supplier config; private final ConnectLogger logger; private final Supplier now; private final BedrockIdentityKeyProvider keyProvider; private final BedrockAdmissionCoordinator admissionCoordinator; private final BedrockIdentityReplayCache replayCache = new BedrockIdentityReplayCache(); + /** + * Injection seam used by the plugin. The config is resolved lazily from {@link ConfigHolder} so + * this enforcer can be constructed by the (config-agnostic) parent injector while the platform + * injector is built, before {@code ConnectPlatform.init()} loads the configuration. + */ @Inject public BedrockIdentityEnforcer( - ConnectConfig config, + ConfigHolder configHolder, ConnectLogger logger, BedrockIdentityKeyProvider keyProvider, BedrockAdmissionCoordinator admissionCoordinator) { - this(config, logger, Instant::now, keyProvider, admissionCoordinator); + this(Objects.requireNonNull(configHolder, "configHolder")::get, logger, Instant::now, + keyProvider, admissionCoordinator); } public BedrockIdentityEnforcer( @@ -89,6 +96,15 @@ public BedrockIdentityEnforcer( Supplier now, BedrockIdentityKeyProvider keyProvider, BedrockAdmissionCoordinator admissionCoordinator) { + this(constant(config), logger, now, keyProvider, admissionCoordinator); + } + + private BedrockIdentityEnforcer( + Supplier config, + ConnectLogger logger, + Supplier now, + BedrockIdentityKeyProvider keyProvider, + BedrockAdmissionCoordinator admissionCoordinator) { this.config = Objects.requireNonNull(config, "config"); this.logger = Objects.requireNonNull(logger, "logger"); this.now = Objects.requireNonNull(now, "now"); @@ -96,6 +112,15 @@ public BedrockIdentityEnforcer( this.admissionCoordinator = admissionCoordinator; } + private static Supplier constant(ConnectConfig config) { + Objects.requireNonNull(config, "config"); + return () -> config; + } + + private ConnectConfig config() { + return Objects.requireNonNull(config.get(), "config"); + } + BedrockIdentityEnforcer( ConnectConfig config, ConnectLogger logger, @@ -180,7 +205,7 @@ Decision verify( SessionProtocol protocol) { Objects.requireNonNull(player, "player"); Objects.requireNonNull(protocol, "protocol"); - BedrockIdentityConfig bedrockIdentity = config.getBedrockIdentity(); + BedrockIdentityConfig bedrockIdentity = config().getBedrockIdentity(); BedrockIdentityConfiguration identityConfiguration = BedrockIdentityConfiguration.from(bedrockIdentity); if (identityConfiguration.isDisabled()) { return Decision.allowed(null); @@ -280,7 +305,7 @@ private BedrockIdentityVerifier verifier( .publicKey(publicKey) .now(now) .endpointId(endpointId) - .endpointName(config.getEndpoint()) + .endpointName(config().getEndpoint()) .orgId(endpointOrgId) .sessionId(player.getSessionId()) .protocol(PROTOCOL_BEDROCK) diff --git a/core/src/main/java/com/minekube/connect/bedrock/BedrockIdentityKeyProvider.java b/core/src/main/java/com/minekube/connect/bedrock/BedrockIdentityKeyProvider.java index 86ff415a..de616a2a 100644 --- a/core/src/main/java/com/minekube/connect/bedrock/BedrockIdentityKeyProvider.java +++ b/core/src/main/java/com/minekube/connect/bedrock/BedrockIdentityKeyProvider.java @@ -2,7 +2,9 @@ import com.google.gson.Gson; import com.google.gson.JsonSyntaxException; +import com.google.inject.name.Named; import com.minekube.connect.api.player.bedrock.BedrockIdentityVerifier; +import com.minekube.connect.config.ConfigHolder; import com.minekube.connect.config.ConnectConfig; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -16,29 +18,49 @@ import java.util.Objects; import java.util.concurrent.TimeUnit; import java.util.function.Supplier; +import javax.inject.Inject; +import javax.inject.Singleton; import okhttp3.HttpUrl; import okhttp3.OkHttpClient; import okhttp3.Request; import okhttp3.Response; import okhttp3.ResponseBody; +@Singleton public final class BedrockIdentityKeyProvider { private static final Gson GSON = new Gson(); private static final long METADATA_BODY_LIMIT_BYTES = 64 * 1024; private static final long METADATA_CALL_TIMEOUT_SECONDS = 5; private static final long METADATA_FAILURE_BACKOFF_SECONDS = 5; - private final ConnectConfig config; + private final Supplier config; private final OkHttpClient httpClient; private final Supplier now; private CachedKeys cachedKeys; private Instant nextRefreshAt = Instant.MIN; + /** + * Injection seam used by the plugin. The config is resolved lazily from {@link ConfigHolder} so + * this provider can be constructed by the (config-agnostic) parent injector before + * {@code ConnectPlatform.init()} loads the configuration. + */ + @Inject + public BedrockIdentityKeyProvider( + ConfigHolder configHolder, + @Named("defaultHttpClient") OkHttpClient httpClient) { + this(Objects.requireNonNull(configHolder, "configHolder")::get, httpClient, Instant::now); + } + public BedrockIdentityKeyProvider(ConnectConfig config, OkHttpClient httpClient) { this(config, httpClient, Instant::now); } BedrockIdentityKeyProvider(ConnectConfig config, OkHttpClient httpClient, Supplier now) { + this(constant(config), httpClient, now); + } + + private BedrockIdentityKeyProvider( + Supplier config, OkHttpClient httpClient, Supplier now) { this.config = Objects.requireNonNull(config, "config"); this.httpClient = Objects.requireNonNull(httpClient, "httpClient").newBuilder() .callTimeout(METADATA_CALL_TIMEOUT_SECONDS, TimeUnit.SECONDS) @@ -48,9 +70,18 @@ public BedrockIdentityKeyProvider(ConnectConfig config, OkHttpClient httpClient) this.now = Objects.requireNonNull(now, "now"); } + private static Supplier constant(ConnectConfig config) { + Objects.requireNonNull(config, "config"); + return () -> config; + } + + private ConnectConfig config() { + return Objects.requireNonNull(config.get(), "config"); + } + synchronized List keys() { List staticKeys = staticKeys(); - String metadataUrl = config.getBedrockIdentity().getMetadataUrl(); + String metadataUrl = config().getBedrockIdentity().getMetadataUrl(); if (metadataUrl == null || metadataUrl.isEmpty()) { return staticKeys; } @@ -83,7 +114,7 @@ synchronized List keys() { synchronized boolean hasUsableKeys() { List keys = keys(); - String metadataUrl = config.getBedrockIdentity().getMetadataUrl(); + String metadataUrl = config().getBedrockIdentity().getMetadataUrl(); if (metadataUrl == null || metadataUrl.isEmpty()) { return !keys.isEmpty(); } @@ -103,7 +134,7 @@ private RemoteKeys fetchKeys(String metadataUrl) throws IOException { } VerifierMetadata metadata = GSON.fromJson(readBody(body), VerifierMetadata.class); if (metadata == null || !"Ed25519".equals(metadata.algorithm) || - !config.getBedrockIdentity().getExpectedIssuer().equals(metadata.issuer)) { + !config().getBedrockIdentity().getExpectedIssuer().equals(metadata.issuer)) { throw new IllegalArgumentException("metadata scope is invalid"); } List keys = new ArrayList<>(); @@ -178,9 +209,9 @@ private Instant failureBackoffUntil(Instant current) { private List staticKeys() { List keys = new ArrayList<>(); - addKey(keys, config.getBedrockIdentity().getPublicKey()); - if (config.getBedrockIdentity().getPublicKeys() != null) { - for (String key : config.getBedrockIdentity().getPublicKeys()) { + addKey(keys, config().getBedrockIdentity().getPublicKey()); + if (config().getBedrockIdentity().getPublicKeys() != null) { + for (String key : config().getBedrockIdentity().getPublicKeys()) { addKey(keys, key); } } @@ -213,7 +244,7 @@ private static void addMetadataKey(List keys, String encoded) { } private int metadataCacheSeconds(int remoteCacheSeconds) { - int configured = config.getBedrockIdentity().getMetadataCacheSeconds(); + int configured = config().getBedrockIdentity().getMetadataCacheSeconds(); if (configured <= 0) { return remoteCacheSeconds; } @@ -221,7 +252,7 @@ private int metadataCacheSeconds(int remoteCacheSeconds) { } private int metadataMaxStaleSeconds() { - return Math.max(0, config.getBedrockIdentity().getMetadataMaxStaleSeconds()); + return Math.max(0, config().getBedrockIdentity().getMetadataMaxStaleSeconds()); } private static final class CachedKeys { diff --git a/core/src/main/java/com/minekube/connect/module/CommonModule.java b/core/src/main/java/com/minekube/connect/module/CommonModule.java index e7849f55..95d9f689 100644 --- a/core/src/main/java/com/minekube/connect/module/CommonModule.java +++ b/core/src/main/java/com/minekube/connect/module/CommonModule.java @@ -125,20 +125,16 @@ public OkHttpClient defaultOkHttpClient() { return HttpUtils.defaultOkHttpClient(); } - @Provides - @Singleton - public BedrockIdentityKeyProvider bedrockIdentityKeyProvider( - ConnectConfig config, - @Named("defaultHttpClient") OkHttpClient httpClient) { - return new BedrockIdentityKeyProvider(config, httpClient); - } + // BedrockIdentityKeyProvider is bound just-in-time (@Inject @Singleton) so it and the enforcer + // can be resolved by the config-agnostic parent injector without pulling ConnectConfig into it. + // See BedrockParentInjectorStartupTest. @Provides @Singleton public BedrockIdentityReadiness bedrockIdentityReadiness( - ConnectConfig config, + ConfigHolder configHolder, BedrockIdentityKeyProvider keyProvider) { - return new BedrockIdentityReadiness(config, keyProvider); + return new BedrockIdentityReadiness(configHolder.get(), keyProvider); } @Provides diff --git a/core/src/test/java/com/minekube/connect/module/BedrockParentInjectorStartupTest.java b/core/src/test/java/com/minekube/connect/module/BedrockParentInjectorStartupTest.java new file mode 100644 index 00000000..4acfee75 --- /dev/null +++ b/core/src/test/java/com/minekube/connect/module/BedrockParentInjectorStartupTest.java @@ -0,0 +1,75 @@ +package com.minekube.connect.module; + +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.Mockito.mock; + +import com.google.inject.AbstractModule; +import com.google.inject.Guice; +import com.google.inject.Injector; +import com.google.inject.Key; +import com.google.inject.name.Names; +import com.minekube.connect.api.logger.ConnectLogger; +import com.minekube.connect.bedrock.BedrockIdentityEnforcer; +import com.minekube.connect.config.ConfigHolder; +import com.minekube.connect.config.ConnectConfig; +import okhttp3.OkHttpClient; +import org.junit.jupiter.api.Test; + +/** + * Regression test for the connect-java 0.12.0 Paper startup crash. + * + *

PR #57 wired the Bedrock identity graph ({@link BedrockIdentityEnforcer} and its + * {@code BedrockIdentityKeyProvider}) with a hard dependency on {@link ConnectConfig}. That graph is + * resolved by the platform injector while constructing the platform, i.e. before + * {@code ConnectPlatform.init()} loads the config. {@code ConnectConfig} is only bound in the child + * injector that {@code init()} creates via {@link ConfigLoadedModule}. Resolving the Bedrock graph + * from the parent injector therefore forced Guice to create a just-in-time {@code ConnectConfig} + * binding on the parent, which then collided with the child binding: + * + * 

+ * [Guice/JitBindingAlreadySet]: A just-in-time binding to ConnectConfig was already configured
+ * on a parent injector at ConfigLoadedModule.config(ConfigLoadedModule.java:49)
+ *
+ * + *

The connector crashed in {@code SpigotPlugin.onLoad()} before token initialization, so + * {@code plugins/Connect/token.json} was never created. The fix makes the Bedrock graph resolve + * config lazily through the parent-bound {@link ConfigHolder} so no {@code ConnectConfig} binding is + * ever established on the parent injector. + */ +class BedrockParentInjectorStartupTest { + + /** + * Mirrors the production ordering: a parent injector (with only pre-config bindings available) + * resolves the Bedrock enforcer the way {@code SpigotPlatformModule.platformInjector(...)} does, + * and only afterwards does {@code ConnectPlatform.init()} create the config-owning child injector. + */ + @Test + void resolvingBedrockGraphDoesNotBindConfigOnParentInjector() { + Injector parent = Guice.createInjector(new AbstractModule() { + @Override + protected void configure() { + // ConfigHolder is bound on the parent and populated by init() before config is read. + bind(ConfigHolder.class).toInstance(new ConfigHolder()); + bind(ConnectLogger.class).toInstance(mock(ConnectLogger.class)); + bind(OkHttpClient.class) + .annotatedWith(Names.named("defaultHttpClient")) + .toInstance(new OkHttpClient()); + } + }); + + // The platform injector resolves the Bedrock enforcer before config load. + parent.getInstance(BedrockIdentityEnforcer.class); + + // Regression guard: resolving the Bedrock graph must NOT create a ConnectConfig binding on + // the parent, otherwise the child ConfigLoadedModule below fails with JitBindingAlreadySet. + assertNull( + parent.getExistingBinding(Key.get(ConnectConfig.class)), + "resolving the Bedrock graph must not establish a ConnectConfig binding on the parent injector"); + + // Reproduces ConnectPlatform.init(): the loaded config is bound in a child injector. + ConnectConfig loaded = new ConnectConfig(); + Injector child = parent.createChildInjector(new ConfigLoadedModule(loaded)); + assertSame(loaded, child.getInstance(ConnectConfig.class)); + } +} From 598b76b4ea29b62431b7ead63e966a82eefb7aa1 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 22 Jul 2026 13:40:06 +0200 Subject: [PATCH 2/3] no-mistakes(review): Restore legacy Bedrock enforcer constructor compatibility --- .../connect/bedrock/BedrockIdentityEnforcer.java | 8 ++++++++ .../bedrock/BedrockIdentityEnforcerTest.java | 15 +++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/core/src/main/java/com/minekube/connect/bedrock/BedrockIdentityEnforcer.java b/core/src/main/java/com/minekube/connect/bedrock/BedrockIdentityEnforcer.java index a2fff147..354b71b8 100644 --- a/core/src/main/java/com/minekube/connect/bedrock/BedrockIdentityEnforcer.java +++ b/core/src/main/java/com/minekube/connect/bedrock/BedrockIdentityEnforcer.java @@ -51,6 +51,14 @@ public BedrockIdentityEnforcer( keyProvider, admissionCoordinator); } + public BedrockIdentityEnforcer( + ConnectConfig config, + ConnectLogger logger, + BedrockIdentityKeyProvider keyProvider, + BedrockAdmissionCoordinator admissionCoordinator) { + this(config, logger, Instant::now, keyProvider, admissionCoordinator); + } + public BedrockIdentityEnforcer( ConnectConfig config, ConnectLogger logger, diff --git a/core/src/test/java/com/minekube/connect/bedrock/BedrockIdentityEnforcerTest.java b/core/src/test/java/com/minekube/connect/bedrock/BedrockIdentityEnforcerTest.java index ce0ab97b..eeb1fb9e 100644 --- a/core/src/test/java/com/minekube/connect/bedrock/BedrockIdentityEnforcerTest.java +++ b/core/src/test/java/com/minekube/connect/bedrock/BedrockIdentityEnforcerTest.java @@ -78,6 +78,21 @@ void retainsLegacyHttpClientConstructor() throws Exception { new ConnectConfig(), mock(ConnectLogger.class), new OkHttpClient())); } + @Test + void retainsLegacyConfigConstructor() throws Exception { + Constructor constructor = BedrockIdentityEnforcer.class.getConstructor( + ConnectConfig.class, + ConnectLogger.class, + BedrockIdentityKeyProvider.class, + BedrockAdmissionCoordinator.class); + + assertNotNull(constructor.newInstance( + new ConnectConfig(), + mock(ConnectLogger.class), + new BedrockIdentityKeyProvider(new ConnectConfig(), new OkHttpClient()), + null)); + } + @Test void legacyConstructorUsesContextAdmissionRegistry() throws Exception { KeyPair keyPair = ed25519KeyPair(); From f2539ab5a438ec4f4823a5301b69981ca36331fa Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 22 Jul 2026 13:57:30 +0200 Subject: [PATCH 3/3] no-mistakes(document): Consolidated injector guidance; compilation and diff checks pass --- AGENTS.md | 17 ++++++-------- .../BedrockParentInjectorStartupTest.java | 22 ++++--------------- 2 files changed, 11 insertions(+), 28 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 0a6d7343..c4cfef9d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,16 +38,13 @@ curl -I -L --fail https://github.com/minekube/connect-java/releases/download/PR #57 wired the Bedrock identity graph ({@link BedrockIdentityEnforcer} and its - * {@code BedrockIdentityKeyProvider}) with a hard dependency on {@link ConnectConfig}. That graph is - * resolved by the platform injector while constructing the platform, i.e. before - * {@code ConnectPlatform.init()} loads the config. {@code ConnectConfig} is only bound in the child - * injector that {@code init()} creates via {@link ConfigLoadedModule}. Resolving the Bedrock graph - * from the parent injector therefore forced Guice to create a just-in-time {@code ConnectConfig} - * binding on the parent, which then collided with the child binding: - * - * 

- * [Guice/JitBindingAlreadySet]: A just-in-time binding to ConnectConfig was already configured
- * on a parent injector at ConfigLoadedModule.config(ConfigLoadedModule.java:49)
- *
- * - *

The connector crashed in {@code SpigotPlugin.onLoad()} before token initialization, so - * {@code plugins/Connect/token.json} was never created. The fix makes the Bedrock graph resolve - * config lazily through the parent-bound {@link ConfigHolder} so no {@code ConnectConfig} binding is - * ever established on the parent injector. + *

The production parent injector builds the platform before {@code ConnectPlatform.init()} + * creates the config child. The graph must resolve config lazily through the parent-bound + * {@link ConfigHolder}, so the loaded {@link ConnectConfig} can be bound in the child afterward. */ class BedrockParentInjectorStartupTest {