You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by br...@apache.org on 2011/09/26 23:29:16 UTC

svn commit: r1176072 - in /thrift/trunk: compiler/cpp/src/generate/t_java_generator.cc lib/java/src/org/apache/thrift/protocol/TTupleProtocol.java lib/java/test/org/apache/thrift/protocol/TestTTupleProtocol.java test/DebugProtoTest.thrift

Author: bryanduxbury
Date: Mon Sep 26 21:29:15 2011
New Revision: 1176072

URL: http://svn.apache.org/viewvc?rev=1176072&view=rev
Log:
THRIFT-1365. java: TupleProtocol#writeBitSet unintentionally writes a variable length byte array

Modified:
    thrift/trunk/compiler/cpp/src/generate/t_java_generator.cc
    thrift/trunk/lib/java/src/org/apache/thrift/protocol/TTupleProtocol.java
    thrift/trunk/lib/java/test/org/apache/thrift/protocol/TestTTupleProtocol.java
    thrift/trunk/test/DebugProtoTest.thrift

Modified: thrift/trunk/compiler/cpp/src/generate/t_java_generator.cc
URL: http://svn.apache.org/viewvc/thrift/trunk/compiler/cpp/src/generate/t_java_generator.cc?rev=1176072&r1=1176071&r2=1176072&view=diff
==============================================================================
--- thrift/trunk/compiler/cpp/src/generate/t_java_generator.cc (original)
+++ thrift/trunk/compiler/cpp/src/generate/t_java_generator.cc Mon Sep 26 21:29:15 2011
@@ -4059,8 +4059,10 @@ void t_java_generator::generate_java_str
   const vector<t_field*>& fields = tstruct->get_members();
   vector<t_field*>::const_iterator f_iter;
   bool has_optional = false;
+  int optional_count = 0;
   for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
     if ((*f_iter)->get_req() == t_field::T_OPTIONAL || (*f_iter)->get_req() == t_field::T_OPT_IN_REQ_OUT) {
+      optional_count++;
       has_optional = true;
     }
     if ((*f_iter)->get_req() == t_field::T_REQUIRED) {
@@ -4081,7 +4083,7 @@ void t_java_generator::generate_java_str
       }
     }
 
-    indent(out) << "oprot.writeBitSet(optionals);" << endl;
+    indent(out) << "oprot.writeBitSet(optionals, " << optional_count << ");" << endl;
     int j = 0;
     for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
       if ((*f_iter)->get_req() == t_field::T_OPTIONAL || (*f_iter)->get_req() == t_field::T_OPT_IN_REQ_OUT) {

Modified: thrift/trunk/lib/java/src/org/apache/thrift/protocol/TTupleProtocol.java
URL: http://svn.apache.org/viewvc/thrift/trunk/lib/java/src/org/apache/thrift/protocol/TTupleProtocol.java?rev=1176072&r1=1176071&r2=1176072&view=diff
==============================================================================
--- thrift/trunk/lib/java/src/org/apache/thrift/protocol/TTupleProtocol.java (original)
+++ thrift/trunk/lib/java/src/org/apache/thrift/protocol/TTupleProtocol.java Mon Sep 26 21:29:15 2011
@@ -43,8 +43,8 @@ public final class TTupleProtocol extend
     return TupleScheme.class;
   }
 
-  public void writeBitSet(BitSet bs) throws TException {
-    byte[] bytes = toByteArray(bs);
+  public void writeBitSet(BitSet bs, int vectorWidth) throws TException {
+    byte[] bytes = toByteArray(bs, vectorWidth);
     for (byte b : bytes) {
       writeByte(b);
     }
@@ -82,10 +82,11 @@ public final class TTupleProtocol extend
    * assumed to be the least significant bit.
    * 
    * @param bits
+   * @param vectorWidth 
    * @return a byte array of at least length 1
    */
-  public static byte[] toByteArray(BitSet bits) {
-    byte[] bytes = new byte[bits.length() / 8 + 1];
+  public static byte[] toByteArray(BitSet bits, int vectorWidth) {
+    byte[] bytes = new byte[vectorWidth / 8 + 1];
     for (int i = 0; i < bits.length(); i++) {
       if (bits.get(i)) {
         bytes[bytes.length - i / 8 - 1] |= 1 << (i % 8);

Modified: thrift/trunk/lib/java/test/org/apache/thrift/protocol/TestTTupleProtocol.java
URL: http://svn.apache.org/viewvc/thrift/trunk/lib/java/test/org/apache/thrift/protocol/TestTTupleProtocol.java?rev=1176072&r1=1176071&r2=1176072&view=diff
==============================================================================
--- thrift/trunk/lib/java/test/org/apache/thrift/protocol/TestTTupleProtocol.java (original)
+++ thrift/trunk/lib/java/test/org/apache/thrift/protocol/TestTTupleProtocol.java Mon Sep 26 21:29:15 2011
@@ -1,14 +1,10 @@
 package org.apache.thrift.protocol;
 
-import java.util.HashMap;
-import java.util.Map;
+import org.apache.thrift.TDeserializer;
+import org.apache.thrift.TSerializer;
 
-import org.apache.thrift.TException;
-import org.apache.thrift.transport.TMemoryBuffer;
-import org.apache.thrift.transport.TMemoryInputTransport;
+import thrift.test.TupleProtocolTestStruct;
 
-import thrift.test.Complex;
-import thrift.test.Simple;
 
 public class TestTTupleProtocol extends ProtocolTestBase {
 
@@ -21,4 +17,11 @@ public class TestTTupleProtocol extends 
   protected TProtocolFactory getFactory() {
     return new TTupleProtocol.Factory();
   }
+
+  public void testBitsetLengthIssue() throws Exception {
+    final TupleProtocolTestStruct t1 = new TupleProtocolTestStruct();
+    t1.setField1(0);
+    t1.setField2(12);
+    new TDeserializer(new TTupleProtocol.Factory()).deserialize(new TupleProtocolTestStruct(), new TSerializer(new TTupleProtocol.Factory()).serialize(t1));
+  }
 }

Modified: thrift/trunk/test/DebugProtoTest.thrift
URL: http://svn.apache.org/viewvc/thrift/trunk/test/DebugProtoTest.thrift?rev=1176072&r1=1176071&r2=1176072&view=diff
==============================================================================
--- thrift/trunk/test/DebugProtoTest.thrift (original)
+++ thrift/trunk/test/DebugProtoTest.thrift Mon Sep 26 21:29:15 2011
@@ -351,3 +351,17 @@ struct BreaksRubyCompactProtocol {
   3: i32 field3;
 }
 
+struct TupleProtocolTestStruct {
+  optional i32 field1;
+  optional i32 field2;
+  optional i32 field3;
+  optional i32 field4;
+  optional i32 field5;
+  optional i32 field6;
+  optional i32 field7;
+  optional i32 field8;
+  optional i32 field9;
+  optional i32 field10;
+  optional i32 field11;
+  optional i32 field12;
+}
\ No newline at end of file