diff --git a/lib/java/src/main/java/org/apache/thrift/protocol/TBinaryProtocol.java b/lib/java/src/main/java/org/apache/thrift/protocol/TBinaryProtocol.java index 64d62b15a3f..6b016b89591 100644 --- a/lib/java/src/main/java/org/apache/thrift/protocol/TBinaryProtocol.java +++ b/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); } diff --git a/lib/java/src/main/java/org/apache/thrift/protocol/TCompactProtocol.java b/lib/java/src/main/java/org/apache/thrift/protocol/TCompactProtocol.java index bd36b6bf6da..1976c00970f 100644 --- a/lib/java/src/main/java/org/apache/thrift/protocol/TCompactProtocol.java +++ b/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. */ diff --git a/lib/java/src/main/java/org/apache/thrift/protocol/TLegacyUuidProtocolDecorator.java b/lib/java/src/main/java/org/apache/thrift/protocol/TLegacyUuidProtocolDecorator.java new file mode 100644 index 00000000000..26b09240a18 --- /dev/null +++ b/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. + * + *

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. + * + *

This decorator is especially useful where migration is not needed and interop between other + * languages, that follows the specification, are not needed. + * + *

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); + } +} diff --git a/lib/java/src/test/java/org/apache/thrift/protocol/TestTLegacyUuidProtocolDecorator.java b/lib/java/src/test/java/org/apache/thrift/protocol/TestTLegacyUuidProtocolDecorator.java new file mode 100644 index 00000000000..c128c1d8e21 --- /dev/null +++ b/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. + * + *

Use TLegacyUuidProtocolDecorator when: - communicating with a peer that was built against + * the old TBinaryProtocol - reading data that was serialized with the old implementation + * + *

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. + * + *

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 + * + *

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)); + } +}