Skip to content

fix(jdbc): various perf improvements #4114

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
logachev wants to merge 1 commit into
base: main
Choose a base branch
from
+83 −27
Open

fix(jdbc): various perf improvements #4114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 4 additions & 3 deletions ...-cloud-bigquery-jdbc/src/main/java/com/google/cloud/bigquery/jdbc/BigQueryConnection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import com.google.cloud.bigquery.Job;
import com.google.cloud.bigquery.JobInfo;
import com.google.cloud.bigquery.QueryJobConfiguration;
import com.google.cloud.bigquery.QueryJobConfiguration.JobCreationMode;
import com.google.cloud.bigquery.exception.BigQueryJdbcException;
import com.google.cloud.bigquery.exception.BigQueryJdbcRuntimeException;
import com.google.cloud.bigquery.exception.BigQueryJdbcSqlFeatureNotSupportedException;
Expand Down Expand Up @@ -159,10 +160,10 @@ public class BigQueryConnection extends BigQueryNoOpsConnection {
this.authProperties =
BigQueryJdbcOAuthUtility.parseOAuthProperties(url, this.connectionClassName);
this.catalog =
BigQueryJdbcUrlUtility.parseStringProperty(
BigQueryJdbcUrlUtility.parseStringPropertyLazyDefault(
url,
BigQueryJdbcUrlUtility.PROJECT_ID_PROPERTY_NAME,
BigQueryOptions.getDefaultProjectId(),
() -> BigQueryOptions.getDefaultProjectId(),
this.connectionClassName);
this.universeDomain =
BigQueryJdbcUrlUtility.parseStringProperty(
Expand Down Expand Up @@ -1064,7 +1065,7 @@ private BigQuery getBigQueryConnection() {
}

BigQueryOptions options = bigQueryOptions.setHeaderProvider(HEADER_PROVIDER).build();
options.setQueryPreviewEnabled(String.valueOf(this.useStatelessQueryMode));
options.setDefaultJobCreationMode(this.useStatelessQueryMode ? JobCreationMode.JOB_CREATION_OPTIONAL : JobCreationMode.JOB_CREATION_REQUIRED);
return options.getService();
}

Expand Down
12 changes: 12 additions & 0 deletions ...igquery-jdbc/src/main/java/com/google/cloud/bigquery/jdbc/BigQueryConnectionProperty.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@
package com.google.cloud.bigquery.jdbc;

import java.util.List;
import java.util.function.Supplier;

class BigQueryConnectionProperty {

private final String name;
private final String description;
private final String defaultValue;
private final Supplier<String> defaultValueSupplier;
private final List<String> validValues;

public String getName() {
Expand All @@ -34,6 +36,9 @@ public String getDescription() {
}

public String getDefaultValue() {
if (defaultValueSupplier != null){
return defaultValueSupplier.get();
}
return defaultValue;
}

Expand All @@ -43,6 +48,7 @@ public List<String> getValidValues() {

BigQueryConnectionProperty(Builder builder) {
this.name = builder.name;
this.defaultValueSupplier = builder.defaultValueSupplier;
this.defaultValue = builder.defaultValue;
this.description = builder.description;
this.validValues = builder.validValues;
Expand Down Expand Up @@ -79,6 +85,7 @@ static final class Builder {
private String name;
private String description;
private String defaultValue;
private Supplier<String> defaultValueSupplier = null;
private List<String> validValues;

private Builder(BigQueryConnectionProperty bigQueryConnectionProperty) {
Expand All @@ -105,6 +112,11 @@ Builder setDefaultValue(String defaultValue) {
return this;
}

Builder setLazyDefaultValue(Supplier<String> defaultValueSupplier) {
this.defaultValueSupplier = defaultValueSupplier;
return this;
}

Builder setValidValues(List<String> validValues) {
this.validValues = validValues;
return this;
Expand Down
24 changes: 13 additions & 11 deletions ...-bigquery-jdbc/src/main/java/com/google/cloud/bigquery/jdbc/BigQueryJdbcOAuthUtility.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,11 +331,11 @@ private static boolean isFileExists(String filename) {
}
}

private static boolean isJson(String value) {
private static boolean isJson(byte[] value) {
try {
// This is done this way to ensure strict Json parsing
// https://github.com/google/gson/issues/1208#issuecomment-2120764686
InputStream stream = new ByteArrayInputStream(value.getBytes());
InputStream stream = new ByteArrayInputStream(value);
InputStreamReader reader = new InputStreamReader(stream);
JsonReader jsonReader = new JsonReader(reader);
jsonReader.setStrictness(Strictness.STRICT);
Expand Down Expand Up @@ -366,16 +366,18 @@ private static GoogleCredentials getGoogleServiceAccountCredentials(
final String keyPath = pvtKeyPath != null ? pvtKeyPath : pvtKey;
PrivateKey key = null;
InputStream stream = null;

byte[] keyBytes = pvtKey != null ? pvtKey.getBytes() : null;

if (isFileExists(keyPath)) {
key = privateKeyFromP12File(keyPath, p12Password);
if (key == null) {
stream = Files.newInputStream(Paths.get(keyPath));
}
} else if (isJson(pvtKey)) {
stream = new ByteArrayInputStream(pvtKey.getBytes());
keyBytes = Files.readAllBytes(Paths.get(keyPath));
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Feb 24, 2026

Choose a reason for hiding this comment

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

security-medium medium

The use of Files.readAllBytes() to read a key file specified in the connection string can lead to a Denial of Service (DoS) via an OutOfMemoryError. An attacker who can control the connection string (e.g., in applications that allow users to provide their own JDBC URL) can point the OAuthPvtKeyPath or OAuthPvtKey property to a very large file (such as /dev/zero or a large log file) on the server. This will cause the application to attempt to read the entire file into memory, potentially crashing the JVM. Additionally, on Windows systems, providing a UNC path (e.g., \\attacker-ip\share\file) could lead to an information disclosure by leaking the Windows user's NetNTLM hash when the driver attempts to access the remote share.

      if (isFileExists(keyPath)) {
        try (InputStream is = Files.newInputStream(Paths.get(keyPath))) {
          // Limit the read size to a reasonable maximum for a key file (e.g., 1MB)
          byte[] buffer = new byte[1024 * 1024];
          int bytesRead = is.read(buffer);
          if (bytesRead != -1) {
            keyBytes = java.util.Arrays.copyOf(buffer, bytesRead);
          }
        }
      }

}

if (isJson(keyBytes)) {
stream = new ByteArrayInputStream(keyBytes);
} else if (pvtKey != null) {
key = privateKeyFromPkcs8(pvtKey);
} else {
key = privateKeyFromP12Bytes(keyBytes, p12Password);
}
Comment on lines +375 to 381
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Feb 24, 2026

Choose a reason for hiding this comment

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

high

The updated logic for parsing private keys has a flaw. It doesn't correctly handle PKCS8 keys provided via a file path (OAuthPvtKeyPath or OAuthPvtKey as a path).

  1. If OAuthPvtKeyPath is used for a PKCS8 key file, pvtKey will be null, causing the else if (pvtKey != null) branch to be skipped and the code will incorrectly try to parse the key as P12 format.
  2. If OAuthPvtKey contains a path to a PKCS8 key file, privateKeyFromPkcs8(pvtKey) will be called with the file path string instead of its content, which will fail.

This breaks authentication for these scenarios. The logic should attempt to parse the loaded keyBytes in the desired order (JSON -> PKCS8 -> P12), regardless of whether the key came from a string or a file.

Here is a suggested fix that correctly sequences the parsing attempts on the keyBytes.

      if (isJson(keyBytes)) {
        stream = new ByteArrayInputStream(keyBytes);
      } else if (keyBytes != null) {
        // Try to parse as PKCS8 private key
        key = privateKeyFromPkcs8(new String(keyBytes, java.nio.charset.StandardCharsets.UTF_8));
        if (key == null) {
          // If not PKCS8, try to parse as P12
          key = privateKeyFromP12Bytes(keyBytes, p12Password);
        }
      }


if (stream != null) {
Expand Down Expand Up @@ -703,9 +705,9 @@ private static GoogleCredentials getServiceAccountImpersonatedCredentials(
impersonationLifetimeInt);
}

static PrivateKey privateKeyFromP12File(String privateKeyFile, String password) {
static PrivateKey privateKeyFromP12Bytes(byte[] privateKey, String password) {
try {
InputStream stream = Files.newInputStream(Paths.get(privateKeyFile));
InputStream stream = new ByteArrayInputStream(privateKey);
return SecurityUtils.loadPrivateKeyFromKeyStore(
SecurityUtils.getPkcs12KeyStore(), stream, "notasecret", "privatekey", password);
} catch (IOException | GeneralSecurityException e) {
Expand Down
13 changes: 12 additions & 1 deletion ...ud-bigquery-jdbc/src/main/java/com/google/cloud/bigquery/jdbc/BigQueryJdbcUrlUtility.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.function.Supplier;
import java.util.Properties;
import java.util.Set;
import java.util.logging.Level;
Expand Down Expand Up @@ -340,7 +341,7 @@ final class BigQueryJdbcUrlUtility {
BigQueryConnectionProperty.newBuilder()
.setName(PROJECT_ID_PROPERTY_NAME)
.setDescription("A globally unique identifier for your project.")
.setDefaultValue(BigQueryOptions.getDefaultProjectId())
.setLazyDefaultValue(() -> BigQueryOptions.getDefaultProjectId())
.build(),
BigQueryConnectionProperty.newBuilder()
.setName(LOG_PATH_PROPERTY_NAME)
Expand Down Expand Up @@ -680,6 +681,16 @@ static String parseStringProperty(
return defaultValue;
}

static String parseStringPropertyLazyDefault(
String url, String propertyName, Supplier<String> defaultValueSupplier, String callerClassName) {
LOG.finest("++enter++\t" + callerClassName);
String parsedValue = BigQueryJdbcUrlUtility.parseUriProperty(url, propertyName);
if (parsedValue != null) {
return parsedValue;
}
return defaultValueSupplier.get();
}

static List<String> parseStringListProperty(
String url, String propertyName, String callerClassName) {
LOG.finest("++enter++\t" + callerClassName);
Expand Down
35 changes: 35 additions & 0 deletions ...ud-bigquery-jdbc/src/test/java/com/google/cloud/bigquery/jdbc/BigQueryConnectionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider;
import com.google.api.gax.rpc.HeaderProvider;
import com.google.api.gax.rpc.TransportChannelProvider;
import com.google.cloud.bigquery.BigQuery;
import com.google.cloud.bigquery.QueryJobConfiguration.JobCreationMode;
import com.google.cloud.bigquery.exception.BigQueryJdbcException;
import com.google.cloud.bigquery.storage.v1.BigQueryReadClient;
import com.google.cloud.bigquery.storage.v1.BigQueryWriteClient;
Expand Down Expand Up @@ -381,4 +383,37 @@ public void testBigQueryReadClientKeepAliveSettings() throws SQLException, IOExc
assertTrue(grpcProvider.getKeepAliveWithoutCalls());
}
}

@Test
public void testBigQueryJobCreationMode_required() throws Exception {
String url =
"jdbc:bigquery://https://www.googleapis.com/bigquery/v2:443;"
+ "OAuthType=3;JobCreationMode=1;";
try (BigQueryConnection connection = new BigQueryConnection(url)) {
BigQuery bq = connection.getBigQuery();
assertEquals(bq.getOptions().getDefaultJobCreationMode(), JobCreationMode.JOB_CREATION_REQUIRED);
}
}

@Test
public void testBigQueryJobCreationMode_optional() throws Exception {
String url =
"jdbc:bigquery://https://www.googleapis.com/bigquery/v2:443;"
+ "OAuthType=3;JobCreationMode=2;";
try (BigQueryConnection connection = new BigQueryConnection(url)) {
BigQuery bq = connection.getBigQuery();
assertEquals(bq.getOptions().getDefaultJobCreationMode(), JobCreationMode.JOB_CREATION_OPTIONAL);
}
}

@Test
public void testBigQueryJobCreationMode_default() throws Exception {
String url =
"jdbc:bigquery://https://www.googleapis.com/bigquery/v2:443;"
+ "OAuthType=3;";
try (BigQueryConnection connection = new BigQueryConnection(url)) {
BigQuery bq = connection.getBigQuery();
assertEquals(bq.getOptions().getDefaultJobCreationMode(), JobCreationMode.JOB_CREATION_OPTIONAL);
}
}
}
19 changes: 7 additions & 12 deletions ...query-jdbc/src/test/java/com/google/cloud/bigquery/jdbc/BigQueryJdbcOAuthUtilityTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.security.PrivateKey;
import java.util.Collections;
Expand Down Expand Up @@ -472,31 +473,25 @@ public void testPrivateKeyFromPkcs8_wrong() {
// keytool -genkey -alias privatekey -keyalg RSA -keysize 2048 -storepass notasecret \
// -keypass notasecret -storetype pkcs12 -keystore ./fake.p12
@Test
public void testPrivateKeyFromP12File() {
public void testPrivateKeyFromP12Bytes() {
URL resource = BigQueryJdbcOAuthUtilityTest.class.getResource("/fake.p12");
try {
PrivateKey pk =
BigQueryJdbcOAuthUtility.privateKeyFromP12File(
Paths.get(resource.toURI()).toAbsolutePath().toString(), "notasecret");
BigQueryJdbcOAuthUtility.privateKeyFromP12Bytes(
Files.readAllBytes(Paths.get(resource.toURI())), "notasecret");
assertNotNull(pk);
} catch (Exception e) {
assertTrue(false);
}
}

@Test
public void testPrivateKeyFromP12File_missing_file() {
PrivateKey pk = BigQueryJdbcOAuthUtility.privateKeyFromP12File("", "");
assertNull(pk);
}

@Test
public void testPrivateKeyFromP12File_wrong_password() {
public void testPrivateKeyFromP12Bytes_wrong_password() {
URL resource = BigQueryJdbcOAuthUtilityTest.class.getResource("/fake.p12");
try {
PrivateKey pk =
BigQueryJdbcOAuthUtility.privateKeyFromP12File(
Paths.get(resource.toURI()).toAbsolutePath().toString(), "fake");
BigQueryJdbcOAuthUtility.privateKeyFromP12Bytes(
Files.readAllBytes(Paths.get(resource.toURI())), "fake");
assertNull(pk);
} catch (Exception e) {
assertTrue(false);
Expand Down
Loading