THRIFT-5925: Java UUID fix

lib/java/src/main/java/org/apache/thrift/protocol/TBinaryProtocol.java

@@ -221,28 +221,9 @@ public void writeI64(long i64) throws TException {
 
   @Override
   public void writeUuid(UUID uuid) throws TException {
-    {
-      long lsb = uuid.getLeastSignificantBits();
-      inoutTemp[0] = (byte) (0xff & (lsb >> 56));
-      inoutTemp[1] = (byte) (0xff & (lsb >> 48));
-      inoutTemp[2] = (byte) (0xff & (lsb >> 40));
-      inoutTemp[3] = (byte) (0xff & (lsb >> 32));
-      inoutTemp[4] = (byte) (0xff & (lsb >> 24));
-      inoutTemp[5] = (byte) (0xff & (lsb >> 16));
-      inoutTemp[6] = (byte) (0xff & (lsb >> 8));
-      inoutTemp[7] = (byte) (0xff & (lsb));
-    }
-    {
-      long msb = uuid.getMostSignificantBits();
-      inoutTemp[8] = (byte) (0xff & (msb >> 56));
-      inoutTemp[1 + 8] = (byte) (0xff & (msb >> 48));
-      inoutTemp[2 + 8] = (byte) (0xff & (msb >> 40));
-      inoutTemp[3 + 8] = (byte) (0xff & (msb >> 32));
-      inoutTemp[4 + 8] = (byte) (0xff & (msb >> 24));
-      inoutTemp[5 + 8] = (byte) (0xff & (msb >> 16));
-      inoutTemp[6 + 8] = (byte) (0xff & (msb >> 8));
-      inoutTemp[7 + 8] = (byte) (0xff & (msb));
-    }
+    ByteBuffer bb = ByteBuffer.wrap(inoutTemp);
+    bb.putLong(uuid.getMostSignificantBits());
+    bb.putLong(uuid.getLeastSignificantBits());
     trans_.write(inoutTemp, 0, 16);
   }
 
@@ -427,25 +408,10 @@ public UUID readUuid() throws TException {
     } else {
       readAll(inoutTemp, 0, 16);
     }
-    long lsb =
-        ((long) (buf[off] & 0xff) << 56)
-            | ((long) (buf[off + 1] & 0xff) << 48)
-            | ((long) (buf[off + 2] & 0xff) << 40)
-            | ((long) (buf[off + 3] & 0xff) << 32)
-            | ((long) (buf[off + 4] & 0xff) << 24)
-            | ((long) (buf[off + 5] & 0xff) << 16)
-            | ((long) (buf[off + 6] & 0xff) << 8)
-            | ((long) (buf[off + 7] & 0xff));
-
-    long msb =
-        ((long) (buf[off + 8] & 0xff) << 56)
-            | ((long) (buf[off + 8 + 1] & 0xff) << 48)
-            | ((long) (buf[off + 8 + 2] & 0xff) << 40)
-            | ((long) (buf[off + 8 + 3] & 0xff) << 32)
-            | ((long) (buf[off + 8 + 4] & 0xff) << 24)
-            | ((long) (buf[off + 8 + 5] & 0xff) << 16)
-            | ((long) (buf[off + 8 + 6] & 0xff) << 8)
-            | ((long) (buf[off + 8 + 7] & 0xff));
+
+    ByteBuffer bb = ByteBuffer.wrap(buf, off, 16);
+    long msb = bb.getLong();
+    long lsb = bb.getLong();
     return new UUID(msb, lsb);
   }
 

lib/java/src/main/java/org/apache/thrift/protocol/TCompactProtocol.java

