Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -706,10 +706,16 @@ private static Map<String, String> parseUrlInternal(String url) {
if (kv.length != 2 || !PROPERTY_NAME_MAP.containsKey(key)) {
String ref = (kv.length == 2) ? key : part;
String safeRef = ref.length() > 32 ? ref.substring(0, 32) + "..." : ref;
throw new BigQueryJdbcRuntimeException(
String.format("Wrong value or unknown setting: %s", safeRef));
// Some tools can pass unknown keys. In order not to break compatibility, throw
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.

Doubt: So, now we allow unknown connection properties BUT with correct format of key=value. Should we consider allowing unknown connection properties that do not follow correct format (e.g. just a key like ;CustomToolFlag;)

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.

Is there a use case in the JDBC spec(or any known tools) that use just the key without value in the connection property?

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.

I guess not really. I was just thinking if the tool passes connection property like SomeProp= and then we end up with same problem. But yeah, maybe it is an edge case and we would still want to throw error for it for now

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Long-term I want to have this enforcement enabled for unknown parameters.. Maybe if we collect information about parameters use, we can get a full list of parameters that should be allowlisted (even if ignored).

// an exception only with incorrect format, otherwise log an error.
if (kv.length != 2) {
throw new BigQueryJdbcRuntimeException(
String.format("Wrong value or unknown setting: %s", safeRef));
} else {
LOG.warning("Wrong value or unknown setting: %s", safeRef);
continue;
}
}

map.put(PROPERTY_NAME_MAP.get(key), CharEscapers.decodeUriPath(kv[1].replace("+", "%2B")));
}
return Collections.unmodifiableMap(map);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@
package com.google.cloud.bigquery.jdbc;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.jupiter.api.Assertions.fail;

import com.google.cloud.bigquery.exception.BigQueryJdbcException;
import com.google.cloud.bigquery.exception.BigQueryJdbcRuntimeException;
import java.sql.Connection;
import java.sql.DriverPropertyInfo;
import java.sql.SQLException;
Expand Down Expand Up @@ -98,15 +95,13 @@ public void testJDBCCompliantReturnsFalse() {
}

@Test
public void testConnectWithInvalidUrlChainsException() {
try {
bigQueryDriver.connect(
"jdbc:bigquery://https://www.googleapis.com/bigquery/v2:443;InvalidProperty=Value",
new Properties());
fail("Expected SQLException");
} catch (SQLException e) {
assertThat((Throwable) e).isInstanceOf(BigQueryJdbcException.class);
assertThat(e.getCause()).isInstanceOf(BigQueryJdbcRuntimeException.class);
}
public void testConnectWithInvalidUrlChainsNoException() throws SQLException {
Connection connection =
bigQueryDriver.connect(
"jdbc:bigquery://https://www.googleapis.com/bigquery/v2:443;"
+ "OAuthType=2;OAuthAccessToken=redactedToken;ProjectId=t;"
+ "InvalidProperty=Value",
new Properties());
assertThat(connection.isClosed()).isFalse();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* Copyright 2026 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.cloud.bigquery.jdbc;

import java.util.ArrayList;
import java.util.List;
import java.util.logging.Handler;
import java.util.logging.LogRecord;
import java.util.logging.Logger;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;

public abstract class BigQueryJdbcLoggingBaseTest extends BigQueryJdbcBaseTest {

protected List<LogRecord> capturedLogs = new ArrayList<>();
private Handler handler;
private Logger logger;
private long threadId;

@BeforeEach
public void setUpLogValidator() {
logger = BigQueryJdbcRootLogger.getRootLogger();
capturedLogs.clear();
threadId = Thread.currentThread().getId();
handler =
new Handler() {
@Override
public void publish(LogRecord record) {
if (record.getThreadID() == threadId) {
capturedLogs.add(record);
}
}

@Override
public void flush() {}

@Override
public void close() throws SecurityException {}
};
logger.addHandler(handler);
}

@AfterEach
public void tearDownLogValidator() {
if (logger != null && handler != null) {
logger.removeHandler(handler);
}
}

protected boolean assertLogContains(String snippet) {
return capturedLogs.stream().anyMatch(r -> r.getMessage().contains(snippet));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,16 @@
package com.google.cloud.bigquery.jdbc;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;

import com.google.cloud.bigquery.exception.BigQueryJdbcRuntimeException;
import java.util.Collections;
import java.util.Map;
import java.util.Properties;
import org.junit.jupiter.api.Test;

public class BigQueryJdbcUrlUtilityTest {
public class BigQueryJdbcUrlUtilityTest extends BigQueryJdbcLoggingBaseTest {

@Test
public void testParsePropertyWithNoDefault() {
Expand All @@ -41,27 +40,25 @@ public void testParsePropertyWithNoDefault() {
}

@Test
public void testParseUrlWithUnknownProperty_throwsException() {
public void testParseUrlWithUnknownProperty_no_exception() {
String url =
"jdbc:bigquery://https://www.googleapis.com/bigquery/v2:443;"
+ "ProjectId=MyBigQueryProject;"
+ "UnknownProperty=SomeValue";

assertThrows(
BigQueryJdbcRuntimeException.class,
() -> BigQueryJdbcUrlUtility.parseUriProperty(url, "ProjectId"));
BigQueryJdbcUrlUtility.parseUriProperty(url, "ProjectId");
assertThat(assertLogContains("Wrong value or unknown setting")).isTrue();
}

@Test
public void testParseUrlWithTypo_throwsException() {
public void testParseUrlWithTypo_no_exception() {
String url =
"jdbc:bigquery://https://www.googleapis.com/bigquery/v2:443;"
+ "ProjectId=MyBigQueryProject;"
+ "ProjeectId=TypoValue";

assertThrows(
BigQueryJdbcRuntimeException.class,
() -> BigQueryJdbcUrlUtility.parseUriProperty(url, "ProjectId"));
assertDoesNotThrow(() -> BigQueryJdbcUrlUtility.parseUriProperty(url, "ProjectId"));
assertThat(assertLogContains("Wrong value or unknown setting")).isTrue();
}

@Test
Expand All @@ -72,7 +69,7 @@ public void testParsePropertyWithDefault() {
+ "OAuthAccessToken=RedactedToken";

String result = BigQueryJdbcUrlUtility.parseUriProperty(url, "OAuthType");
assertThat(result).isEqualTo(null);
assertThat(result).isNull();
}

@Test
Expand Down Expand Up @@ -143,14 +140,12 @@ public void testParseUrl_longUnknownProperty_sanitized() {
String longKey = String.join("", Collections.nCopies(50, "a"));
String url = "jdbc:bigquery://https://www.googleapis.com/bigquery/v2:443;" + longKey + "=value";

BigQueryJdbcRuntimeException e =
assertThrows(
BigQueryJdbcRuntimeException.class, () -> BigQueryJdbcUrlUtility.parseUrl(url));

assertThat(e.getMessage()).contains("Wrong value or unknown setting: ");
assertThat(e.getMessage()).contains("...");
assertThat(e.getMessage()).doesNotContain(longKey);
assertThat(e.getMessage().length()).isLessThan(100);
assertDoesNotThrow(() -> BigQueryJdbcUrlUtility.parseUrl(url));
String message = capturedLogs.get(0).getMessage();
assertThat(message).contains("Wrong value or unknown setting: ");
assertThat(message).contains("...");
assertThat(message).doesNotContain(longKey);
assertThat(message.length()).isLessThan(100);
}

@Test
Expand Down
Loading