Repository navigation
fix(auth): late-bind FastLoginPlus injector during pre-login session verification - #6
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 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); |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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 👍 / 👎.
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 showedDisconnected. v1.0.9 was supposed to associate ProtocolLib TemporaryPlayer with a real NettyChannelInjector but still gated that association on the nativepacket_handlerbeing present during LOGIN_START.Fix
getTemporaryInjector(User, temporary)now associates any active Netty Channel with the pre-login temporary facade even whenpacket_handlerhas 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 nativenet.minecraft.network.Connectionto become available beforesetEncryptionKey(SecretKey).Tests
Userproxy: verifies the temporary facade binds before a native Connection exists, retains the same injector when it appears, and can refresh its native connection.