Skip to content

fix(bigquerystorage): accept base64 and byte[] values for BYTES columns #13981

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
laughingman7743 wants to merge 2 commits into
base: main
Choose a base branch
from
+156 −0
Open

fix(bigquerystorage): accept base64 and byte[] values for BYTES columns #13981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c4c78b9
fix(bigquerystorage): accept base64 and byte[] values for BYTES columns
laughingman7743 Aug 1, 2026
931c717
fix(bigquerystorage): pick the base64 alphabet up front instead of by…
laughingman7743 Aug 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions ...igquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1/JsonToProtoMessage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import java.time.temporal.ChronoField;
import java.time.temporal.TemporalAccessor;
import java.util.ArrayList;
import java.util.Base64;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -624,6 +625,9 @@ private void fillField(
if (val instanceof ByteString) {
protoMsg.setField(fieldDescriptor, ((ByteString) val).toByteArray());
return;
} else if (val instanceof byte[]) {
protoMsg.setField(fieldDescriptor, val);
return;
} else if (val instanceof JSONArray) {
byte[] bytes = new byte[((JSONArray) val).length()];
for (int j = 0; j < ((JSONArray) val).length(); j++) {
Expand All @@ -636,6 +640,11 @@ private void fillField(
}
protoMsg.setField(fieldDescriptor, bytes);
return;
} else if (val instanceof String
&& fieldSchema != null
&& fieldSchema.getType() == TableFieldSchema.Type.BYTES) {
protoMsg.setField(fieldDescriptor, parseBase64(currentScope, (String) val));
return;
}
break;
case INT64:
Expand Down Expand Up @@ -901,6 +910,11 @@ private void fillRepeatedField(
+ index
+ "] could not be converted to byte[]."));
}
} else if (val instanceof String
&& fieldSchema != null
&& fieldSchema.getType() == TableFieldSchema.Type.BYTES) {
protoMsg.addRepeatedField(
fieldDescriptor, parseBase64(currentScope + "[" + index + "]", (String) val));
} else {
throwWrongFieldType(fieldDescriptor, currentScope, index);
}
Expand Down Expand Up @@ -1105,6 +1119,30 @@ private static void throwWrongFieldType(
FIELD_TYPE_TO_DEBUG_MESSAGE.get(fieldDescriptor.getType()), currentScope, index));
}

/**
* Decodes a base64 string into bytes, the encoding protobuf's canonical JSON mapping defines for
* the {@code bytes} type. Standard and URL-safe alphabets are both accepted, with or without
* padding, as that mapping requires. The two differ only in {@code +/} versus {@code -_}, so the
* alphabet is picked by looking for the URL-safe characters rather than by decoding twice, which
* would make an exception part of the happy path for every URL-safe record.
*
* @throws IllegalArgumentException if the value is not valid base64
*/
private static byte[] parseBase64(String currentScope, String value) {
try {
if (value.indexOf('-') >= 0 || value.indexOf('_') >= 0) {
return Base64.getUrlDecoder().decode(value);
}
return Base64.getDecoder().decode(value);
} catch (IllegalArgumentException e) {
throw new IllegalArgumentException(
"Error: "
+ currentScope
+ " could not be converted to byte[]: not a valid base64 string.",
e);
}
}
Comment on lines +1131 to +1144

@gemini-code-assist gemini-code-assist Bot Aug 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using exceptions for control flow (catching IllegalArgumentException from the standard decoder to fall back to the URL-safe decoder) can introduce significant performance overhead, especially when processing large datasets containing URL-safe base64 strings. Since standard and URL-safe base64 alphabets only differ by the characters +// versus -/_, we can check if the string contains - or _ to decide which decoder to use. This avoids throwing and catching exceptions in the happy path for URL-safe base64 strings, and also simplifies the code by removing the nested try-catch blocks.

Suggested change
private static byte[] parseBase64(String currentScope, String value) {
try {
return Base64.getDecoder().decode(value);
} catch (IllegalArgumentException standardAlphabetFailed) {
try {
return Base64.getUrlDecoder().decode(value);
} catch (IllegalArgumentException urlSafeAlphabetFailed) {
throw new IllegalArgumentException(
"Error: "
+ currentScope
+ " could not be converted to byte[]: not a valid base64 string.");
}
}
}
private static byte[] parseBase64(String currentScope, String value) {
try {
if (value.contains("-") || value.contains("_")) {
return Base64.getUrlDecoder().decode(value);
}
return Base64.getDecoder().decode(value);
} catch (IllegalArgumentException e) {
throw new IllegalArgumentException(
"Error: "
+ currentScope
+ " could not be converted to byte[]: not a valid base64 string.",
e);
}
}

@laughingman7743 laughingman7743 Aug 1, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — applied.

Falling back through a caught exception would have put an exception on the happy path for every
record when a producer uses the URL-safe alphabet, not just occasionally, which is a real cost in a
streaming write path. Picking the alphabet up front is equivalent: a value containing - or _ can
never be valid under the standard alphabet, and one containing neither is handled by the standard
decoder, so the outcomes match in both the success and the failure case.

Applied with indexOf('-') >= 0 || indexOf('_') >= 0 rather than contains("-"), to use the char
overload; behaviour is identical. I also kept your propagation of the original exception as the
cause, which the previous version dropped.

JsonToProtoMessageTest still passes (58 tests), and the module suite is green (402 tests).


