-
Notifications
You must be signed in to change notification settings - Fork 427
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
Added connection properties to specify custom SocketFactory #1217
Conversation
Is this PR for a particular issue or is it just an overall improvement suggestion? |
I haven't opened a separate issue for it but I can if it helps track things. I'd like to be able to customize the Without this PR there is no way to do so as the driver issues a FYI, it's a common option on most Java drivers (both JDBC and non-JDBC):
|
Just a quick update to this PR. The team will need some time to review the contents of this PR, but it will probably start no earlier than early February as our bandwidth is currently limited. |
Adds a Util.newInstance(...) helper to instantiate a class by name with an optional argument and replaces similar code to create a custom TrustManager with the new helper.
Adds a connection property, socketFactoryClass, to override the SocketFactory used to create Sockets for driver connections. If unspecified, the default, then the system default SocketFactory.getDefault() will be used. If a class name is specified then it must be assignable to javax.net.SocketFactory. An optional argument can be specified via the property socketFactoryConstructorArg. If specified then the single String argument constructor of the class will be invoked. Otherwise the no-argument constructor will be invoked.
b66f063
to
4c40a55
Compare
@rene-ye Thanks for the update. |
Rebased to resolve some merge conflicts on dev, fixed a typo, and added one more test for invalid class values (similar to the one for the trust manager property). |
f051f95
…at-add-custom-socket-factory # Conflicts: # src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java
…ehrope/mssql-jdbc into feat-add-custom-socket-factory
Hi @sehrope , |
Nice. Thanks for the review and merging. |
Hi. @sehrope
Thanks a lot. |
Adds a new connection property,
socketFactoryClass
, to allow overriding the SocketFactory used to create Sockets. Similar to the custom trust manager, there's also an optional string property,socketFactorConstructorArg
, to pass as a constructor arg to further customize the SocketFactory. If not specified then the system default viaSocketFactory.getDefault()
is used.First commit adds an internal helper for instantiating custom class instances as the logic is the same as for the custom trust manager.
Second commit has the new feature and adds a test that configures a basic SocketFactory that keeps logs its usage (to test that it's actually being used).
The SocketFactory is created once for each connection and cached in private variable. It may be used multiple times if the driver attempts to concurrently open sockets (ex: via multiple address resolution for the DB host). My reading of that code suggests that no thread synchronization is required as the sockets are created by the parent prior to executing in their own threads. Would be good for another set of eyes to confirm: https://github.com/microsoft/mssql-jdbc/blob/dev/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java#L2638-L2654