Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/182#discussion_r141254109
--- Diff:
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/SQLServerAuthenticationProviderModule.java
---
@@ -0,0 +1,116 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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
+ *
+ * http://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 org.apache.guacamole.auth.sqlserver;
+
+import com.google.inject.Binder;
+import com.google.inject.Module;
+import com.google.inject.name.Names;
+import java.util.Properties;
+import org.apache.guacamole.GuacamoleException;
+import org.mybatis.guice.datasource.helper.JdbcHelper;
+
+/**
+ * Guice module which configures SQLServer-specific injections.
+ */
+public class SQLServerAuthenticationProviderModule implements Module {
+
+ /**
+ * MyBatis-specific configuration properties.
+ */
+ private final Properties myBatisProperties = new Properties();
+
+ /**
+ * SQLServer-specific driver configuration properties.
+ */
+ private final Properties driverProperties = new Properties();
+
+ /**
+ * Which SQL Server driver should be used.
+ */
+ private SQLServerDriver sqlServerDriver;
+
+ /**
+ * Creates a new SQLServer authentication provider module that
configures
+ * driver and MyBatis properties using the given environment.
+ *
+ * @param environment
+ * The environment to use when configuring MyBatis and the
underlying
+ * JDBC driver.
+ *
+ * @throws GuacamoleException
+ * If a required property is missing, or an error occurs while
parsing
+ * a property.
+ */
+ public SQLServerAuthenticationProviderModule(SQLServerEnvironment
environment)
+ throws GuacamoleException {
+
+ // Set the SQLServer-specific properties for MyBatis.
+ myBatisProperties.setProperty("mybatis.environment.id",
"guacamole");
+ myBatisProperties.setProperty("JDBC.host",
environment.getSQLServerHostname());
+ myBatisProperties.setProperty("JDBC.port",
String.valueOf(environment.getSQLServerPort()));
+ myBatisProperties.setProperty("JDBC.schema",
environment.getSQLServerDatabase());
+ myBatisProperties.setProperty("JDBC.username",
environment.getSQLServerUsername());
+ myBatisProperties.setProperty("JDBC.password",
environment.getSQLServerPassword());
+ myBatisProperties.setProperty("JDBC.autoCommit", "false");
+ myBatisProperties.setProperty("mybatis.pooled.pingEnabled",
"true");
+ myBatisProperties.setProperty("mybatis.pooled.pingQuery", "SELECT
1");
+
+ // Use UTF-8 in database
+ driverProperties.setProperty("characterEncoding", "UTF-8");
+
+ // Capture which driver to use for the connection.
+ this.sqlServerDriver = environment.getSQLServerDriver();
+
+ }
+
+ @Override
+ public void configure(Binder binder) {
+
+ // Bind SQLServer-specific properties with the configured driver.
+ switch(sqlServerDriver) {
+ case JTDS:
+ JdbcHelper.SQL_Server_jTDS.configure(binder);
+ break;
+
+ case DATA_DIRECT:
+ JdbcHelper.SQL_Server_DataDirect.configure(binder);
+ break;
+
+ case MICROSOFT_LEGACY:
+ JdbcHelper.SQL_Server_MS_Driver.configure(binder);
+ break;
+
+ case MICROSOFT_2005:
+ default:
--- End diff --
Since the `default` case will only match if we add a new SQL Server driver
type without actually implementing that type, silently failing over to the 2005
driver is dangerous behavior. I would recommend either:
1. Bailing out with a hard and unmistakable
[`UnsupportedOperationException`](https://docs.oracle.com/javase/7/docs/api/java/lang/UnsupportedOperationException.html).
2. Documenting for humans and compilers that this condition is expected to
be impossible through an `assert(false)`.
Example of the above:
https://github.com/apache/incubator-guacamole-client/blob/1c0ee41d0ecd5bc4a3550804b74b73b901e074c2/guacamole/src/main/java/org/apache/guacamole/tunnel/TunnelRequestService.java#L184-L186
---
