Bukkit Support

[jonesdevelopment]

Resolves #179

• Implements support for Bukkit 1.7-1.21
• Changes the way players are "hijacked":
  • Instead of replacing existing pipelines, Sonar now removes every pipeline and adds the necessary ones, therefore saving RAM and CPU usage by minimizing the handlers in the channel.

Tested versions

• [x] PandaSpigot 1.8.8
• [x] Paper 1.12.2
• [x] Paper 1.16.5
• [x] Paper 1.18.2
• [x] Paper 1.19.4
• [x] Paper 1.20.2
• [x] Paper 1.20.4
• [x] Paper 1.20.6
• [x] Paper 1.21
• [x] Folia 1.20.6

TODO

1. [x] Fix PE/ViaVersion/... incompatibilites:
   • Fix injection issue where childHandler is not found
   • Fix random disconnects on <1.16.5 (?)

Netty versions lower than 4.1.x are unsupported — if your server depends on one of those old Netty versions (mainly 1.7.10 and 1.8.8 servers have this problem), please consider using an updated fork such as PandaSpigot (or similar).

[jonesdevelopment]

ViaFabric (AUTO), 1.21

Dionysus 1.12.2

I know it's not working yet. It's still WIP.

[jonesdevelopment]

What about the rest of it?

I've only tested it on Paper 1.12.2 with a 1.12.2 vanilla client. Do you think it's better to migrate to packetevents or ProtocolLib to avoid all these upcoming issues?

[jonesdevelopment]

I don't think it's a good idea, because you've got everything working as it is.

Really? I didn't know that. That's good. I'll fix the ViaVersion issue first, and then I'll try to fix the rest. You can expect this version in a week or two. It's nowhere near finished.

[jonesdevelopment]

I enter the wrong code and get kicked and error

Interesting. Seems like I broke the exception handling. Good to know. I'll add another handler at the end of the pipeline to make sure any exceptions are just ignored and the channel is closed.

[jonesdevelopment]

it's me misha

I know — you're that guy who spammed my phone (and webhook) with >100 project starred notifications

[jonesdevelopment]

[02:31:48 ERROR]: [Sonar] An error has occurred while launching Sonar: java.lang.ExceptionInInitializerError io.papermc.paper.plugin.manager.PaperPluginInstanceManager.enablePlugin(PaperPluginInstanceManager.java:202) [02:31:48 WARN]: at xyz.jonesdev.sonar.bukkit.fallback.FallbackBukkitInjector.(FallbackBukkitInjector.java:55) [02:31:48 WARN]: ... 15 more

Does this still happen with the latest commit? If so, it's likely some injection error.

[jonesdevelopment]

jdk 21?

Everything above JDK11 should work out-of-the-box.

Seems like there are some issues with the injection on 1.20.5+. Do you know if there any other versions where Sonar fails to inject?

[jonesdevelopment]

[16:42:17 WARN]: java.lang.ArrayIndexOutOfBoundsException: Index 3 out of bounds for length 3
[16:42:17 WARN]:        at Sonar-Bukkit.jar//xyz.jonesdev.sonar.bukkit.fallback.FallbackBukkitInjector.<clinit>(FallbackBukkitInjector.java:61)

Also happening on 1.20.5/1.20.6

[jonesdevelopment]

Does not happen on 1.20.4. Can confirm this injection bug on >=1.20.5.

[jonesdevelopment]

@F3F5 1.20.5+ injection fixed in https://github.com/jonesdevelopment/sonar/pull/347/commits/ba6d1a1a185c1bc15d71bded1821fed2da831fc3

[jonesdevelopment]

Seems to work fine even with some server forks. Tested 1.16.5 client on a 1.16.5 Purpur (fork) server.

[jonesdevelopment]

1.20.4 ViaVersion on 1.8.8 PandaSpigot

[01:25:39 INFO]: UUID of player rena is ab9f098d-07e5-43b7-b96c-501754673e02
[01:25:39 INFO]: rena[/[0:0:0:0:0:0:0:1]:55300] logged in with entity id 2 at ([practice-lobby]0, 66.0, 0)
[01:25:39 WARN]: [ViaVersion] ERROR IN Protocol1_20_3To1_20_2 IN REMAP OF DISGUISED_CHAT (0x1C)
[01:25:39 WARN]: [ViaVersion] ERROR IN Protocol1_20_3To1_20_2 IN REMAP OF TAB_COMPLETE (0x10)
[01:25:39 INFO]: Loaded rena's data in 5 ms.
[01:25:39 INFO]: rena lost connection: Disconnected
[01:25:39 INFO]: rena left the game.

[jonesdevelopment]

1.20.2

[jonesdevelopment]

An exceptionCaught() event was fired, and it reached at the tail of the pipeline. It usually means the last handler in the pipeline did not handle the exception.

This is fixed as of https://github.com/jonesdevelopment/sonar/pull/347/commits/ce25824f0cebe38991227e1a75bc939bb75de6b4

[jonesdevelopment]

[08:18:58 WARN]: [ViaVersion] ERROR IN Protocol1_20_2To1_20_3 IN REMAP OF COMMAND_SUGGESTIONS (0x10) [08:18:58 WARN]: [ViaVersion] ERROR IN Protocol1_20_3To1_20_5 IN REMAP OF UPDATE_MOB_EFFECT (0x72)

PacketEvents also fails to inject when rejoining after the verification. I'm going to continue experimenting a bit with the packet handler, however, I honestly don't know what could cause this. I'll also play with the way packets are read in general.

[jonesdevelopment]

The only issue remaining is the ViaVersion incompatibility. If anyone has any ideas, please comment them on this PR. I'll try to solve this within the next few days. I already have an idea, but I haven't been able to test it yet.

[jonesdevelopment]

@FallenCrystal Could you please check if Geyser players still have trouble joining?

[FallenCrystal]

@FallenCrystal Could you please check if Geyser players still have trouble joining?

It's working fine for me now.

[jonesdevelopment]

what about this

Have you tried it again with the latest commit?

[jonesdevelopment]

will there be any attempt to fix via today?

yes

[jonesdevelopment]

@FallenCrystal Could you please try Bedrock again with the latest commit?

@F3F5 Could you please test it on your server again?

[jonesdevelopment]

Issue with PacketEvents:

[18:22:18 ERROR]: [Sonar] A reflective operation resulted in error:
[18:22:18 WARN]: java.lang.NoSuchFieldException: childHandler
[18:22:18 WARN]:        at java.base/java.lang.Class.getDeclaredField(Class.java:2792)
[18:22:18 WARN]:        at xyz.jonesdev.sonar.bukkit.fallback.FallbackBukkitInjector.inject(FallbackBukkitInjector.java:169)
[18:22:18 WARN]:        at org.bukkit.craftbukkit.v1_12_R1.scheduler.CraftTask.run(CraftTask.java:64)
[18:22:18 WARN]:        at org.bukkit.craftbukkit.v1_12_R1.scheduler.CraftScheduler.mainThreadHeartbeat(CraftScheduler.java:423)
[18:22:18 WARN]:        at net.minecraft.server.v1_12_R1.MinecraftServer.D(MinecraftServer.java:840)
[18:22:18 WARN]:        at net.minecraft.server.v1_12_R1.DedicatedServer.D(DedicatedServer.java:423)
[18:22:18 WARN]:        at net.minecraft.server.v1_12_R1.MinecraftServer.C(MinecraftServer.java:774)
[18:22:18 WARN]:        at net.minecraft.server.v1_12_R1.MinecraftServer.run(MinecraftServer.java:666)
[18:22:18 WARN]:        at java.base/java.lang.Thread.run(Thread.java:1570)

[jonesdevelopment]

latest devbuild sonar

I'm aware of this issue. I'll try addressing it tomorrow.

[jonesdevelopment]

@F3F5 I'm not getting the maximum IP online kick on my server. Could you test the latest commit of this branch on your server?

[FallenCrystal]

lgtm

[jonesdevelopment]

lgtm

@F3F5 Reported memory issues with the latest commit: https://pastebin.com/ymYBDDCy

[jonesdevelopment]

@F3F5 Reported memory issues with the latest commit: https://pastebin.com/ymYBDDCy

I think this might be related to https://github.com/jonesdevelopment/sonar/pull/358/files#diff-d26d2b645dcfc01fdc8fd82bc5374b72b6e0282382247fd69a1130596771351fL125-L126

[FallenCrystal]

I think this might be related to https://github.com/jonesdevelopment/sonar/pull/358/files#diff-d26d2b645dcfc01fdc8fd82bc5374b72b6e0282382247fd69a1130596771351fL125-L126

It looks like yes. But that didn't happen to me.

[FallenCrystal]

I think it should only be release when the player is connected to the fallback.

[jonesdevelopment]

handleLogin(ctx.channel(), ctx, () -> {
          byteBuf.readerIndex(originalReaderIndex);
          ctx.fireChannelRead(byteBuf);
          byteBuf.release();
          //...
        }, loginStart.getUsername(), socketAddress);

Like this?

[FallenCrystal]

No. If the player has been verified and joined to the server. Release it will cause io.netty.util.IllegalReferenceCountException. If the player needs to verify and connected to the fallback. The ByteBuf should be released.

[jonesdevelopment]

No. If the player has been verified and joined to the server. Release it will cause io.netty.util.IllegalReferenceCountException. If the player needs to verify and connected to the fallback. The ByteBuf should be released.

Can't we just always release and then retain? Otherwise, we'd have to modify the InboundHandlerAdapter to some extent.

[jonesdevelopment]

E.g.

byteBuf.release();
handleLogin(ctx.channel(), ctx, () -> {
          byteBuf.readerIndex(originalReaderIndex);
          ctx.fireChannelRead(byteBuf.retain());
          final ChannelHandler inboundHandler = ctx.channel().pipeline().remove(FALLBACK_INBOUND_HANDLER);
          if (inboundHandler != null) {
            channelRemovalListener.accept(ctx.pipeline(), FALLBACK_INBOUND_HANDLER, inboundHandler);
          }
        }, loginStart.getUsername(), socketAddress);

[FallenCrystal]

E.g.

Putting the position of ByteBuf#release of that snippet to the underside of invoke handleLogin worked fine for me.

[jonesdevelopment]

Putting the position of ByteBuf#release of that snippet to the underside of invoke handleLogin worked fine for me.

With retaining or without?

[FallenCrystal]

With retaining or without?

retaining