Fix incompatibility by skipping offine encrypt instead of kick

[Lumine1909]

Summary of your change

Do not kick player when processing encryption on offline players, so the later netty handler can work correctly.

Motivation

I'm currently maintaining a plugin that enable encryption on offline players, and it is incompatible with FastLogin on both online and offline players. I added a fix in my plugin that resolved online player encrypted twice. But currently FastLogin kick the offline player due to exception when processing encryption related packet, since their verifyToken is a empty array.

I added a check for offline player when doing encryption, so that my plugin can process the encrypt response packet later : )

Related pr

Lumine1909/OfflineEncryptor#6

Summary by CodeRabbit

• Bug Fixes
  • Improved login handling for offline players by skipping unnecessary encryption-token verification.
  • Preserved existing behavior for invalid login sessions, including session rejection.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: edde2ce0-25cc-4258-acdd-8444b15513a5

📥 Commits

Reviewing files that changed from the base of the PR and between 9486138 and 3c22946.

📒 Files selected for processing (1)

• bukkit/src/main/java/com/github/games647/fastlogin/bukkit/listener/protocollib/ProtocolLibListener.java

📜 Recent review details

🔇 Additional comments (1)

bukkit/src/main/java/com/github/games647/fastlogin/bukkit/listener/protocollib/ProtocolLibListener.java (1)

181-193: LGTM!

---

📝 Walkthrough

Walkthrough

onEncryptionBegin now skips nonce verification when a Bukkit login session has an empty verify token, while preserving the existing verification flow for non-empty tokens and invalid-session kick behavior.

Estimated code review effort: 2 (Simple) | ~5 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly reflects the main change: skipping offline encryption handling to avoid kicking players.
Description check	✅ Passed	The description covers the change, motivation, and related work, but it omits the template's requested related issue.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[TuxCoding]

Hi @Lumine1909 Thanks for the suggestion. It would need a little bit more documentation to be included in this project. First of all what does the client if it is a offline player which we requested encryption from and second does your plugin make any modifications of FastLogin state? If the latter is the case, this behavior should be documented or better we might need to add an event, so it's much more future proof.

[Lumine1909]

Hi @Lumine1909 Thanks for the suggestion. It would need a little bit more documentation to be included in this project. First of all what does the client if it is a offline player which we requested encryption from and second does your plugin make any modifications of FastLogin state? If the latter is the case, this behavior should be documented or better we might need to add an event, so it's much more future proof.

Thank you for the reply! For your question:

1. FL does not process offline player and pass its C2SHello to the following handler. Then OE will intercept this packet and start encryption protocol. And when C2SKey arrived, currently FL will throw an exception since that player is not registered as a online user. Since OE handles everything afterward, FL can pass it through instead of throw, as what I modified in this pr.

2. OE doesn't modify any states of FL on bukkit, since its handler is down the pipeline, it only check if FL has finished the online process, it works well rn but is fragile if FL changes its relevant logic in the future. However, for Velocity, since FL implement the feature by listen to VC's event that triggers later than netty handler, the approach in OE is quite hacky. But it also doesn't touch FL's internal state.