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

[BUG]loginTimeout in URL not valid if DriverManager.setLoginTimeout #1048

Closed
slggamerTrue opened this issue Apr 25, 2019 · 5 comments · Fixed by #1049
Closed

[BUG]loginTimeout in URL not valid if DriverManager.setLoginTimeout #1048

slggamerTrue opened this issue Apr 25, 2019 · 5 comments · Fixed by #1049
Labels
Bug A bug in the driver. A high priority item that one can expect to be addressed quickly.

Comments

@slggamerTrue
Copy link

Driver version

6.2.2

SQL Server version

Any

Client Operating System

Linux or Mac or Windows

JAVA/JVM version

1.8

Table schema

No need

Problem description

When I set logintimeout in URL, I expect that the connection would timeout by the setting in URL. But it would timeout by the global loginTimeout setting.

And the problem is caused by the loginTimeout property is overwritten in SQLServerDriver.parseAndMergeProperties
image

Reproduction code

With the following code, the connection would timeout in 60 seconds not 5 seconds.
DriverManager.setLoginTimeout(60)
println(new Date().toString())
try {
url = "jdbc:sqlserver://10.0.2.15;userName=sa;password=PASSW0RD;database=master;socketTimeout=10000;logintimeout=5"
def sql = Sql.newInstance(url, "system", "123456", "com.microsoft.sqlserver.jdbc.SQLServerDriver")
}
catch (Exception e) {
println(e.getMessage())
println(new Date().toString())
}

@slggamerTrue slggamerTrue added the Bug A bug in the driver. A high priority item that one can expect to be addressed quickly. label Apr 25, 2019
@cheenamalhotra
Copy link
Member

Hi @slggamerTrue

I can confirm this is a valid issue in the driver, and the Login Timeout property value must not be overridden if it does not exceed global Max Timeout value (returned by DriverManager.getLoginTimeout()).

I've created PR #1049 to fix the issue, please take a look and let me know if works well for you.

@slggamerTrue
Copy link
Author

Hi @cheenamalhotra
I think taking DriverManager.getLoginTimeout() as Max Timeout value is a little confusing. Since it's not DriverManager.setMaxLoginTimeout().
And the behavior of setting loginTimeout in URL and using SQLServerDataSource.setLoginTimeout(int) is inconsistent.

Do you think if we can use DriverManger.getLoginTimeout() when there is no LoginTimeout property? Thanks
int currentTimeout = Integer.valueOf(connectProperties.getProperty(SQLServerDriverIntProperty.LOGIN_TIMEOUT.toString(), "0"));
int globalTimeout = DriverManager.getLoginTimeout();
if (globalTimeout > 0 && currentTimeout <= 0) {
connectProperties.setProperty(SQLServerDriverIntProperty.LOGIN_TIMEOUT.toString(),
String.valueOf(globalTimeout));
}

@cheenamalhotra
Copy link
Member

@slggamerTrue

According to Java Specs, setLoginTimeout() and getLoginTimeout() are properties for "maximum time in seconds that a driver can wait when attempting to log in to a database."

There is no alternative or "setMaxLoginTimeout()" API available in DriverManager, and hence the driver must respect Java specifications.

About the default value of this property, as mentioned in Microsoft Docs, the default value for this connection property is 15 seconds and will not change.

@slggamerTrue
Copy link
Author

Hi @cheenamalhotra
In my opinion, the "maximum time" in Java Specs means the maximum waiting time before throw an exception. It's not a global maximum timeout setting. For example, the MySql and Postgresql have "connectTimeout" in URL which can be larger than DriverManager.setLoginTimeout(int).
The "connectTimeout" is the LoginTimeout from the code
image

The current fix is fine to me for I won't set LoginTimeout larger than DriverManager.setLoginTimeout. But I'm worried that others may be confusing that why loginTimeout in URL is not valid if it's larger than a specific value when they forget they've set the DriverManager.setLoginTimeout somewhere.

@cheenamalhotra
Copy link
Member

cheenamalhotra commented Apr 29, 2019

Hi @slggamerTrue

I verified the behavior with other JDBC drivers and seems like the DriverManager.loginTimeout value is applied only when it is set by user (and value > 0) and connection property is not provided. It however is not treated as "Max Timeout".

I fixed my PR accordingly, please take a look.
Driver's default login timeout stays 15 seconds, it won't change.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in the driver. A high priority item that one can expect to be addressed quickly.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants