Skip to content

Commit b54dd49

Browse files
Replace CGLib with ByteBuddy (#984)
- The gclib dependency in the EnchancerFactory has been removed. All classes that used the actual factory part of it have been updated to use bytebuddy instead. This class will have to be removed at some point, but at the moment it is still used for accessing its class loader. - Renamed EnhancerFactory to ByteBuddyFactory. All ByteBuddy actions should go through this now. Every subclass created here implements the ByteBuddyGenerated interface. This makes it possible to recognize classes generated using ByteBuddy (by default, it doesn't leave such a trace). - Removed the method DefaultInstances#forEnhancer(Enhancer). This method isn't used anywhere; the last trace of usage of the method I could find was in 2013 (in the NetworkServerInjector). External plugins (I couldn't find any that used it), they should really have their own implementation, given that they already require an instance of an Enchancer. As such, I feel it is safe to remove rather than update it.
1 parent 26274fe commit b54dd49

19 files changed

+726
-442
lines changed

pom.xml

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,8 @@
5151

5252
<relocations>
5353
<relocation>
54-
<pattern>net.sf</pattern>
55-
<shadedPattern>com.comphenix.net.sf</shadedPattern>
54+
<pattern>net.bytebuddy</pattern>
55+
<shadedPattern>com.comphenix.net.bytebuddy</shadedPattern>
5656
</relocation>
5757
</relocations>
5858

@@ -100,6 +100,8 @@
100100
<artifactId>maven-surefire-plugin</artifactId>
101101
<version>3.0.0-M5</version>
102102
<configuration>
103+
<trimStackTrace>false</trimStackTrace>
104+
<useFile>false</useFile>
103105
<systemProperties>
104106
<property>
105107
<name>projectVersion</name>
@@ -291,11 +293,11 @@
291293
<version>${spigot.version}</version>
292294
<scope>provided</scope>
293295
</dependency>
296+
<!-- https://mvnrepository.com/artifact/net.bytebuddy/byte-buddy -->
294297
<dependency>
295-
<groupId>cglib</groupId>
296-
<artifactId>cglib-nodep</artifactId>
297-
<version>3.2.5</version>
298-
<scope>compile</scope>
298+
<groupId>net.bytebuddy</groupId>
299+
<artifactId>byte-buddy</artifactId>
300+
<version>1.10.16</version>
299301
</dependency>
300302

301303
<!-- Testing dependencies -->

src/main/java/com/comphenix/protocol/CommandPacket.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,12 @@
2929
import java.util.logging.Level;
3030
import java.util.logging.Logger;
3131

32-
import net.sf.cglib.proxy.Factory;
33-
3432
import org.bukkit.ChatColor;
3533
import org.bukkit.command.CommandSender;
3634
import org.bukkit.entity.Player;
3735
import org.bukkit.plugin.Plugin;
3836

37+
import com.comphenix.protocol.utility.ByteBuddyGenerated;
3938
import com.comphenix.protocol.PacketType.Sender;
4039
import com.comphenix.protocol.concurrency.PacketTypeSet;
4140
import com.comphenix.protocol.error.ErrorReporter;
@@ -464,7 +463,7 @@ public String getPacketDescription(PacketContainer packetContainer) throws Illeg
464463
// Get the first Minecraft super class
465464
while (clazz != null && clazz != Object.class &&
466465
(!MinecraftReflection.isMinecraftClass(clazz) ||
467-
Factory.class.isAssignableFrom(clazz))) {
466+
ByteBuddyGenerated.class.isAssignableFrom(clazz))) {
468467
clazz = clazz.getSuperclass();
469468
}
470469

src/main/java/com/comphenix/protocol/ProtocolLib.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
import com.comphenix.protocol.updater.Updater;
3838
import com.comphenix.protocol.updater.Updater.UpdateType;
3939
import com.comphenix.protocol.utility.ChatExtensions;
40-
import com.comphenix.protocol.utility.EnhancerFactory;
40+
import com.comphenix.protocol.utility.ByteBuddyFactory;
4141
import com.comphenix.protocol.utility.MinecraftVersion;
4242
import com.google.common.base.Splitter;
4343
import com.google.common.collect.Iterables;
@@ -144,7 +144,7 @@ public void onLoad() {
144144
ProtocolLogger.init(this);
145145

146146
// Initialize enhancer factory
147-
EnhancerFactory.getInstance().setClassLoader(getClassLoader());
147+
ByteBuddyFactory.getInstance().setClassLoader(getClassLoader());
148148

149149
// Add global parameters
150150
DetailedErrorReporter detailedReporter = new DetailedErrorReporter(this);

src/main/java/com/comphenix/protocol/events/SerializedOfflinePlayer.java

Lines changed: 86 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,32 @@
2121
import java.io.ObjectInputStream;
2222
import java.io.ObjectOutputStream;
2323
import java.io.Serializable;
24+
import java.lang.reflect.Constructor;
25+
import java.lang.reflect.InvocationTargetException;
2426
import java.lang.reflect.Method;
2527
import java.util.Map;
2628
import java.util.UUID;
27-
import java.util.concurrent.ConcurrentHashMap;
28-
29-
import net.sf.cglib.proxy.Enhancer;
30-
import net.sf.cglib.proxy.MethodInterceptor;
31-
import net.sf.cglib.proxy.MethodProxy;
29+
import java.util.function.Function;
30+
31+
import net.bytebuddy.description.ByteCodeElement;
32+
import net.bytebuddy.description.modifier.Visibility;
33+
import net.bytebuddy.dynamic.loading.ClassLoadingStrategy;
34+
import net.bytebuddy.dynamic.scaffold.subclass.ConstructorStrategy;
35+
import net.bytebuddy.implementation.FieldAccessor;
36+
import net.bytebuddy.implementation.InvocationHandlerAdapter;
37+
import net.bytebuddy.implementation.MethodCall;
38+
import net.bytebuddy.implementation.MethodDelegation;
39+
import net.bytebuddy.implementation.bind.annotation.FieldValue;
40+
import net.bytebuddy.implementation.bind.annotation.Pipe;
41+
import net.bytebuddy.implementation.bind.annotation.RuntimeType;
42+
import net.bytebuddy.matcher.ElementMatcher;
43+
import net.bytebuddy.matcher.ElementMatchers;
3244

3345
import org.bukkit.*;
3446
import org.bukkit.entity.EntityType;
3547
import org.bukkit.entity.Player;
3648

37-
import com.comphenix.protocol.utility.EnhancerFactory;
49+
import com.comphenix.protocol.utility.ByteBuddyFactory;
3850

3951
/**
4052
* Represents a player object that can be serialized by Java.
@@ -60,9 +72,8 @@ class SerializedOfflinePlayer implements OfflinePlayer, Serializable {
6072
private boolean playedBefore;
6173
private boolean online;
6274
private boolean whitelisted;
63-
64-
// Proxy helper
65-
private static Map<String, Method> lookup = new ConcurrentHashMap<String, Method>();
75+
76+
private static final Constructor<?> proxyPlayerConstructor = setupProxyPlayerConstructor();
6677

6778
/**
6879
* Constructor used by serialization.
@@ -243,42 +254,78 @@ public Player getPlayer() {
243254
}
244255

245256
/**
246-
* Retrieve a player object that implements OfflinePlayer by refering to this object.
257+
* Retrieve a player object that implements OfflinePlayer by referring to this object.
247258
* <p>
248259
* All other methods cause an exception.
249260
* @return Proxy object.
250261
*/
251262
public Player getProxyPlayer() {
252-
253-
// Remember to initialize the method filter
254-
if (lookup.size() == 0) {
255-
// Add all public methods
256-
for (Method method : OfflinePlayer.class.getMethods()) {
257-
lookup.put(method.getName(), method);
258-
}
263+
try {
264+
return (Player) proxyPlayerConstructor.newInstance(this);
265+
} catch (IllegalAccessException e) {
266+
throw new RuntimeException("Cannot access reflection.", e);
267+
} catch (InstantiationException e) {
268+
throw new RuntimeException("Cannot instantiate object.", e);
269+
} catch (InvocationTargetException e) {
270+
throw new RuntimeException("Error in invocation.", e);
259271
}
260-
261-
// MORE CGLIB magic!
262-
Enhancer ex = EnhancerFactory.getInstance().createEnhancer();
263-
ex.setSuperclass(Player.class);
264-
ex.setCallback(new MethodInterceptor() {
265-
@Override
266-
public Object intercept(Object obj, Method method, Object[] args, MethodProxy proxy) throws Throwable {
267-
268-
// There's no overloaded methods, so we don't care
269-
Method offlineMethod = lookup.get(method.getName());
270-
271-
// Ignore all other methods
272-
if (offlineMethod == null) {
272+
}
273+
274+
private static Constructor<? extends Player> setupProxyPlayerConstructor()
275+
{
276+
final Method[] offlinePlayerMethods = OfflinePlayer.class.getMethods();
277+
final String[] methodNames = new String[offlinePlayerMethods.length];
278+
for (int idx = 0; idx < offlinePlayerMethods.length; ++idx)
279+
methodNames[idx] = offlinePlayerMethods[idx].getName();
280+
281+
final ElementMatcher.Junction<ByteCodeElement> forwardedMethods = ElementMatchers.namedOneOf(methodNames);
282+
283+
try {
284+
final MethodDelegation forwarding = MethodDelegation.withDefaultConfiguration()
285+
.withBinders(Pipe.Binder.install(Function.class))
286+
.to(new Object() {
287+
@RuntimeType
288+
public Object intercept(@Pipe Function<OfflinePlayer, Object> pipe,
289+
@FieldValue("offlinePlayer") OfflinePlayer proxy) {
290+
return pipe.apply(proxy);
291+
}
292+
});
293+
294+
final InvocationHandlerAdapter throwException = InvocationHandlerAdapter.of((obj, method, args) -> {
273295
throw new UnsupportedOperationException(
274-
"The method " + method.getName() + " is not supported for offline players.");
275-
}
276-
277-
// Invoke our on method
278-
return offlineMethod.invoke(SerializedOfflinePlayer.this, args);
279-
}
280-
});
281-
282-
return (Player) ex.create();
296+
"The method " + method.getName() + " is not supported for offline players.");
297+
});
298+
299+
return ByteBuddyFactory.getInstance()
300+
.createSubclass(PlayerUnion.class, ConstructorStrategy.Default.NO_CONSTRUCTORS)
301+
.name(SerializedOfflinePlayer.class.getPackage().getName() + ".PlayerInvocationHandler")
302+
303+
.defineField("offlinePlayer", OfflinePlayer.class, Visibility.PRIVATE)
304+
.defineConstructor(Visibility.PUBLIC)
305+
.withParameters(OfflinePlayer.class)
306+
.intercept(MethodCall.invoke(Object.class.getDeclaredConstructor())
307+
.andThen(FieldAccessor.ofField("offlinePlayer").setsArgumentAt(0)))
308+
309+
.method(forwardedMethods)
310+
.intercept(forwarding)
311+
312+
.method(ElementMatchers.not(forwardedMethods))
313+
.intercept(throwException)
314+
315+
.make()
316+
.load(ByteBuddyFactory.getInstance().getClassLoader(), ClassLoadingStrategy.Default.INJECTION)
317+
.getLoaded()
318+
.getDeclaredConstructor(OfflinePlayer.class);
319+
} catch (NoSuchMethodException e) {
320+
throw new RuntimeException("Failed to find Player constructor!", e);
321+
}
322+
}
323+
324+
/**
325+
* This interface extends both OfflinePlayer and Player (in that order) so that the class generated by ByteBuddy
326+
* looks at OfflinePlayer's methods first while still being a Player.
327+
*/
328+
private interface PlayerUnion extends OfflinePlayer, Player
329+
{
283330
}
284331
}

src/main/java/com/comphenix/protocol/injector/netty/ChannelInjector.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,22 +30,26 @@
3030
import com.comphenix.protocol.reflect.VolatileField;
3131
import com.comphenix.protocol.reflect.accessors.Accessors;
3232
import com.comphenix.protocol.reflect.accessors.FieldAccessor;
33+
import com.comphenix.protocol.utility.ByteBuddyFactory;
34+
import com.comphenix.protocol.utility.ByteBuddyGenerated;
3335
import com.comphenix.protocol.utility.MinecraftFields;
3436
import com.comphenix.protocol.utility.MinecraftMethods;
3537
import com.comphenix.protocol.utility.MinecraftProtocolVersion;
3638
import com.comphenix.protocol.utility.MinecraftReflection;
3739
import com.comphenix.protocol.utility.ObjectReconstructor;
3840
import com.comphenix.protocol.wrappers.Pair;
3941
import com.comphenix.protocol.wrappers.WrappedGameProfile;
42+
4043
import com.google.common.base.Preconditions;
44+
4145
import io.netty.buffer.ByteBuf;
4246
import io.netty.channel.*;
4347
import io.netty.channel.socket.SocketChannel;
4448
import io.netty.handler.codec.ByteToMessageDecoder;
4549
import io.netty.handler.codec.MessageToByteEncoder;
4650
import io.netty.util.AttributeKey;
4751
import io.netty.util.internal.TypeParameterMatcher;
48-
import net.sf.cglib.proxy.Factory;
52+
4953
import org.apache.commons.lang.Validate;
5054
import org.bukkit.Bukkit;
5155
import org.bukkit.entity.Player;
@@ -224,7 +228,7 @@ public boolean inject() {
224228
synchronized (networkManager) {
225229
if (closed)
226230
return false;
227-
if (originalChannel instanceof Factory)
231+
if (originalChannel instanceof ByteBuddyFactory)
228232
return false;
229233
if (!originalChannel.isActive())
230234
return false;
@@ -703,7 +707,7 @@ private byte[] getBytes(ByteBuf buffer) {
703707
*/
704708
private void disconnect(String message) {
705709
// If we're logging in, we can only close the channel
706-
if (playerConnection == null || player instanceof Factory) {
710+
if (playerConnection == null || player instanceof ByteBuddyGenerated) {
707711
originalChannel.disconnect();
708712
} else {
709713
// Call the disconnect method
@@ -736,7 +740,7 @@ private void invokeSendPacket(Object packet) {
736740

737741
// Attempt to send the packet with NetworkMarker.handle(), or the PlayerConnection if its active
738742
try {
739-
if (player instanceof Factory) {
743+
if (player instanceof ByteBuddyGenerated) {
740744
MinecraftMethods.getNetworkManagerHandleMethod().invoke(networkManager, packet);
741745
} else {
742746
MinecraftMethods.getSendPacketMethod().invoke(getPlayerConnection(), packet);

0 commit comments

Comments
 (0)