You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by "jmuehlner (via GitHub)" <gi...@apache.org> on 2023/05/11 00:16:33 UTC

[GitHub] [guacamole-client] jmuehlner opened a new pull request, #862: GUACAMOLE-926: Fix SQL Server JDBC auth extension by disabling batch executor.

jmuehlner opened a new pull request, #862:
URL: https://github.com/apache/guacamole-client/pull/862

   Turns out SQL Server didn't work at all after my changes in https://github.com/apache/guacamole-client/pull/809. 
   
   Login attempts fail with the following error:
   ```
   Error committing transaction.  
   Cause: org.apache.ibatis.executor.ExecutorException: Error getting generated key or setting result to parameter object. 
   Cause: com.microsoft.sqlserver.jdbc.SQLServerException: The statement must be executed before any results can be obtained.
   ```
   
   This change disables the batch executor entirely for SQL Server and adds a warning to that effect.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [guacamole-client] jmuehlner commented on a diff in pull request #862: GUACAMOLE-926: Fix SQL Server JDBC auth extension by disabling batch executor.

Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on code in PR #862:
URL: https://github.com/apache/guacamole-client/pull/862#discussion_r1190493677


##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/conf/SQLServerEnvironment.java:
##########
@@ -295,4 +295,36 @@ public boolean enforceAccessWindowsForActiveSessions() throws GuacamoleException
                 true);
     }
 
+    @Override
+    public boolean shouldUseBatchExecutor() {

Review Comment:
   This change isn't required to fix the problem with the batch executor, but it is awfully handy for testing using self-hosted SQL Server instances with self-signed certificates.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [guacamole-client] mike-jumper merged pull request #862: GUACAMOLE-926: Fix SQL Server JDBC auth extension by disabling batch executor.

Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper merged PR #862:
URL: https://github.com/apache/guacamole-client/pull/862


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [guacamole-client] jmuehlner commented on a diff in pull request #862: GUACAMOLE-926: Fix SQL Server JDBC auth extension by disabling batch executor.

Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on code in PR #862:
URL: https://github.com/apache/guacamole-client/pull/862#discussion_r1190493780


##########
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/conf/SQLServerEnvironment.java:
##########
@@ -295,4 +295,36 @@ public boolean enforceAccessWindowsForActiveSessions() throws GuacamoleException
                 true);
     }
 
+    @Override
+    public boolean shouldUseBatchExecutor() {
+
+        // The SQL Server driver does not work when batch execution is enabled.
+        // Specifically, inserts fail with com.microsoft.sqlserver.jdbc.SQLServerException:
+        // The statement must be executed before any results can be obtained.
+        // See https://github.com/microsoft/mssql-jdbc/issues/358 for more.
+        logger.warn(
+                "JDBC batch executor is disabled for SQL Server Connections. "
+                + "Large batched updates may run slower."
+        );
+        return false;
+        
+    }
+
+    /**
+     * Returns true if all server certificates should be trusted, including
+     * those signed by an unknown certificate authority, such as self-signed
+     * certificates, or false otherwise.
+     *
+     * @throws GuacamoleException
+     *     If an error occurs while retrieving the property value, or if the
+     *     value was not set, as this property is required.
+     */
+    public boolean trustAllServerCertificates() throws GuacamoleException {

Review Comment:
   This isn't required to fix the problem with the batch executor, but it is awfully handy for testing using self-hosted SQL Server instances with self-signed certificates.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org