From 34130f54c9c3f3b6b269d775475fbfe9901b98bf Mon Sep 17 00:00:00 2001 From: Douglas Heriot Date: Mon, 23 Mar 2020 21:23:56 +1100 Subject: [PATCH 1/6] GUACAMOLE-919: pass through defaultStatementTimeout Testing a workaround for handling dropped TCP connections to the Postgres database. --- ...ostgreSQLAuthenticationProviderModule.java | 1 + .../conf/PostgreSQLEnvironment.java | 25 +++++++++++++++++++ .../conf/PostgreSQLGuacamoleProperties.java | 12 +++++++++ guacamole-docker/bin/start.sh | 4 +++ 4 files changed, 42 insertions(+) diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/PostgreSQLAuthenticationProviderModule.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/PostgreSQLAuthenticationProviderModule.java index 331707c38..03339a3f8 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/PostgreSQLAuthenticationProviderModule.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/PostgreSQLAuthenticationProviderModule.java @@ -69,6 +69,7 @@ public class PostgreSQLAuthenticationProviderModule implements Module { myBatisProperties.setProperty("JDBC.autoCommit", "false"); myBatisProperties.setProperty("mybatis.pooled.pingEnabled", "true"); myBatisProperties.setProperty("mybatis.pooled.pingQuery", "SELECT 1"); + myBatisProperties.setProperty("mybatis.configuration.defaultStatementTimeout", environment.getPostgreSQLDefaultStatementTimeout()); // Use UTF-8 in database driverProperties.setProperty("characterEncoding", "UTF-8"); diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLEnvironment.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLEnvironment.java index e81e6949e..8142d9c04 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLEnvironment.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLEnvironment.java @@ -48,6 +48,14 @@ public class PostgreSQLEnvironment extends JDBCEnvironment { */ private static final int DEFAULT_PORT = 5432; + /** + * The default defaultStatementTimeout (in seconds), + * if POSTGRESQL_DEFAULT_STATEMENT_TIMEOUT is not specified. + * Default to null (no timeout) + * https://mybatis.org/mybatis-3/configuration.html + */ + private static final String DEFAULT_DEFAULT_STATEMENT_TIMEOUT = "null"; + /** * Whether a database user account is required by default for authentication * to succeed. @@ -249,6 +257,23 @@ public class PostgreSQLEnvironment extends JDBCEnvironment { public String getPostgreSQLPassword() throws GuacamoleException { return getRequiredProperty(PostgreSQLGuacamoleProperties.POSTGRESQL_PASSWORD); } + + /** + * Returns the defaultStatementTimeout set for PostgreSQL connections. + * If unspecified, this will be the default "null" (no timeout) + * + * @return + * The statement timeout (in seconds) + * + * @throws GuacamoleException + * If an error occurs while retrieving the property value. + */ + public String getPostgreSQLDefaultStatementTimeout() throws GuacamoleException { + return getProperty( + PostgreSQLGuacamoleProperties.POSTGRESQL_DEFAULT_STATEMENT_TIMEOUT, + DEFAULT_DEFAULT_STATEMENT_TIMEOUT + ); + } @Override public boolean isRecursiveQuerySupported(SqlSession session) { diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLGuacamoleProperties.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLGuacamoleProperties.java index d2ae2532c..3295d7775 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLGuacamoleProperties.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLGuacamoleProperties.java @@ -94,6 +94,18 @@ public class PostgreSQLGuacamoleProperties { }; + /** + * Sets the number of seconds the driver will wait for + * a response from the database. + */ + public static final StringGuacamoleProperty + POSTGRESQL_DEFAULT_STATEMENT_TIMEOUT = new StringGuacamoleProperty(){ + + @Override + public String getName() { return "postgresql-default-statement-timeout"; } + + }; + /** * Whether a user account within the database is required for authentication * to succeed, even if the user has been authenticated via another diff --git a/guacamole-docker/bin/start.sh b/guacamole-docker/bin/start.sh index 62f9496dd..d661d1a23 100755 --- a/guacamole-docker/bin/start.sh +++ b/guacamole-docker/bin/start.sh @@ -354,6 +354,10 @@ END "postgresql-default-max-group-connections-per-user" \ "$POSTGRES_DEFAULT_MAX_GROUP_CONNECTIONS_PER_USER" + set_optional_property \ + "postgresql-default-statement-timeout" \ + "$POSTGRES_DEFAULT_STATEMENT_TIMEOUT" + set_optional_property \ "postgresql-user-required" \ "$POSTGRES_USER_REQUIRED" From 306e1ad3abd73a5c1e6def72bda83f4513917bee Mon Sep 17 00:00:00 2001 From: Douglas Heriot Date: Mon, 23 Mar 2020 21:52:29 +1100 Subject: [PATCH 2/6] GUACAMOLE-919: pass postgres driver socketTimout --- ...ostgreSQLAuthenticationProviderModule.java | 3 +++ .../conf/PostgreSQLEnvironment.java | 26 ++++++++++++++++++- .../conf/PostgreSQLGuacamoleProperties.java | 12 +++++++++ guacamole-docker/bin/start.sh | 4 +++ 4 files changed, 44 insertions(+), 1 deletion(-) diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/PostgreSQLAuthenticationProviderModule.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/PostgreSQLAuthenticationProviderModule.java index 03339a3f8..aa331c2e2 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/PostgreSQLAuthenticationProviderModule.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/PostgreSQLAuthenticationProviderModule.java @@ -111,6 +111,9 @@ public class PostgreSQLAuthenticationProviderModule implements Module { } + // Handle case where TCP connection to database is silently dropped + driverProperties.setProperty("socketTimeout", String.valueOf(environment.getPostgreSQLSocketTimeout())); + } @Override diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLEnvironment.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLEnvironment.java index 8142d9c04..d7e4daeee 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLEnvironment.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLEnvironment.java @@ -56,6 +56,13 @@ public class PostgreSQLEnvironment extends JDBCEnvironment { */ private static final String DEFAULT_DEFAULT_STATEMENT_TIMEOUT = "null"; + /** + * The default socketTimeout (in seconds), if POSTGRESQL_SOCKET_TIMEOUT is not specified. + * Default to 0 (no timeout) + * https://jdbc.postgresql.org/documentation/head/connect.html + */ + private static final int DEFAULT_SOCKET_TIMEOUT = 0; + /** * Whether a database user account is required by default for authentication * to succeed. @@ -257,7 +264,7 @@ public class PostgreSQLEnvironment extends JDBCEnvironment { public String getPostgreSQLPassword() throws GuacamoleException { return getRequiredProperty(PostgreSQLGuacamoleProperties.POSTGRESQL_PASSWORD); } - + /** * Returns the defaultStatementTimeout set for PostgreSQL connections. * If unspecified, this will be the default "null" (no timeout) @@ -274,6 +281,23 @@ public class PostgreSQLEnvironment extends JDBCEnvironment { DEFAULT_DEFAULT_STATEMENT_TIMEOUT ); } + + /** + * Returns the socketTimeout property to set on PostgreSQL connections. + * If unspecified, this will be the default to 0 (no timeout) + * + * @return + * The socketTimeout to use when waiting on read operations (in seconds) + * + * @throws GuacamoleException + * If an error occurs while retrieving the property value. + */ + public int getPostgreSQLSocketTimeout() throws GuacamoleException { + return getProperty( + PostgreSQLGuacamoleProperties.POSTGRESQL_SOCKET_TIMEOUT, + DEFAULT_SOCKET_TIMEOUT + ); + } @Override public boolean isRecursiveQuerySupported(SqlSession session) { diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLGuacamoleProperties.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLGuacamoleProperties.java index 3295d7775..128b5c348 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLGuacamoleProperties.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLGuacamoleProperties.java @@ -106,6 +106,18 @@ public class PostgreSQLGuacamoleProperties { }; + /** + * Sets the number of seconds the driver will wait in a read() call + * on the TCP connection to the database. + */ + public static final IntegerGuacamoleProperty + POSTGRESQL_SOCKET_TIMEOUT = new IntegerGuacamoleProperty(){ + + @Override + public String getName() { return "postgresql-socket-timeout"; } + + }; + /** * Whether a user account within the database is required for authentication * to succeed, even if the user has been authenticated via another diff --git a/guacamole-docker/bin/start.sh b/guacamole-docker/bin/start.sh index d661d1a23..6bc6ee8fb 100755 --- a/guacamole-docker/bin/start.sh +++ b/guacamole-docker/bin/start.sh @@ -362,6 +362,10 @@ END "postgresql-user-required" \ "$POSTGRES_USER_REQUIRED" + set_optional_property \ + "postgresql-socket-timeout" \ + "$POSTGRES_SOCKET_TIMEOUT" + set_optional_property \ "postgresql-ssl-mode" \ "$POSTGRESQL_SSL_MODE" From 0528ca0564d0010000ffd0d068bed0f19d1ca408 Mon Sep 17 00:00:00 2001 From: Douglas Heriot Date: Thu, 2 Apr 2020 12:08:00 +1100 Subject: [PATCH 3/6] GUACAMOLE-919: make defaultStatementTimeout an int More consistent with how socketTimeout. MyBatis documentation says only positive integers are valid, and property should not be set otherwise. So in the case of 0 the property will not be set. --- .../PostgreSQLAuthenticationProviderModule.java | 7 ++++++- .../auth/postgresql/conf/PostgreSQLEnvironment.java | 9 +++++---- .../postgresql/conf/PostgreSQLGuacamoleProperties.java | 4 ++-- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/PostgreSQLAuthenticationProviderModule.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/PostgreSQLAuthenticationProviderModule.java index aa331c2e2..5241e1276 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/PostgreSQLAuthenticationProviderModule.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/PostgreSQLAuthenticationProviderModule.java @@ -69,7 +69,12 @@ public class PostgreSQLAuthenticationProviderModule implements Module { myBatisProperties.setProperty("JDBC.autoCommit", "false"); myBatisProperties.setProperty("mybatis.pooled.pingEnabled", "true"); myBatisProperties.setProperty("mybatis.pooled.pingQuery", "SELECT 1"); - myBatisProperties.setProperty("mybatis.configuration.defaultStatementTimeout", environment.getPostgreSQLDefaultStatementTimeout()); + + // Only set if > 0. Underlying backend does not take 0 as not-set. + int defaultStatementTimeout = environment.getPostgreSQLDefaultStatementTimeout(); + if(defaultStatementTimeout > 0) { + myBatisProperties.setProperty("mybatis.configuration.defaultStatementTimeout", String.valueOf(defaultStatementTimeout)); + } // Use UTF-8 in database driverProperties.setProperty("characterEncoding", "UTF-8"); diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLEnvironment.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLEnvironment.java index d7e4daeee..7ce4795ce 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLEnvironment.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLEnvironment.java @@ -51,10 +51,10 @@ public class PostgreSQLEnvironment extends JDBCEnvironment { /** * The default defaultStatementTimeout (in seconds), * if POSTGRESQL_DEFAULT_STATEMENT_TIMEOUT is not specified. - * Default to null (no timeout) + * Default to 0 (no timeout, property won't be set) * https://mybatis.org/mybatis-3/configuration.html */ - private static final String DEFAULT_DEFAULT_STATEMENT_TIMEOUT = "null"; + private static final int DEFAULT_DEFAULT_STATEMENT_TIMEOUT = 0; /** * The default socketTimeout (in seconds), if POSTGRESQL_SOCKET_TIMEOUT is not specified. @@ -267,7 +267,8 @@ public class PostgreSQLEnvironment extends JDBCEnvironment { /** * Returns the defaultStatementTimeout set for PostgreSQL connections. - * If unspecified, this will be the default "null" (no timeout) + * If unspecified, this will be the default 0, + * and should not be passed through to the backend. * * @return * The statement timeout (in seconds) @@ -275,7 +276,7 @@ public class PostgreSQLEnvironment extends JDBCEnvironment { * @throws GuacamoleException * If an error occurs while retrieving the property value. */ - public String getPostgreSQLDefaultStatementTimeout() throws GuacamoleException { + public int getPostgreSQLDefaultStatementTimeout() throws GuacamoleException { return getProperty( PostgreSQLGuacamoleProperties.POSTGRESQL_DEFAULT_STATEMENT_TIMEOUT, DEFAULT_DEFAULT_STATEMENT_TIMEOUT diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLGuacamoleProperties.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLGuacamoleProperties.java index 128b5c348..feaf85536 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLGuacamoleProperties.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLGuacamoleProperties.java @@ -98,8 +98,8 @@ public class PostgreSQLGuacamoleProperties { * Sets the number of seconds the driver will wait for * a response from the database. */ - public static final StringGuacamoleProperty - POSTGRESQL_DEFAULT_STATEMENT_TIMEOUT = new StringGuacamoleProperty(){ + public static final IntegerGuacamoleProperty + POSTGRESQL_DEFAULT_STATEMENT_TIMEOUT = new IntegerGuacamoleProperty(){ @Override public String getName() { return "postgresql-default-statement-timeout"; } From 6cf9787f7be9408909c18630175ab4e2ff370ebf Mon Sep 17 00:00:00 2001 From: Douglas Heriot Date: Wed, 8 Jul 2020 14:02:46 +1000 Subject: [PATCH 4/6] GUACAMOLE-919: Fix style issues, improve comments. --- ...ostgreSQLAuthenticationProviderModule.java | 2 +- .../conf/PostgreSQLEnvironment.java | 19 ++++++++++--------- .../conf/PostgreSQLGuacamoleProperties.java | 4 ++-- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/PostgreSQLAuthenticationProviderModule.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/PostgreSQLAuthenticationProviderModule.java index 5241e1276..0ec44cd78 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/PostgreSQLAuthenticationProviderModule.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/PostgreSQLAuthenticationProviderModule.java @@ -72,7 +72,7 @@ public class PostgreSQLAuthenticationProviderModule implements Module { // Only set if > 0. Underlying backend does not take 0 as not-set. int defaultStatementTimeout = environment.getPostgreSQLDefaultStatementTimeout(); - if(defaultStatementTimeout > 0) { + if (defaultStatementTimeout > 0) { myBatisProperties.setProperty("mybatis.configuration.defaultStatementTimeout", String.valueOf(defaultStatementTimeout)); } diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLEnvironment.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLEnvironment.java index 7ce4795ce..ba77c2608 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLEnvironment.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLEnvironment.java @@ -49,17 +49,18 @@ public class PostgreSQLEnvironment extends JDBCEnvironment { private static final int DEFAULT_PORT = 5432; /** - * The default defaultStatementTimeout (in seconds), - * if POSTGRESQL_DEFAULT_STATEMENT_TIMEOUT is not specified. - * Default to 0 (no timeout, property won't be set) - * https://mybatis.org/mybatis-3/configuration.html + * The default number of seconds the driver will wait for a response from + * the database. A value of 0 (the default) means the timeout is disabled. */ - private static final int DEFAULT_DEFAULT_STATEMENT_TIMEOUT = 0; + private static final int DEFAULT_STATEMENT_TIMEOUT = 0; /** - * The default socketTimeout (in seconds), if POSTGRESQL_SOCKET_TIMEOUT is not specified. - * Default to 0 (no timeout) - * https://jdbc.postgresql.org/documentation/head/connect.html + * The default timeout (in seconds) used for socket read operations. + * If reading from the server takes longer than this value, the + * connection is closed. This can be used to handle network problems + * such as a dropped connection to the database. Similar to + * DEFAULT_STATEMENT_TIMEOUT, it will also abort queries that take too + * long. The value 0 (the default) means the timeout is disabled. */ private static final int DEFAULT_SOCKET_TIMEOUT = 0; @@ -279,7 +280,7 @@ public class PostgreSQLEnvironment extends JDBCEnvironment { public int getPostgreSQLDefaultStatementTimeout() throws GuacamoleException { return getProperty( PostgreSQLGuacamoleProperties.POSTGRESQL_DEFAULT_STATEMENT_TIMEOUT, - DEFAULT_DEFAULT_STATEMENT_TIMEOUT + DEFAULT_STATEMENT_TIMEOUT ); } diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLGuacamoleProperties.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLGuacamoleProperties.java index feaf85536..2feee8085 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLGuacamoleProperties.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLGuacamoleProperties.java @@ -95,7 +95,7 @@ public class PostgreSQLGuacamoleProperties { }; /** - * Sets the number of seconds the driver will wait for + * The number of seconds the driver will wait for * a response from the database. */ public static final IntegerGuacamoleProperty @@ -107,7 +107,7 @@ public class PostgreSQLGuacamoleProperties { }; /** - * Sets the number of seconds the driver will wait in a read() call + * The number of seconds the driver will wait in a read() call * on the TCP connection to the database. */ public static final IntegerGuacamoleProperty From 20b1dbf00f806146789ce2f04d17e44e79c1f25d Mon Sep 17 00:00:00 2001 From: Douglas Heriot Date: Wed, 15 Jul 2020 18:46:45 +1000 Subject: [PATCH 5/6] GUACAMOLE-919: More updates to style and comments. --- ...PostgreSQLAuthenticationProviderModule.java | 10 ++++++++-- .../postgresql/conf/PostgreSQLEnvironment.java | 15 ++++++--------- .../conf/PostgreSQLGuacamoleProperties.java | 18 ++++++++++++------ 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/PostgreSQLAuthenticationProviderModule.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/PostgreSQLAuthenticationProviderModule.java index 0ec44cd78..280cead71 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/PostgreSQLAuthenticationProviderModule.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/PostgreSQLAuthenticationProviderModule.java @@ -73,7 +73,10 @@ public class PostgreSQLAuthenticationProviderModule implements Module { // Only set if > 0. Underlying backend does not take 0 as not-set. int defaultStatementTimeout = environment.getPostgreSQLDefaultStatementTimeout(); if (defaultStatementTimeout > 0) { - myBatisProperties.setProperty("mybatis.configuration.defaultStatementTimeout", String.valueOf(defaultStatementTimeout)); + myBatisProperties.setProperty( + "mybatis.configuration.defaultStatementTimeout", + String.valueOf(defaultStatementTimeout) + ); } // Use UTF-8 in database @@ -117,7 +120,10 @@ public class PostgreSQLAuthenticationProviderModule implements Module { } // Handle case where TCP connection to database is silently dropped - driverProperties.setProperty("socketTimeout", String.valueOf(environment.getPostgreSQLSocketTimeout())); + driverProperties.setProperty( + "socketTimeout", + String.valueOf(environment.getPostgreSQLSocketTimeout()) + ); } diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLEnvironment.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLEnvironment.java index ba77c2608..012877cf6 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLEnvironment.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLEnvironment.java @@ -50,17 +50,14 @@ public class PostgreSQLEnvironment extends JDBCEnvironment { /** * The default number of seconds the driver will wait for a response from - * the database. A value of 0 (the default) means the timeout is disabled. + * the database, before aborting the query. + * A value of 0 (the default) means the timeout is disabled. */ private static final int DEFAULT_STATEMENT_TIMEOUT = 0; /** - * The default timeout (in seconds) used for socket read operations. - * If reading from the server takes longer than this value, the - * connection is closed. This can be used to handle network problems - * such as a dropped connection to the database. Similar to - * DEFAULT_STATEMENT_TIMEOUT, it will also abort queries that take too - * long. The value 0 (the default) means the timeout is disabled. + * The default number of seconds to wait for socket read operations. + * A value of 0 (the default) means the timeout is disabled. */ private static final int DEFAULT_SOCKET_TIMEOUT = 0; @@ -268,7 +265,7 @@ public class PostgreSQLEnvironment extends JDBCEnvironment { /** * Returns the defaultStatementTimeout set for PostgreSQL connections. - * If unspecified, this will be the default 0, + * If unspecified, this will default to 0, * and should not be passed through to the backend. * * @return @@ -286,7 +283,7 @@ public class PostgreSQLEnvironment extends JDBCEnvironment { /** * Returns the socketTimeout property to set on PostgreSQL connections. - * If unspecified, this will be the default to 0 (no timeout) + * If unspecified, this will default to 0 (no timeout) * * @return * The socketTimeout to use when waiting on read operations (in seconds) diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLGuacamoleProperties.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLGuacamoleProperties.java index 2feee8085..271d9c0dd 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLGuacamoleProperties.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-postgresql/src/main/java/org/apache/guacamole/auth/postgresql/conf/PostgreSQLGuacamoleProperties.java @@ -95,11 +95,12 @@ public class PostgreSQLGuacamoleProperties { }; /** - * The number of seconds the driver will wait for - * a response from the database. + * The number of seconds the driver will wait for a response from + * the database, before aborting the query. + * A value of 0 (the default) means the timeout is disabled. */ public static final IntegerGuacamoleProperty - POSTGRESQL_DEFAULT_STATEMENT_TIMEOUT = new IntegerGuacamoleProperty(){ + POSTGRESQL_DEFAULT_STATEMENT_TIMEOUT = new IntegerGuacamoleProperty() { @Override public String getName() { return "postgresql-default-statement-timeout"; } @@ -107,11 +108,16 @@ public class PostgreSQLGuacamoleProperties { }; /** - * The number of seconds the driver will wait in a read() call - * on the TCP connection to the database. + * The number of seconds to wait for socket read operations. + * If reading from the server takes longer than this value, the + * connection will be closed. This can be used to handle network problems + * such as a dropped connection to the database. Similar to + * postgresql-default-statement-timeout, it will have the effect of + * aborting queries that take too long. + * A value of 0 (the default) means the timeout is disabled. */ public static final IntegerGuacamoleProperty - POSTGRESQL_SOCKET_TIMEOUT = new IntegerGuacamoleProperty(){ + POSTGRESQL_SOCKET_TIMEOUT = new IntegerGuacamoleProperty() { @Override public String getName() { return "postgresql-socket-timeout"; } From a1922b6fdb07278604c42867de079f4893bb9954 Mon Sep 17 00:00:00 2001 From: Douglas Heriot Date: Tue, 21 Jul 2020 12:05:46 +1000 Subject: [PATCH 6/6] GUACAMOLE-919: Whitespace fix --- guacamole-docker/bin/start.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/guacamole-docker/bin/start.sh b/guacamole-docker/bin/start.sh index 6bc6ee8fb..cf82987bd 100755 --- a/guacamole-docker/bin/start.sh +++ b/guacamole-docker/bin/start.sh @@ -354,7 +354,7 @@ END "postgresql-default-max-group-connections-per-user" \ "$POSTGRES_DEFAULT_MAX_GROUP_CONNECTIONS_PER_USER" - set_optional_property \ + set_optional_property \ "postgresql-default-statement-timeout" \ "$POSTGRES_DEFAULT_STATEMENT_TIMEOUT" @@ -362,7 +362,7 @@ END "postgresql-user-required" \ "$POSTGRES_USER_REQUIRED" - set_optional_property \ + set_optional_property \ "postgresql-socket-timeout" \ "$POSTGRES_SOCKET_TIMEOUT"