Skip to content

fix(auth): late-bind FastLoginPlus injector during pre-login session verification - #6

Merged
Namnarak merged 1 commit into
mainfrom
fix/late-prelogin-injector-race-20261013
Oct 12, 2026
Merged

Namnarak merged 1 commit into
mainfrom
fix/late-prelogin-injector-race-20261013

Conversation

@Namnarak

Copy link
Copy Markdown
Contributor

Production reproduction

CyoriaSMP Leaf 1.21.11 / ProtocolLib2PacketEvents v1.0.9 still rejects actual Java Premium logins. At 2026-10-13 01:21:34 +07, FastLoginPlus logged Player NamKrub disconnected during session verification, aborting, and at 01:22:02 a login timed out. Client showed Disconnected. v1.0.9 was supposed to associate ProtocolLib TemporaryPlayer with a real NettyChannelInjector but still gated that association on the native packet_handler being present during LOGIN_START.

Fix

  • getTemporaryInjector(User, temporary) now associates any active Netty Channel with the pre-login temporary facade even when packet_handler has not yet been installed. An inactive/non-Netty channel still cannot bind.
  • TemporaryPlayerFactory.getInjectorFromPlayer() re-resolves and refreshes the User/Channel mapping on each FastLoginPlus lookup, allowing a late native net.minecraft.network.Connection to become available before setEncryptionKey(SecretKey).
  • Never invent a native Connection, never bypass Mojang Session verification, and preserve the earlier v1.0.7–1.0.9 transport/encryption fixes.

Tests

  • Local Maven: 30 test executions, 0 failures/errors, 1 existing skipped.
  • New regression test constructs an actual embedded Netty channel with a PacketEvents User proxy: verifies the temporary facade binds before a native Connection exists, retains the same injector when it appears, and can refresh its native connection.
  • Shipping v1.0.10 pre-release pending real successful Java Premium login; CI and controlled restart required.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-12T18:30:31.970658Z b34d7ae PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@Namnarak
Namnarak merged commit 7727b5a into main Oct 12, 2026
1 check passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b34d7aef50

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

if (user != null) {
org.bukkit.plugin.Plugin plugin = com.comphenix.protocol.ProtocolLibrary.getPlugin();
if (plugin instanceof com.comphenix.protocol.ProtocolLib bridge) {
Injector resolved = bridge.getTemporaryInjector(user, player);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid rebinding a promoted injector to the temporary player

If a delayed asynchronous callback invokes this accessor after onPlayerJoin has promoted the channel injector to the real Bukkit player, bridge.getTemporaryInjector(...) reaches NetworkManagerInjector.getTemporaryInjector() and unconditionally calls netty.setPlayer(temporary). The injector cached under the real player's UUID is then left pointing at the temporary facade because InjectionFactory.fromPlayer() returns UUID hits without restoring the player, causing subsequent post-login packet events and player-dependent lookups to use the wrong identity. Only retry the bridge binding while the temporary player has no injector; the existing injector can be refreshed directly.

Useful? React with 👍 / 👎.

if (plugin instanceof com.comphenix.protocol.ProtocolLib bridge) {
Injector resolved = bridge.getTemporaryInjector(user, player);
if (resolved != null) {
if (resolved != injector) temporary.setInjector(resolved);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Make concurrent late binding idempotent

When the initial TemporaryPlayerAdapter.create() call cannot bind because the user's channel is not active yet, two asynchronous login callbacks can both read a null injector, resolve the same newly available channel, and execute this line. TemporaryPlayer.setInjector() rejects any second assignment and is not synchronized, so depending on interleaving one callback can receive an uncaught IllegalStateException and abort authentication. The late-binding operation should atomically install an injector or accept an identical injector that another caller has already installed.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant