From 441c4de9397280b2d6685a93bd3e4239472394b9 Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Sun, 27 Sep 2026 15:49:26 +0200 Subject: [PATCH] THRIFT-6080: Stop copying a binary field on every call to its byte[] getter Client: java The generated byte[] getX() right-sized the field and stored it through setX(), which copies again with copyBinary, so every call copied the bytes although it returns the field's own array either way. It now assigns the right-sized buffer directly, as Apache Accumulo already does by patching the generated code. Union getters and bufferForX() are unchanged. Co-Authored-By: Claude Opus 5.5 --- .../cpp/src/thrift/generate/t_java_generator.cc | 6 ++++-- .../test/java/org/apache/thrift/TestStruct.java | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/compiler/cpp/src/thrift/generate/t_java_generator.cc b/compiler/cpp/src/thrift/generate/t_java_generator.cc index ae5b69199e1..f66da120c70 100644 --- a/compiler/cpp/src/thrift/generate/t_java_generator.cc +++ b/compiler/cpp/src/thrift/generate/t_java_generator.cc @@ -2616,8 +2616,10 @@ void t_java_generator::generate_java_bean_boilerplate(ostream& out, t_struct* ts indent(out) << "@Deprecated" << '\n'; } indent(out) << "public byte[] get" << cap_name << "() {" << '\n'; - indent(out) << " set" << cap_name << "(org.apache.thrift.TBaseHelper.rightSize(" - << field_name << "));" << '\n'; + // Assign the right-sized buffer directly: going through the setter would copy it again + // (copyBinary) on every call, although the returned array is the field's own either way. + indent(out) << " this." << field_name << " = org.apache.thrift.TBaseHelper.rightSize(" + << field_name << ");" << '\n'; indent(out) << " return " << field_name << " == null ? null : " << field_name << ".array();" << '\n'; indent(out) << "}" << '\n' << '\n'; diff --git a/lib/java/src/test/java/org/apache/thrift/TestStruct.java b/lib/java/src/test/java/org/apache/thrift/TestStruct.java index aa283f4b542..b3ebfdc69aa 100644 --- a/lib/java/src/test/java/org/apache/thrift/TestStruct.java +++ b/lib/java/src/test/java/org/apache/thrift/TestStruct.java @@ -22,7 +22,9 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -427,4 +429,17 @@ public void testSubStructValidation() throws Exception { b = new StructB().setAb(valid).setAa(invalid); assertThrows(TException.class, b::validate); } + + @Test + public void testBinaryGetterDoesNotCopyOnEveryRead() { + OneOfEach ooe = new OneOfEach(); + ooe.setBase64(ByteBuffer.wrap(new byte[] {0, 1, 2, 3, 4}, 1, 3)); + + byte[] first = ooe.getBase64(); + assertArrayEquals(new byte[] {1, 2, 3}, first); + // Once right-sized, the field is read as it is, not copied again on each read. + assertSame(first, ooe.getBase64()); + // bufferForBase64() still hands out a copy. + assertNotSame(first, ooe.bufferForBase64().array()); + } }