You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by el...@apache.org on 2016/05/02 20:14:10 UTC
calcite git commit: [CALCITE-1209] Ensure protobuf does not serialize
bytes as base64 strings
Repository: calcite
Updated Branches:
refs/heads/master 3599cebbc -> e4dd6dc76
[CALCITE-1209] Ensure protobuf does not serialize bytes as base64 strings
CALCITE-1103 inadvertently introduced a fix for binary data being
encoded as base64'ed strings instead of native binary data. This
left a backwards compatibility problem where older clients would
be unable to communicate with newer servers.
Unit tests were added for the legacy parsing for backwards compatibility
as well as verifying new data is serialized as native bytes.
Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/e4dd6dc7
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/e4dd6dc7
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/e4dd6dc7
Branch: refs/heads/master
Commit: e4dd6dc765e125b156c2317fbe567f11585ea9f2
Parents: 3599ceb
Author: Josh Elser <el...@apache.org>
Authored: Mon May 2 13:09:36 2016 -0400
Committer: Josh Elser <el...@apache.org>
Committed: Mon May 2 14:13:55 2016 -0400
----------------------------------------------------------------------
.../calcite/avatica/remote/TypedValue.java | 17 +++++++++
.../calcite/avatica/remote/TypedValueTest.java | 39 ++++++++++++++++++++
2 files changed, 56 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/calcite/blob/e4dd6dc7/avatica/core/src/main/java/org/apache/calcite/avatica/remote/TypedValue.java
----------------------------------------------------------------------
diff --git a/avatica/core/src/main/java/org/apache/calcite/avatica/remote/TypedValue.java b/avatica/core/src/main/java/org/apache/calcite/avatica/remote/TypedValue.java
index b72cfbf..b13bfe7 100644
--- a/avatica/core/src/main/java/org/apache/calcite/avatica/remote/TypedValue.java
+++ b/avatica/core/src/main/java/org/apache/calcite/avatica/remote/TypedValue.java
@@ -19,6 +19,7 @@ package org.apache.calcite.avatica.remote;
import org.apache.calcite.avatica.ColumnMetaData;
import org.apache.calcite.avatica.ColumnMetaData.Rep;
import org.apache.calcite.avatica.proto.Common;
+import org.apache.calcite.avatica.util.Base64;
import org.apache.calcite.avatica.util.ByteString;
import org.apache.calcite.avatica.util.DateTimeUtils;
@@ -126,6 +127,10 @@ import static java.nio.charset.StandardCharsets.UTF_8;
public class TypedValue {
private static final FieldDescriptor NUMBER_DESCRIPTOR = Common.TypedValue.getDescriptor()
.findFieldByNumber(Common.TypedValue.NUMBER_VALUE_FIELD_NUMBER);
+ private static final FieldDescriptor STRING_DESCRIPTOR = Common.TypedValue.getDescriptor()
+ .findFieldByNumber(Common.TypedValue.STRING_VALUE_FIELD_NUMBER);
+ private static final FieldDescriptor BYTES_DESCRIPTOR = Common.TypedValue.getDescriptor()
+ .findFieldByNumber(Common.TypedValue.BYTES_VALUE_FIELD_NUMBER);
public static final TypedValue NULL =
new TypedValue(ColumnMetaData.Rep.OBJECT, null);
@@ -379,9 +384,14 @@ public class TypedValue {
byte[] bytes;
// Serial representation is b64. We don't need to do that for protobuf
if (o instanceof String) {
+ // Backwards compatibility for client CALCITE-1209
+ builder.setStringValue((String) o);
// Assume strings are already b64 encoded
bytes = ByteString.parseBase64((String) o);
} else {
+ // Backwards compatibility for client CALCITE-1209
+ builder.setStringValue(Base64.encodeBytes((byte[]) o));
+ // Use the byte array
bytes = (byte[]) o;
}
builder.setBytesValue(HBaseZeroCopyByteString.wrap(bytes));
@@ -484,6 +494,13 @@ public class TypedValue {
case PRIMITIVE_BOOLEAN:
return protoValue.getBoolValue();
case BYTE_STRING:
+ if (protoValue.hasField(STRING_DESCRIPTOR) && !protoValue.hasField(BYTES_DESCRIPTOR)) {
+ // Prior to CALCITE-1103, clients would send b64 strings for bytes instead of using the
+ // native byte format. The value we need to provide as the local format for TypedValue
+ // is directly accessible via the Protobuf representation. Both fields are sent by the
+ // server to support older clients, so only parse the string value when it is alone.
+ return protoValue.getStringValue();
+ }
// TypedValue is still going to expect a b64string for BYTE_STRING even though we sent it
// across the wire natively as bytes. Return it as b64.
return (new ByteString(protoValue.getBytesValue().toByteArray())).toBase64String();
http://git-wip-us.apache.org/repos/asf/calcite/blob/e4dd6dc7/avatica/core/src/test/java/org/apache/calcite/avatica/remote/TypedValueTest.java
----------------------------------------------------------------------
diff --git a/avatica/core/src/test/java/org/apache/calcite/avatica/remote/TypedValueTest.java b/avatica/core/src/test/java/org/apache/calcite/avatica/remote/TypedValueTest.java
index b714e9d..9343ed2 100644
--- a/avatica/core/src/test/java/org/apache/calcite/avatica/remote/TypedValueTest.java
+++ b/avatica/core/src/test/java/org/apache/calcite/avatica/remote/TypedValueTest.java
@@ -18,6 +18,7 @@ package org.apache.calcite.avatica.remote;
import org.apache.calcite.avatica.ColumnMetaData.Rep;
import org.apache.calcite.avatica.proto.Common;
+import org.apache.calcite.avatica.util.Base64;
import org.apache.calcite.avatica.util.ByteString;
import org.junit.Test;
@@ -162,6 +163,44 @@ public class TypedValueTest {
Object o = fromProtoTv.toJdbc(calendar);
assertEquals(decimal, o);
}
+
+ @Test public void testProtobufBytesNotSentAsBase64() {
+ final byte[] bytes = "asdf".getBytes(UTF_8);
+ final byte[] b64Bytes = Base64.encodeBytes(bytes).getBytes(UTF_8);
+ TypedValue tv = TypedValue.ofLocal(Rep.BYTE_STRING, new ByteString(bytes));
+ // JSON encodes it as base64
+ assertEquals(new String(b64Bytes), tv.value);
+
+ // Get the protobuf variant
+ Common.TypedValue protoTv = tv.toProto();
+ Common.Rep protoRep = protoTv.getType();
+ assertEquals(Common.Rep.BYTE_STRING, protoRep);
+
+ // The pb variant should have the native bytes of the original value
+ com.google.protobuf.ByteString protoByteString = protoTv.getBytesValue();
+ assertNotNull(protoByteString);
+ assertArrayEquals(bytes, protoByteString.toByteArray());
+
+ // We should have the b64 string as a backwards compatibility feature
+ assertEquals(new String(b64Bytes), protoTv.getStringValue());
+ }
+
+ @Test public void testLegacyBase64StringEncodingForBytes() {
+ // CALCITE-1103 CALCITE-1209 We observed that binary data was being
+ // serialized as base-64 encoded strings instead of the native binary
+ // data type in protobufs. We need to still handle older clients sending
+ // data in this form.
+ final byte[] bytes = "asdf".getBytes(UTF_8);
+ final String base64Str = Base64.encodeBytes(bytes);
+ Common.TypedValue.Builder builder = Common.TypedValue.newBuilder();
+ builder.setStringValue(base64Str);
+ builder.setType(Common.Rep.BYTE_STRING);
+ Common.TypedValue protoTv = builder.build();
+
+ TypedValue tv = TypedValue.fromProto(protoTv);
+ assertEquals(Rep.BYTE_STRING, tv.type);
+ assertEquals(base64Str, tv.value);
+ }
}
// End TypedValueTest.java