From f0ca63b2ab4a630f600c66cf7c581a4565bb525a Mon Sep 17 00:00:00 2001 From: Bence Simon Date: Wed, 14 Apr 2021 23:05:13 +0200 Subject: [PATCH] NIFI-8429 Changing DBCPConnectionPool driver manager in order to prevent from leaking NIFI-8429 Refining error handling based on review comment This closes #5003. Signed-off-by: Peter Turcsanyi --- .../apache/nifi/dbcp/DBCPConnectionPool.java | 79 ++++++++----------- .../java/org/apache/nifi/dbcp/DriverShim.java | 74 ----------------- 2 files changed, 31 insertions(+), 122 deletions(-) delete mode 100644 nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/main/java/org/apache/nifi/dbcp/DriverShim.java diff --git a/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/main/java/org/apache/nifi/dbcp/DBCPConnectionPool.java b/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/main/java/org/apache/nifi/dbcp/DBCPConnectionPool.java index 3972cec97f..b43f691038 100644 --- a/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/main/java/org/apache/nifi/dbcp/DBCPConnectionPool.java +++ b/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/main/java/org/apache/nifi/dbcp/DBCPConnectionPool.java @@ -21,6 +21,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.commons.pool2.impl.GenericObjectPoolConfig; import org.apache.nifi.annotation.behavior.DynamicProperties; import org.apache.nifi.annotation.behavior.DynamicProperty; +import org.apache.nifi.annotation.behavior.RequiresInstanceClassLoading; import org.apache.nifi.annotation.documentation.CapabilityDescription; import org.apache.nifi.annotation.documentation.Tags; import org.apache.nifi.annotation.lifecycle.OnDisabled; @@ -43,10 +44,8 @@ import org.apache.nifi.security.krb.KerberosAction; import org.apache.nifi.security.krb.KerberosKeytabUser; import org.apache.nifi.security.krb.KerberosPasswordUser; import org.apache.nifi.security.krb.KerberosUser; -import org.apache.nifi.util.file.classloader.ClassLoaderUtils; import javax.security.auth.login.LoginException; -import java.net.MalformedURLException; import java.sql.Connection; import java.sql.Driver; import java.sql.DriverManager; @@ -74,6 +73,7 @@ import java.util.stream.Collectors; expressionLanguageScope = ExpressionLanguageScope.NONE, description = "JDBC driver property name prefixed with 'SENSITIVE.' handled as a sensitive property.") }) +@RequiresInstanceClassLoading public class DBCPConnectionPool extends AbstractControllerService implements DBCPService { /** Property Name Prefix for Sensitive Dynamic Properties */ protected static final String SENSITIVE_PROPERTY_PREFIX = "SENSITIVE."; @@ -131,6 +131,7 @@ public class DBCPConnectionPool extends AbstractControllerService implements DBC .required(false) .identifiesExternalResource(ResourceCardinality.MULTIPLE, ResourceType.FILE, ResourceType.DIRECTORY, ResourceType.URL) .expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY) + .dynamicallyModifiesClasspath(true) .build(); public static final PropertyDescriptor DB_USER = new PropertyDescriptor.Builder() @@ -386,9 +387,10 @@ public class DBCPConnectionPool extends AbstractControllerService implements DBC @OnEnabled public void onConfigured(final ConfigurationContext context) throws InitializationException { - final String drv = context.getProperty(DB_DRIVERNAME).evaluateAttributeExpressions().getValue(); + final String driverName = context.getProperty(DB_DRIVERNAME).evaluateAttributeExpressions().getValue(); final String user = context.getProperty(DB_USER).evaluateAttributeExpressions().getValue(); final String passw = context.getProperty(DB_PASSWORD).evaluateAttributeExpressions().getValue(); + final String dburl = context.getProperty(DATABASE_URL).evaluateAttributeExpressions().getValue(); final Integer maxTotal = context.getProperty(MAX_TOTAL_CONNECTIONS).evaluateAttributeExpressions().asInteger(); final String validationQuery = context.getProperty(VALIDATION_QUERY).evaluateAttributeExpressions().getValue(); final Long maxWaitMillis = extractMillisWithInfinite(context.getProperty(MAX_WAIT_TIME).evaluateAttributeExpressions()); @@ -417,14 +419,7 @@ public class DBCPConnectionPool extends AbstractControllerService implements DBC } dataSource = new BasicDataSource(); - dataSource.setDriverClassName(drv); - - // Optional driver URL, when exist, this URL will be used to locate driver jar file location - final String urlString = context.getProperty(DB_DRIVER_LOCATION).evaluateAttributeExpressions().getValue(); - dataSource.setDriverClassLoader(getDriverClassLoader(urlString, drv)); - - final String dburl = context.getProperty(DATABASE_URL).evaluateAttributeExpressions().getValue(); - + dataSource.setDriver(getDriver(driverName, dburl)); dataSource.setMaxWaitMillis(maxWaitMillis); dataSource.setMaxTotal(maxTotal); dataSource.setMinIdle(minIdle); @@ -460,45 +455,33 @@ public class DBCPConnectionPool extends AbstractControllerService implements DBC }); } - private Long extractMillisWithInfinite(PropertyValue prop) { - return "-1".equals(prop.getValue()) ? -1 : prop.asTimePeriod(TimeUnit.MILLISECONDS); + private Driver getDriver(final String driverName, final String url) { + final Class clazz; + + try { + clazz = Class.forName(driverName); + } catch (final ClassNotFoundException e) { + throw new ProcessException("Driver class " + driverName + " is not found", e); + } + + try { + return DriverManager.getDriver(url); + } catch (final SQLException e) { + // In case the driver is not registered by the implementation, we explicitly try to register it. + try { + final Driver driver = (Driver) clazz.newInstance(); + DriverManager.registerDriver(driver); + return DriverManager.getDriver(url); + } catch (final SQLException e2) { + throw new ProcessException("No suitable driver for the given Database Connection URL", e2); + } catch (final IllegalAccessException | InstantiationException e2) { + throw new ProcessException("Creating driver instance is failed", e2); + } + } } - /** - * using Thread.currentThread().getContextClassLoader(); will ensure that you are using the ClassLoader for you NAR. - * - * @throws InitializationException - * if there is a problem obtaining the ClassLoader - */ - protected ClassLoader getDriverClassLoader(String locationString, String drvName) throws InitializationException { - if (locationString != null && locationString.length() > 0) { - try { - // Split and trim the entries - final ClassLoader classLoader = ClassLoaderUtils.getCustomClassLoader( - locationString, - this.getClass().getClassLoader(), - (dir, name) -> name != null && name.endsWith(".jar") - ); - - // Workaround which allows to use URLClassLoader for JDBC driver loading. - // (Because the DriverManager will refuse to use a driver not loaded by the system ClassLoader.) - final Class clazz = Class.forName(drvName, true, classLoader); - if (clazz == null) { - throw new InitializationException("Can't load Database Driver " + drvName); - } - final Driver driver = (Driver) clazz.newInstance(); - DriverManager.registerDriver(new DriverShim(driver)); - - return classLoader; - } catch (final MalformedURLException e) { - throw new InitializationException("Invalid Database Driver Jar Url", e); - } catch (final Exception e) { - throw new InitializationException("Can't load Database Driver", e); - } - } else { - // That will ensure that you are using the ClassLoader for you NAR. - return Thread.currentThread().getContextClassLoader(); - } + private Long extractMillisWithInfinite(PropertyValue prop) { + return "-1".equals(prop.getValue()) ? -1 : prop.asTimePeriod(TimeUnit.MILLISECONDS); } /** diff --git a/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/main/java/org/apache/nifi/dbcp/DriverShim.java b/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/main/java/org/apache/nifi/dbcp/DriverShim.java deleted file mode 100644 index 596f1719f6..0000000000 --- a/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/main/java/org/apache/nifi/dbcp/DriverShim.java +++ /dev/null @@ -1,74 +0,0 @@ -/* - * 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.nifi.dbcp; - -import java.sql.Connection; -import java.sql.Driver; -import java.sql.DriverPropertyInfo; -import java.sql.SQLException; -import java.sql.SQLFeatureNotSupportedException; -import java.util.Properties; -import java.util.logging.Logger; - -/** - * Workaround which allows to use URLClassLoader for JDBC driver loading. - * (Because the DriverManager will refuse to use a driver not loaded by the system ClassLoader.) - * - */ -class DriverShim implements Driver { - private Driver driver; - - DriverShim(Driver d) { - this.driver = d; - } - - @Override - public boolean acceptsURL(String u) throws SQLException { - return this.driver.acceptsURL(u); - } - - @Override - public Connection connect(String u, Properties p) throws SQLException { - return this.driver.connect(u, p); - } - - @Override - public int getMajorVersion() { - return this.driver.getMajorVersion(); - } - - @Override - public int getMinorVersion() { - return this.driver.getMinorVersion(); - } - - @Override - public DriverPropertyInfo[] getPropertyInfo(String u, Properties p) throws SQLException { - return this.driver.getPropertyInfo(u, p); - } - - @Override - public boolean jdbcCompliant() { - return this.driver.jdbcCompliant(); - } - - @Override - public Logger getParentLogger() throws SQLFeatureNotSupportedException { - return driver.getParentLogger(); - } - -} \ No newline at end of file