From 6ff5cac4d31862449862daae7ec12d0abfafdaec Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sat, 19 Oct 2019 23:56:44 -0400 Subject: [PATCH] Fix issues with decoding and Java fallback native --- .../encryption/JavaVelocityCipher.java | 7 ++-- .../natives/util/BufferPreference.java | 4 ++ .../natives/util/MoreByteBufUtils.java | 42 ++++++++++++------- .../netty/MinecraftCipherDecoder.java | 3 +- 4 files changed, 36 insertions(+), 20 deletions(-) diff --git a/native/src/main/java/com/velocitypowered/natives/encryption/JavaVelocityCipher.java b/native/src/main/java/com/velocitypowered/natives/encryption/JavaVelocityCipher.java index 0a02a78fc..dc3cd2318 100644 --- a/native/src/main/java/com/velocitypowered/natives/encryption/JavaVelocityCipher.java +++ b/native/src/main/java/com/velocitypowered/natives/encryption/JavaVelocityCipher.java @@ -37,12 +37,13 @@ public class JavaVelocityCipher implements VelocityCipher { @Override public void process(ByteBuf source) { ensureNotDisposed(); + Preconditions.checkArgument(source.hasArray(), "No source array"); int inBytes = source.readableBytes(); - byte[] asBytes = ByteBufUtil.getBytes(source); try { - cipher.update(asBytes, 0, inBytes, asBytes); + cipher.update(source.array(), source.arrayOffset(), inBytes, source.array(), + source.arrayOffset()); } catch (ShortBufferException ex) { /* This _really_ shouldn't happen - AES CFB8 will work in place. If you run into this, that means that for whatever reason the Java Runtime has determined @@ -64,6 +65,6 @@ public class JavaVelocityCipher implements VelocityCipher { @Override public BufferPreference preferredBufferType() { - return BufferPreference.HEAP_PREFERRED; + return BufferPreference.HEAP_REQUIRED; } } diff --git a/native/src/main/java/com/velocitypowered/natives/util/BufferPreference.java b/native/src/main/java/com/velocitypowered/natives/util/BufferPreference.java index 7be4fa74e..6814b11fd 100644 --- a/native/src/main/java/com/velocitypowered/natives/util/BufferPreference.java +++ b/native/src/main/java/com/velocitypowered/natives/util/BufferPreference.java @@ -1,6 +1,10 @@ package com.velocitypowered.natives.util; public enum BufferPreference { + /** + * A heap buffer is required. + */ + HEAP_REQUIRED, /** * A heap buffer is preferred (but not required). */ diff --git a/native/src/main/java/com/velocitypowered/natives/util/MoreByteBufUtils.java b/native/src/main/java/com/velocitypowered/natives/util/MoreByteBufUtils.java index 5bf328396..dd41e3fc7 100644 --- a/native/src/main/java/com/velocitypowered/natives/util/MoreByteBufUtils.java +++ b/native/src/main/java/com/velocitypowered/natives/util/MoreByteBufUtils.java @@ -20,28 +20,30 @@ public class MoreByteBufUtils { * @return a buffer compatible with the native */ public static ByteBuf ensureCompatible(ByteBufAllocator alloc, Native nativeStuff, ByteBuf buf) { - if (nativeStuff.preferredBufferType() != BufferPreference.DIRECT_REQUIRED - || buf.hasMemoryAddress()) { + if (isCompatible(nativeStuff, buf)) { return buf.retain(); } // It's not, so we must make a direct copy. - ByteBuf newBuf = alloc.directBuffer(buf.readableBytes()); + ByteBuf newBuf = preferredBuffer(alloc, nativeStuff, buf.readableBytes()); newBuf.writeBytes(buf); return newBuf; } - /** - * Creates a {@link ByteBuf} that will have the best performance with the specified - * {@code nativeStuff}. - * - * @param alloc the {@link ByteBufAllocator} to use - * @param nativeStuff the native we are working with - * @return a buffer compatible with the native - */ - public static ByteBuf preferredBuffer(ByteBufAllocator alloc, Native nativeStuff) { - return nativeStuff.preferredBufferType() != BufferPreference.HEAP_PREFERRED - ? alloc.directBuffer() : alloc.heapBuffer(); + private static boolean isCompatible(Native nativeStuff, ByteBuf buf) { + BufferPreference preferred = nativeStuff.preferredBufferType(); + switch (preferred) { + case DIRECT_PREFERRED: + case HEAP_PREFERRED: + // The native prefers this type, but doesn't strictly require we provide it. + return true; + case DIRECT_REQUIRED: + return buf.hasMemoryAddress(); + case HEAP_REQUIRED: + return buf.hasArray(); + default: + throw new AssertionError("Preferred buffer type unknown"); + } } /** @@ -55,7 +57,15 @@ public class MoreByteBufUtils { */ public static ByteBuf preferredBuffer(ByteBufAllocator alloc, Native nativeStuff, int initialCapacity) { - return nativeStuff.preferredBufferType() != BufferPreference.HEAP_PREFERRED - ? alloc.directBuffer(initialCapacity) : alloc.heapBuffer(initialCapacity); + switch (nativeStuff.preferredBufferType()) { + case HEAP_REQUIRED: + case HEAP_PREFERRED: + return alloc.heapBuffer(initialCapacity); + case DIRECT_PREFERRED: + case DIRECT_REQUIRED: + return alloc.directBuffer(initialCapacity); + default: + throw new AssertionError("Preferred buffer type unknown"); + } } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCipherDecoder.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCipherDecoder.java index d76b345d3..2b4d0b502 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCipherDecoder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCipherDecoder.java @@ -18,10 +18,11 @@ public class MinecraftCipherDecoder extends ByteToMessageDecoder { @Override protected void decode(ChannelHandlerContext ctx, ByteBuf in, List out) throws Exception { - ByteBuf compatible = MoreByteBufUtils.ensureCompatible(ctx.alloc(), cipher, in); + ByteBuf compatible = MoreByteBufUtils.ensureCompatible(ctx.alloc(), cipher, in).slice(); try { cipher.process(compatible); out.add(compatible); + in.skipBytes(in.readableBytes()); } catch (Exception e) { compatible.release(); // compatible will never be used if we throw an exception throw e;