Skip to content

chore(bqjdbc): log connection properties set by the user #13244

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
Neenu1995 wants to merge 2 commits into
base: main
Choose a base branch
from
+70 −1
Open

chore(bqjdbc): log connection properties set by the user #13244

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
e19d2c3
chore(bqjdbc): log connection properties set by the user
Neenu1995 May 20, 2026
396e627
nit
Neenu1995 May 20, 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
18 changes: 18 additions & 0 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 @@ -152,6 +152,24 @@ public class BigQueryConnection extends BigQueryNoOpsConnection {
this.connectionId = UUID.randomUUID().toString();
try (BigQueryJdbcMdc.MdcCloseable mdc = BigQueryJdbcMdc.registerInstance(this.connectionId)) {
this.connectionUrl = url;
if (LOG.isLoggable(java.util.logging.Level.CONFIG)) {
Properties connectionProps = ds.createProperties();
Properties maskedProps = new Properties();
for (String name : connectionProps.stringPropertyNames()) {
Copy link
Copy Markdown
Contributor

@logachev logachev May 21, 2026

Choose a reason for hiding this comment

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

I'd suggest using reverse approach. Instead of "opt-out" have an "opt-in".
We can create a list of properties that are "safe" to log.

String value = connectionProps.getProperty(name);
String lowerName = name.toLowerCase();
if ((lowerName.contains("key")
|| lowerName.contains("token")
|| lowerName.contains("password")
|| lowerName.contains("pwd")
|| lowerName.contains("secret"))
&& !lowerName.equals("partnertoken")) {
Comment on lines +161 to +166
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 20, 2026

Choose a reason for hiding this comment

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

medium

For better readability and maintainability, consider extracting the sensitive keywords into a Set and using a stream to check if the property name contains any of them. This makes the condition less verbose and easier to extend in the future.

For example, you could define a static set of keywords at the class level:

private static final java.util.Set<String> SENSITIVE_KEYWORDS = new java.util.HashSet<>(java.util.Arrays.asList("key", "token", "password", "pwd", "secret"));

And then use it in the condition like this:

if (SENSITIVE_KEYWORDS.stream().anyMatch(lowerName::contains)
    && !lowerName.equals("partnertoken")) {

value = "*****";
}
maskedProps.setProperty(name, value);
}
LOG.config("Connection properties: %s", maskedProps.toString());
}
this.openStatements = ConcurrentHashMap.newKeySet();
this.autoCommit = true;
this.sqlWarnings = new ArrayList<>();
Expand Down
2 changes: 1 addition & 1 deletion ...y/google-cloud-bigquery-jdbc/src/main/java/com/google/cloud/bigquery/jdbc/DataSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ public Connection getConnection() throws SQLException {
return DriverManager.getConnection(getURL(), createProperties());
}

private Properties createProperties() {
Properties createProperties() {
Properties connectionProperties = new Properties();
if (this.projectId != null) {
connectionProperties.setProperty(
Expand Down
51 changes: 51 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 @@ -456,4 +456,55 @@ public void testIsReadOnlyTokenProvided(String readonlyProp, boolean expectedIsR
assertEquals(expectedIsReadOnly, connection.isReadOnlyTokenUsed());
}
}

@Test
public void testConnectionPropertiesLoggingAndMasking() throws IOException, SQLException {
java.util.logging.Logger rootLogger = BigQueryJdbcRootLogger.getRootLogger();
java.util.logging.Level originalLevel = rootLogger.getLevel();
rootLogger.setLevel(java.util.logging.Level.INFO);
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 20, 2026

Choose a reason for hiding this comment

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

critical

The code in BigQueryConnection logs properties at java.util.logging.Level.CONFIG. For the log message to be captured, the logger's level must be set to CONFIG or a finer level (e.g., ALL). Setting it to INFO will prevent the message from being logged, as INFO (value 800) is a coarser level than CONFIG (value 700).

Suggested change
rootLogger.setLevel(java.util.logging.Level.INFO);
rootLogger.setLevel(java.util.logging.Level.CONFIG);


java.util.List<java.util.logging.LogRecord> records = new java.util.ArrayList<>();
java.util.logging.Handler handler =
new java.util.logging.Handler() {
@Override
public void publish(java.util.logging.LogRecord record) {
records.add(record);
}

@Override
public void flush() {}

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

try {
String url =
"jdbc:bigquery://https://www.googleapis.com/bigquery/v2:443;"
+ "OAuthType=2;ProjectId=MyTestProjectId;"
+ "OAuthAccessToken=secretAccessToken;Location=US;"
+ "PartnerToken=GPN:secretPartnerToken;";
try (BigQueryConnection connection = new BigQueryConnection(url)) {
// Just trigger the constructor
}

boolean foundLog = false;
for (java.util.logging.LogRecord record : records) {
if (record.getMessage().contains("Connection properties:")) {
foundLog = true;
String logMessage = record.getMessage();
assertTrue(logMessage.contains("ProjectId=MyTestProjectId"));
assertTrue(logMessage.contains("Location=US"));
assertTrue(logMessage.contains("OAuthAccessToken=*****"));
assertTrue(logMessage.contains("PartnerToken= (GPN:secretPartnerToken)"));
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 20, 2026

Choose a reason for hiding this comment

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

high

This assertion appears to be incorrect. Given the URL PartnerToken=GPN:secretPartnerToken, the DataSource constructor sets this.partnerToken to GPN:secretPartnerToken. Subsequently, createProperties() in DataSource constructs the property value as " (GPN:" + this.partnerToken + ")", which results in " (GPN:GPN:secretPartnerToken)". The log message will therefore contain PartnerToken= (GPN:GPN:secretPartnerToken). Please update the assertion to reflect the actual behavior.

Suggested change
assertTrue(logMessage.contains("PartnerToken= (GPN:secretPartnerToken)"));
assertTrue(logMessage.contains("PartnerToken= (GPN:GPN:secretPartnerToken)"));

assertFalse(logMessage.contains("secretAccessToken"));
}
}
assertTrue(foundLog, "Log message about Connection properties was not found");
} finally {
rootLogger.removeHandler(handler);
rootLogger.setLevel(originalLevel);
}
}
}
Loading