@@ -344,8 +344,9 @@ public void writeDouble(double dub) throws TException {
 
   @Override
   public void writeUuid(UUID uuid) throws TException {
-    fixedLongToBytes(uuid.getLeastSignificantBits(), temp, 0);
-    fixedLongToBytes(uuid.getMostSignificantBits(), temp, 8);
+    ByteBuffer bb = ByteBuffer.wrap(temp);
+    bb.putLong(uuid.getMostSignificantBits());
+    bb.putLong(uuid.getLeastSignificantBits());
     trans_.write(temp, 0, 16);
   }
 
@@ -642,9 +643,8 @@ public double readDouble() throws TException {
   @Override
   public UUID readUuid() throws TException {
     trans_.readAll(temp, 0, 16);
-    long mostSigBits = bytesToLong(temp, 8);
-    long leastSigBits = bytesToLong(temp, 0);
-    return new UUID(mostSigBits, leastSigBits);
+    ByteBuffer bb = ByteBuffer.wrap(temp, 0, 16);
+    return new UUID(bb.getLong(), bb.getLong());
   }
 
   /** Reads a byte[] (via readBinary), and then UTF-8 decodes it. */

lib/java/src/main/java/org/apache/thrift/protocol/TLegacyUuidProtocolDecorator.java

@@ -0,0 +1,98 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.thrift.protocol;
+
+import java.util.UUID;
+import org.apache.thrift.TException;
+
+/**
+ * The TLegacyUuidProtocolDecorator that decorates an existing TProtocol to provide backwards
+ * compatibility with the old UUID format that the Java library used on the wire.
+ *
+ * <p>The initial UUID implementation in Java was not according to the protocol specification. This
+ * was fixed in THRIFT-5925 and as a result would break backwards compatibility with existing
+ * implementations that depends on that format.
+ *
+ * <p>This decorator is especially useful where migration is not needed and interop between other
+ * languages, that follows the specification, are not needed.
+ *
+ * <p>For usage see the TestTLegacyUuidProtocolDecorator tests.
+ */
+public class TLegacyUuidProtocolDecorator extends TProtocolDecorator {
+
+  public TLegacyUuidProtocolDecorator(TProtocol protocol) {
+    super(protocol);
+  }
+
+  @Override
+  public void writeUuid(UUID uuid) throws TException {
+    byte[] buf = new byte[16];
+
+    long lsb = uuid.getLeastSignificantBits();
+    buf[0] = (byte) (0xff & (lsb >> 56));
+    buf[1] = (byte) (0xff & (lsb >> 48));
+    buf[2] = (byte) (0xff & (lsb >> 40));
+    buf[3] = (byte) (0xff & (lsb >> 32));
+    buf[4] = (byte) (0xff & (lsb >> 24));
+    buf[5] = (byte) (0xff & (lsb >> 16));
+    buf[6] = (byte) (0xff & (lsb >> 8));
+    buf[7] = (byte) (0xff & (lsb));
+
+    long msb = uuid.getMostSignificantBits();
+    buf[8] = (byte) (0xff & (msb >> 56));
+    buf[9] = (byte) (0xff & (msb >> 48));
+    buf[10] = (byte) (0xff & (msb >> 40));
+    buf[11] = (byte) (0xff & (msb >> 32));
+    buf[12] = (byte) (0xff & (msb >> 24));
+    buf[13] = (byte) (0xff & (msb >> 16));
+    buf[14] = (byte) (0xff & (msb >> 8));
+    buf[15] = (byte) (0xff & (msb));
+
+    getTransport().write(buf, 0, 16);
+  }
+
+  @Override
+  public UUID readUuid() throws TException {
+    byte[] buf = new byte[16];
+    getTransport().readAll(buf, 0, 16);
+
+    long lsb =
+        ((long) (buf[0] & 0xff) << 56)
+            | ((long) (buf[1] & 0xff) << 48)
+            | ((long) (buf[2] & 0xff) << 40)
+            | ((long) (buf[3] & 0xff) << 32)
+            | ((long) (buf[4] & 0xff) << 24)
+            | ((long) (buf[5] & 0xff) << 16)
+            | ((long) (buf[6] & 0xff) << 8)
+            | ((long) (buf[7] & 0xff));
+
+    long msb =
+        ((long) (buf[8] & 0xff) << 56)
+            | ((long) (buf[9] & 0xff) << 48)
+            | ((long) (buf[10] & 0xff) << 40)
+            | ((long) (buf[11] & 0xff) << 32)
+            | ((long) (buf[12] & 0xff) << 24)
+            | ((long) (buf[13] & 0xff) << 16)
+            | ((long) (buf[14] & 0xff) << 8)
+            | ((long) (buf[15] & 0xff));
+
+    return new UUID(msb, lsb);
+  }
+}

lib/java/src/test/java/org/apache/thrift/protocol/TestTLegacyUuidProtocolDecorator.java

@@ -0,0 +1,144 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.thrift.protocol;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+
+import java.util.Arrays;
+import java.util.UUID;
+import org.apache.thrift.TException;
+import org.apache.thrift.transport.TMemoryBuffer;
+import org.junit.jupiter.api.Test;
+
+public class TestTLegacyUuidProtocolDecorator {
+
+  private static final UUID TEST_UUID = UUID.fromString("00112233-4455-6677-8899-aabbccddeeff");
+
+  /**
+   * Check the correct way to wrap a TBinaryProtocol with legacy UUID byte-swapping behaviour for
+   * systems that depend on the original wire format.
+   *
+   * <p>Use TLegacyUuidProtocolDecorator when: - communicating with a peer that was built against
+   * the old TBinaryProtocol - reading data that was serialized with the old implementation
+   *
+   * <p>Use TBinaryProtocol directly when: - both peers are on the fixed implementation - starting a
+   * new integration from scratch
+   */
+  @Test
+  public void checkLegacyUuidRoundTrip() throws TException {
+    TMemoryBuffer transport = new TMemoryBuffer(16);
+    TProtocol protocol = new TLegacyUuidProtocolDecorator(new TBinaryProtocol(transport));
+
+    protocol.writeUuid(TEST_UUID);
+    UUID result = protocol.readUuid();
+
+    assertEquals(TEST_UUID, result);
+  }
+
+  /**
+   * Demonstrates that the legacy and fixed protocols produce DIFFERENT bytes on the wire, and are
+   * therefore incompatible with each other.
+   *
+   * <p>This test exists to make that incompatibility explicit and visible.
+   */
+  @Test
+  public void checkWireIncompatibility() throws TException {
+    TMemoryBuffer legacyTransport = new TMemoryBuffer(16);
+    TProtocol legacyProtocol =
+        new TLegacyUuidProtocolDecorator(new TBinaryProtocol(legacyTransport));
+    legacyProtocol.writeUuid(TEST_UUID);
+
+    TMemoryBuffer fixedTransport = new TMemoryBuffer(16);
+    TProtocol fixedProtocol = new TBinaryProtocol(fixedTransport);
+    fixedProtocol.writeUuid(TEST_UUID);
+
+    assertNotEquals(
+        Arrays.toString(legacyTransport.getArray()),
+        Arrays.toString(fixedTransport.getArray()),
+        "Legacy and fixed protocols must produce different bytes");
+  }
+
+  /**
+   * Check that a UUID written by the legacy protocol CANNOT be correctly read back by the fixed
+   * protocol, and vice versa.
+   */
+  @Test
+  public void checkCrossProtocolReadFailure() throws TException {
+    TMemoryBuffer transport = new TMemoryBuffer(16);
+
+    // write with legacy, read with fixed
+    TProtocol legacyWriter = new TLegacyUuidProtocolDecorator(new TBinaryProtocol(transport));
+    legacyWriter.writeUuid(TEST_UUID);
+
+    TProtocol fixedReader = new TBinaryProtocol(transport);
+    UUID result = fixedReader.readUuid();
+
+    assertNotEquals(
+        TEST_UUID,
+        result,
+        "Reading legacy bytes with the fixed protocol must produce a different UUID");
+  }
+
+  /** Check that a UUID written by the legacy protocol is byte swapped as expected. */
+  @Test
+  public void checkLegacySwapping() throws TException {
+    TMemoryBuffer transport = new TMemoryBuffer(16);
+
+    // write with legacy, read with fixed
+    TProtocol protocol = new TLegacyUuidProtocolDecorator(new TBinaryProtocol(transport));
+    protocol.writeUuid(TEST_UUID);
+
+    // The legacy implementation writes LSB first, then MSB - which is the wrong
+    // order
+    byte[] expected = {
+      (byte) 0x88, (byte) 0x99, (byte) 0xaa, (byte) 0xbb, // LSB high bytes
+      (byte) 0xcc, (byte) 0xdd, (byte) 0xee, (byte) 0xff, // LSB low bytes
+      (byte) 0x00, (byte) 0x11, (byte) 0x22, (byte) 0x33, // MSB high bytes
+      (byte) 0x44, (byte) 0x55, (byte) 0x66, (byte) 0x77, // MSB low bytes
+    };
+
+    byte[] actual = transport.getArray();
+    assertArrayEquals(expected, Arrays.copyOf(actual, 16));
+  }
+
+  /**
+   * Check that a UUID written by the correct protocol is as expected
+   *
+   * <p>This test is probably out of place
+   */
+  @Test
+  public void checkCorrectSwapping() throws TException {
+    TMemoryBuffer transport = new TMemoryBuffer(16);
+    TProtocol protocol = new TBinaryProtocol(transport);
+    protocol.writeUuid(TEST_UUID);
+
+    byte[] expected = {
+      (byte) 0x00, (byte) 0x11, (byte) 0x22, (byte) 0x33,
+      (byte) 0x44, (byte) 0x55, (byte) 0x66, (byte) 0x77,
+      (byte) 0x88, (byte) 0x99, (byte) 0xaa, (byte) 0xbb,
+      (byte) 0xcc, (byte) 0xdd, (byte) 0xee, (byte) 0xff,
+    };
+
+    byte[] actual = transport.getArray();
+    assertArrayEquals(expected, Arrays.copyOf(actual, 16));
+  }
+}