Skip to content
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.

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

feat: enable setting ipType configuration option for SQL Server connector #936

Merged
merged 8 commits into from
Dec 7, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,22 @@
package com.google.cloud.sql.sqlserver;

import com.google.cloud.sql.core.CoreSocketFactory;
import com.google.common.annotations.VisibleForTesting;
import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.net.InetAddress;
import java.net.Socket;
import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
import java.util.Properties;
import java.util.logging.Logger;

public class SocketFactory extends javax.net.SocketFactory {

private static final Logger logger = Logger.getLogger(SocketFactory.class.getName());
private Properties props = new Properties();
// props are protected, not private, so that they can be accessed from unit tests
@VisibleForTesting
protected Properties props = new Properties();
Copy link
Contributor

Choose a reason for hiding this comment

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


static {
CoreSocketFactory.addArtifactId("cloud-sql-connector-jdbc-sqlserver");
Expand All @@ -36,8 +42,25 @@ public class SocketFactory extends javax.net.SocketFactory {
* Implements the {@link SocketFactory} constructor, which can be used to create authenticated
* connections to a Cloud SQL instance.
*/
public SocketFactory(String instanceName) {
this.props.setProperty(CoreSocketFactory.CLOUD_SQL_INSTANCE_PROPERTY, instanceName);
public SocketFactory(String socketFactoryConstructorArg)
throws UnsupportedEncodingException {
String[] s = socketFactoryConstructorArg.split("\\?");
Copy link
Collaborator

Choose a reason for hiding this comment

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

You may want to use String.indexOf('?') instead of String.split(). Customer may include more than one '?' characters and you don't really want to split on all '?', just the first one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What would be the use case for more than one ? character? Wouldn't that be a malformed string (which we should probably catch with an error). so far the only allowed args are true/false for enableIamAuth and public/private for ipType.

Copy link
Contributor

@kurtisvg kurtisvg Aug 9, 2022

Choose a reason for hiding this comment

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

Can we use ParseURI?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we use ParseURI?

Unfortunately not - I tried it and the : character in the instance name causes issues, and we can't make users escape the instance name without breaking existing users (see check failures in 53b5936)

this.props.setProperty(CoreSocketFactory.CLOUD_SQL_INSTANCE_PROPERTY, s[0]);
if (s.length == 2 && s[1].length() > 0) {
String[] queryParams = s[1].split("&");
hessjcg marked this conversation as resolved.
Show resolved Hide resolved
for (String param : queryParams) {
String[] splitParam = param.split("=");
if (splitParam.length != 2 || splitParam[0].length() == 0 || splitParam[1].length() == 0) {
throw new IllegalArgumentException(String.format(
"Malformed query param in socketFactoryConstructorArg : %s", param));
}
this.props.setProperty(URLDecoder.decode(splitParam[0], StandardCharsets.UTF_8.name()),
URLDecoder.decode(splitParam[1], StandardCharsets.UTF_8.name()));
}
} else if (s.length > 2) {
throw new IllegalArgumentException(
"Only one query string allowed in socketFactoryConstructorArg");
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.List;
import java.util.Properties;
import java.util.UUID;
import java.util.concurrent.TimeUnit;
import org.junit.After;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/*
* Copyright 2022 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
*
* https://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.sql.sqlserver;

import static com.google.common.truth.Truth.assertThat;

import com.google.cloud.sql.core.CoreSocketFactory;
import java.io.UnsupportedEncodingException;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

@RunWith(JUnit4.class)
public class JdbcSqlServerUnitTests {

private static final String CONNECTION_NAME = "my-projectmy-regionmy-instance";

@Test
public void checkConnectionStringNoQueryParams()
throws UnsupportedEncodingException {
String socketFactoryConstructorArg = CONNECTION_NAME;
SocketFactory socketFactory = new SocketFactory(socketFactoryConstructorArg);
assertThat(socketFactory.props.get(CoreSocketFactory.CLOUD_SQL_INSTANCE_PROPERTY)).isEqualTo(
CONNECTION_NAME);
}

@Test
public void checkConnectionStringWithQueryParam()
throws UnsupportedEncodingException {
String socketFactoryConstructorArg = String.format("%s?%s=%s", CONNECTION_NAME, "ipTypes",
"PRIVATE");
SocketFactory socketFactory = new SocketFactory(socketFactoryConstructorArg);
assertThat(socketFactory.props.get(CoreSocketFactory.CLOUD_SQL_INSTANCE_PROPERTY)).isEqualTo(
CONNECTION_NAME);
assertThat(socketFactory.props.get("ipTypes")).isEqualTo(
"PRIVATE");
}

hessjcg marked this conversation as resolved.
Show resolved Hide resolved
@Test
public void checkConnectionStringWithEmptyQueryParam()
throws UnsupportedEncodingException {
String socketFactoryConstructorArg = String.format("%s?", CONNECTION_NAME);
SocketFactory socketFactory = new SocketFactory(socketFactoryConstructorArg);
assertThat(socketFactory.props.get(CoreSocketFactory.CLOUD_SQL_INSTANCE_PROPERTY)).isEqualTo(
CONNECTION_NAME);
assertThat(socketFactory.props.get("ipTypes")).isEqualTo(
null);
}

@Test
public void checkConnectionStringWithUrlEncodedParam()
throws UnsupportedEncodingException {
String socketFactoryConstructorArg = String.format("%s?token=%s", CONNECTION_NAME,
"abc%20def%20xyz%2F%26%3D");
SocketFactory socketFactory = new SocketFactory(socketFactoryConstructorArg);
assertThat(socketFactory.props.get(CoreSocketFactory.CLOUD_SQL_INSTANCE_PROPERTY)).isEqualTo(
CONNECTION_NAME);
assertThat(socketFactory.props.get("token")).isEqualTo(
"abc def xyz/&=");
}

@Test(expected = IllegalArgumentException.class)
public void checkConnectionStringWithParamMissingKey()
throws UnsupportedEncodingException {
String socketFactoryConstructorArg = String.format("%s?=%s", CONNECTION_NAME, "PRIVATE");
new SocketFactory(socketFactoryConstructorArg);
}

@Test(expected = IllegalArgumentException.class)
public void checkConnectionStringWithParamMissingValue()
throws UnsupportedEncodingException {
String socketFactoryConstructorArg = String.format("%s?enableIamAuth=true&%s", CONNECTION_NAME,
"ipTypes");
new SocketFactory(socketFactoryConstructorArg);
}

}