Handle cookie responses inside Limbo (Player#requestCookie support)#257
Open
SPYmesu wants to merge 4 commits into
Open
Handle cookie responses inside Limbo (Player#requestCookie support)#257SPYmesu wants to merge 4 commits into
SPYmesu wants to merge 4 commits into
Conversation
Player#requestCookie never completed and CookieReceiveEvent never fired while a player was inside a Limbo: LimboSessionHandlerImpl did not handle ServerboundCookieResponsePacket, so it fell through to handleGeneric and the response was dropped. Override handle(ServerboundCookieResponsePacket) to mirror Velocity's ClientPlaySessionHandler: fire CookieReceiveEvent and, when the result is allowed and a backend connection exists, forward the (possibly rewritten) response to that backend. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Drain the data-generator subprocess output (consumeProcessOutput) before waitFor(); otherwise it deadlocks once the child fills the stdout pipe buffer (reproducible on 1.18+ via the bundler's per-library extraction logging; a thread dump shows main stuck in FileOutputStream.writeBytes). - Run each version's generator on a JDK that satisfies its manifest javaVersion.majorVersion (e.g. 26.1 requires Java 25), selecting an installed JDK by scanning sibling JDK dirs; fall back to the current JDK. - Bump the Gradle wrapper to 9.5.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
Author
|
No, it wasn't auto-opened, claude helps me to make PR and its text because im not good enough in technical english. Thanks for the references. I'll rework it to stage the ServerboundCookieResponsePackets the same way Limbo already buffers ClientSettings/brand, carry them forward in the handler, expose a getter and replay them in LoginTasksQueue when connecting to the backend, instead of firing CookieReceiveEvent directly in the Limbo handler. |
A player in a Limbo usually isn't connected to a backend yet, so the cookie response can't be handled right there. Store the packets like we already do for client settings and brand, then replay them in LoginTasksQueue when the player is sent to a server, so the cookie event fires at the right time.
Lets a Limbo handler read the cookie value while the player is still in the Limbo, not only later through CookieReceiveEvent. handle() now also calls onCookieResponse with the key and data when the client replies.
Author
|
So I need this because I check for cookies while player is on auth server (fake server), this helps to enable trusted devices function of security plugin. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
While a player is inside a Limbo, the Velocity cookie flow never completes:
Player#requestCookie(...)never resolves andCookieReceiveEventis never fired.LimboSessionHandlerImpldoesn't handleServerboundCookieResponsePacket, so the packet falls through tohandleGenericand the response is dropped.This PR makes cookies work inside a Limbo. It also bundles the build changes that were required to compile the plugin and run data generation on a clean checkout (data generation deadlocked, and the newest version in the matrix — 26.1 — needs a newer JDK than the rest), plus a Gradle wrapper bump.
Changes
1. Cookie support inside Limbo (main change)
LimboSessionHandlerImplnow overrideshandle(ServerboundCookieResponsePacket), mirroring Velocity'sClientPlaySessionHandler:CookieReceiveEventfor the player;trueso the packet is consumed instead of falling through tohandleGeneric.The cookie response already decodes correctly inside a Limbo (the Limbo registry overlays the PLAY registry); the only missing piece was the handler override, without which
CookieReceiveEventnever fired and the request silently never completed. A player in a Limbo is usually not attached to a backend, so the forward is guarded by agetConnectedServer() != nullcheck — exactly like the default play handler.2. Fix data-generator hang (stdout pipe deadlock) —
plugin/build.gradlegenerateDatalaunched the generator and waited viacommandLine.execute(...).waitFor()without consuming itsstdout/stderr. Once the child fills the OS pipe buffer it blocks onwrite()forever and Gradle blocks onwaitFor()— a classic deadlock, reproducible on 1.18+ where the bundler prints one line per extracted library (a thread dump showsmainstuck inFileOutputStream.writeBytesfromnet.minecraft.bundler.Main). Fixed by draining the streams withconsumeProcessOutput()beforewaitFor().3. Per-version JDK for data generation —
plugin/build.gradleThe generator always ran on the JDK running Gradle. Newer versions need a newer runtime — 26.1 requires Java 25 (
javaVersion.majorVersionin its manifest) — so running it under an older JDK throwsUnsupportedClassVersionError, produces no reports, and fails the build later. AddedselectJavaHome(requiredMajor): it reads each manifest'sjavaVersion.majorVersionand, when the current JDK is too old, picks an installed JDK that satisfies it by scanning sibling JDK directories'releasefiles; otherwise it keeps the current JDK. This lets the build run on one base JDK while still generating data for versions that need a newer one.4. Bump Gradle wrapper to 9.5.0
Updates the wrapper from 9.4.1 to 9.5.0 (the version the build was verified against).
Testing
./gradlew buildcompletes withBUILD SUCCESSFUL.> Using JDK 25 … for data generation).plugin/build/libs/limboapi-<version>-SNAPSHOT.jar.Notes
release) is a pragmatic heuristic; I can switch it to Gradle Java toolchains (javaToolchains.launcherFor { ... }) if preferred.