/**
* Internal helper method to check that the timestamp follows the expected String input of ISO8601
* string. Allows the fractional portion of the timestamp to support up to 12 digits of precision
Expand Down
118 changes: 118 additions & 0 deletions ...erystorage/src/test/java/com/google/cloud/bigquery/storage/v1/JsonToProtoMessageTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1112,6 +1112,124 @@ void testDate() throws Exception {
assertEquals(expectedProto, protoMsg);
}

private TableSchema bytesTableSchema(String fieldName, TableFieldSchema.Mode mode) {
return TableSchema.newBuilder()
.addFields(TableFieldSchema.newBuilder(TEST_BYTES).setName(fieldName).setMode(mode).build())
.build();
}

@Test
void testBytesFromBase64() throws Exception {
TableSchema tableSchema = bytesTableSchema("test_field_type", TableFieldSchema.Mode.NULLABLE);
BytesType expectedProto =
BytesType.newBuilder().setTestFieldType(ByteString.copyFromUtf8("hi")).build();
// Base64 for "hi". Padding is optional in protobuf's canonical JSON mapping, and "hi" encodes
// to characters the standard and URL-safe alphabets share.
for (String encoded : ImmutableList.of("aGk=", "aGk")) {
JSONObject json = new JSONObject();
json.put("test_field_type", encoded);
DynamicMessage protoMsg =
JsonToProtoMessage.INSTANCE.convertToProtoMessage(
BytesType.getDescriptor(), tableSchema, json);
assertEquals(expectedProto, protoMsg);
}
}

@Test
void testBytesFromBase64EitherAlphabet() throws Exception {
TableSchema tableSchema = bytesTableSchema("test_field_type", TableFieldSchema.Mode.NULLABLE);
BytesType expectedProto =
BytesType.newBuilder().setTestFieldType(ByteString.copyFrom(new byte[] {-5, -1})).build();
// 0xFB 0xFF is "+/8" in the standard alphabet and "-_8" in the URL-safe one — the only place
// the two differ. Protobuf's mapping accepts both, with or without padding.
for (String encoded : ImmutableList.of("+/8=", "-_8=", "+/8", "-_8")) {
JSONObject json = new JSONObject();
json.put("test_field_type", encoded);
DynamicMessage protoMsg =
JsonToProtoMessage.INSTANCE.convertToProtoMessage(
BytesType.getDescriptor(), tableSchema, json);
assertEquals(expectedProto, protoMsg);
}
}

@Test
void testBytesRepeatedFromBase64() throws Exception {
TableSchema tableSchema = bytesTableSchema("test_repeated", TableFieldSchema.Mode.REPEATED);
RepeatedBytes expectedProto =
RepeatedBytes.newBuilder()
.addTestRepeated(ByteString.copyFromUtf8("hi"))
.addTestRepeated(ByteString.copyFromUtf8("yo"))
.build();
JSONObject json = new JSONObject();
json.put("test_repeated", new JSONArray(ImmutableList.of("aGk=", "eW8=")));
DynamicMessage protoMsg =
JsonToProtoMessage.INSTANCE.convertToProtoMessage(
RepeatedBytes.getDescriptor(), tableSchema, json);
assertEquals(expectedProto, protoMsg);
}

@Test
void testBytesFromInvalidBase64() throws Exception {
TableSchema tableSchema = bytesTableSchema("test_field_type", TableFieldSchema.Mode.NULLABLE);
JSONObject json = new JSONObject();
json.put("test_field_type", "not base64 at all");
IllegalArgumentException e =
assertThrows(
IllegalArgumentException.class,
() ->
JsonToProtoMessage.INSTANCE.convertToProtoMessage(
BytesType.getDescriptor(), tableSchema, json));
assertTrue(
e.getMessage().contains("could not be converted to byte[]: not a valid base64 string."));
}

@Test
void testBytesFromBase64NeedsTableSchema() throws Exception {
// Without a TableSchema the converter cannot tell BYTES from NUMERIC and BIGNUMERIC, which
// share the same proto type, so a string stays an error rather than being decoded.
JSONObject json = new JSONObject();
json.put("test_field_type", "aGk=");
IllegalArgumentException e =
assertThrows(
IllegalArgumentException.class,
() ->
JsonToProtoMessage.INSTANCE.convertToProtoMessage(BytesType.getDescriptor(), json));
assertTrue(
e.getMessage().contains("JSONObject does not have a bytes field at root.test_field_type."));
}

@Test
void testNumericFromStringIsNotDecodedAsBase64() throws Exception {
TableSchema tableSchema =
TableSchema.newBuilder()
.addFields(TableFieldSchema.newBuilder(TEST_NUMERIC).setName("test_field_type").build())
.build();
BytesType expectedProto =
BytesType.newBuilder()
.setTestFieldType(
BigDecimalByteStringEncoder.encodeToNumericByteString(new BigDecimal("1.5")))
.build();
JSONObject json = new JSONObject();
json.put("test_field_type", "1.5");
DynamicMessage protoMsg =
JsonToProtoMessage.INSTANCE.convertToProtoMessage(
BytesType.getDescriptor(), tableSchema, json);
assertEquals(expectedProto, protoMsg);
}

@Test
void testBytesFromByteArray() throws Exception {
TableSchema tableSchema = bytesTableSchema("test_field_type", TableFieldSchema.Mode.NULLABLE);
BytesType expectedProto =
BytesType.newBuilder().setTestFieldType(ByteString.copyFromUtf8("hi")).build();
JSONObject json = new JSONObject();
json.put("test_field_type", new byte[] {104, 105});
DynamicMessage protoMsg =
JsonToProtoMessage.INSTANCE.convertToProtoMessage(
BytesType.getDescriptor(), tableSchema, json);
assertEquals(expectedProto, protoMsg);
}

@Test
void testAllTypes() throws Exception {
for (Map.Entry<Descriptor, String> entry : AllTypesToDebugMessageTest.entrySet()) {
Expand Down
Loading