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 <turcsanyi@apache.org>
This commit is contained in:
Bence Simon 2021-04-14 23:05:13 +02:00 committed by Peter Turcsanyi
parent 0f8789b8b0
commit f0ca63b2ab
2 changed files with 31 additions and 122 deletions

View File

@ -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);
}
/**

View File

@ -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();
}
}