Skip to content

Commit

Permalink
Fix DefaultChannelId#asLongText NPE
Browse files Browse the repository at this point in the history
Motivation:
The DefaultChannelId class is serializable, but it does not account for variations in the length of machineId.

Modifications:
Adjusted code to accommodate varying lengths of MachineId.

Result:
Fixed NPE.
Respects varying length of machineId.
  • Loading branch information
jchrys committed Apr 14, 2024
1 parent e19c91f commit b2361c0
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 11 deletions.
31 changes: 20 additions & 11 deletions transport/src/main/java/io/netty/channel/DefaultChannelId.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import io.netty.util.internal.MacAddressUtil;
import io.netty.util.internal.PlatformDependent;
import io.netty.util.internal.SystemPropertyUtil;
import io.netty.util.internal.ThreadLocalRandom;
import io.netty.util.internal.logging.InternalLogger;
import io.netty.util.internal.logging.InternalLoggerFactory;

Expand Down Expand Up @@ -53,7 +54,11 @@ public final class DefaultChannelId implements ChannelId {
* Returns a new {@link DefaultChannelId} instance.
*/
public static DefaultChannelId newInstance() {
return new DefaultChannelId();
return new DefaultChannelId(MACHINE_ID,
PROCESS_ID,
nextSequence.getAndIncrement(),
Long.reverse(System.nanoTime()) ^ System.currentTimeMillis(),
ThreadLocalRandom.current().nextInt());
}

static {
Expand Down Expand Up @@ -189,28 +194,31 @@ static int defaultProcessId() {
private transient String shortValue;
private transient String longValue;

private DefaultChannelId() {
final byte[] data = new byte[MACHINE_ID.length + PROCESS_ID_LEN + SEQUENCE_LEN + TIMESTAMP_LEN + RANDOM_LEN];
/**
* Visible for testing
*/
DefaultChannelId(final byte[] machineId, final int processId, final int sequence,
final long timestamp, final int random) {
final byte[] data = new byte[machineId.length + PROCESS_ID_LEN + SEQUENCE_LEN + TIMESTAMP_LEN + RANDOM_LEN];
int i = 0;

// machineId
System.arraycopy(MACHINE_ID, 0, data, i, MACHINE_ID.length);
i += MACHINE_ID.length;
System.arraycopy(machineId, 0, data, i, machineId.length);
i += machineId.length;

// processId
writeInt(data, i, PROCESS_ID);
writeInt(data, i, processId);
i += Integer.BYTES;

// sequence
writeInt(data, i, nextSequence.getAndIncrement());
writeInt(data, i, sequence);
i += Integer.BYTES;

// timestamp (kind of)
writeLong(data, i, Long.reverse(System.nanoTime()) ^ System.currentTimeMillis());
writeLong(data, i, timestamp);
i += Long.BYTES;

// random
int random = PlatformDependent.threadLocalRandom().nextInt();
writeInt(data, i, random);
i += Integer.BYTES;
assert i == data.length;
Expand Down Expand Up @@ -264,9 +272,10 @@ public String asLongText() {
}

private String newLongValue() {
StringBuilder buf = new StringBuilder(2 * data.length + 5);
final StringBuilder buf = new StringBuilder(2 * data.length + 5);
final int machineIdLen = data.length - PROCESS_ID_LEN - SEQUENCE_LEN - TIMESTAMP_LEN - RANDOM_LEN;
int i = 0;
i = appendHexDumpField(buf, i, MACHINE_ID.length);
i = appendHexDumpField(buf, i, machineIdLen);
i = appendHexDumpField(buf, i, PROCESS_ID_LEN);
i = appendHexDumpField(buf, i, SEQUENCE_LEN);
i = appendHexDumpField(buf, i, TIMESTAMP_LEN);
Expand Down
33 changes: 33 additions & 0 deletions transport/src/test/java/io/netty/channel/DefaultChannelIdTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,13 @@
import io.netty.buffer.ByteBufInputStream;
import io.netty.buffer.ByteBufOutputStream;
import io.netty.buffer.Unpooled;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;

import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.util.Arrays;
import java.util.Comparator;

import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.not;
Expand All @@ -33,6 +36,7 @@

@SuppressWarnings("DynamicRegexReplaceableByCompiledPattern")
public class DefaultChannelIdTest {

@Test
public void testShortText() {
String text = DefaultChannelId.newInstance().asShortText();
Expand Down Expand Up @@ -84,4 +88,33 @@ public void testSerialization() throws Exception {
assertThat(a, is(not(sameInstance(b))));
assertThat(a.asLongText(), is(b.asLongText()));
}

@Test
public void testDeserialization() throws Exception {
// DefaultChannelId with 8 byte machineId
final DefaultChannelId c8 = new DefaultChannelId(
new byte[] {
(byte) 0x01, (byte) 0x23, (byte) 0x45, (byte) 0x67,
(byte) 0x89, (byte) 0xab, (byte) 0xcd, (byte) 0xef
},
0x000052af,
0x00000000,
0x06504f638eb4c386L,
0xd964df5e);

// DefaultChannelId with 6 byte machineId
final DefaultChannelId c6 =
new DefaultChannelId(
new byte[] {
(byte) 0x01, (byte) 0x23, (byte) 0x45, (byte) 0x67,
(byte) 0x89, (byte) 0xab,
},
0xce005283,
0x00000001,
0x069e6dce9eb4516fL,
0x721757b7);

assertThat(c8.asLongText(), is("0123456789abcdef-000052af-00000000-06504f638eb4c386-d964df5e"));
assertThat(c6.asLongText(), is("0123456789ab-ce005283-00000001-069e6dce9eb4516f-721757b7"));
}
}

0 comments on commit b2361c0

Please sign in to comment.