From 84326ba4b2853f67e7c60aaff0790d3ccf82fdb5 Mon Sep 17 00:00:00 2001 From: exceptionfactory Date: Tue, 8 Aug 2023 09:22:35 -0500 Subject: [PATCH] NIFI-11920 Improved JDBC and JNDI JMS Connection URL Validation Signed-off-by: Matt Burgess This closes #7586 --- .../nifi/dbcp/utils/DBCPProperties.java | 3 +- .../JndiJmsConnectionFactoryProperties.java | 8 +- .../JndiJmsConnectionFactoryProviderTest.java | 23 ++++- .../nifi/dbcp/ConnectionUrlValidator.java | 6 +- .../nifi/dbcp/DriverClassValidator.java | 57 +++++++++++ .../nifi/dbcp/ConnectionUrlValidatorTest.java | 20 ++++ .../nifi/dbcp/DriverClassValidatorTest.java | 97 +++++++++++++++++++ .../nifi/dbcp/HikariCPConnectionPool.java | 2 +- 8 files changed, 207 insertions(+), 9 deletions(-) create mode 100644 nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-api/src/main/java/org/apache/nifi/dbcp/DriverClassValidator.java create mode 100644 nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-api/src/test/java/org/apache/nifi/dbcp/DriverClassValidatorTest.java diff --git a/nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/src/main/java/org/apache/nifi/dbcp/utils/DBCPProperties.java b/nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/src/main/java/org/apache/nifi/dbcp/utils/DBCPProperties.java index 6f8c94b592..32774810bb 100644 --- a/nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/src/main/java/org/apache/nifi/dbcp/utils/DBCPProperties.java +++ b/nifi-nar-bundles/nifi-extension-utils/nifi-dbcp-base/src/main/java/org/apache/nifi/dbcp/utils/DBCPProperties.java @@ -22,6 +22,7 @@ import org.apache.nifi.components.resource.ResourceCardinality; import org.apache.nifi.components.resource.ResourceType; import org.apache.nifi.dbcp.ConnectionUrlValidator; import org.apache.nifi.dbcp.DBCPValidator; +import org.apache.nifi.dbcp.DriverClassValidator; import org.apache.nifi.expression.ExpressionLanguageScope; import org.apache.nifi.kerberos.KerberosUserService; import org.apache.nifi.processor.util.StandardValidators; @@ -67,7 +68,7 @@ public final class DBCPProperties { .description("Database driver class name") .defaultValue(null) .required(true) - .addValidator(StandardValidators.NON_EMPTY_VALIDATOR) + .addValidator(new DriverClassValidator()) .expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY) .build(); diff --git a/nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/cf/JndiJmsConnectionFactoryProperties.java b/nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/cf/JndiJmsConnectionFactoryProperties.java index 823ae0d976..7267c6ea53 100644 --- a/nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/cf/JndiJmsConnectionFactoryProperties.java +++ b/nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/cf/JndiJmsConnectionFactoryProperties.java @@ -158,10 +158,11 @@ public class JndiJmsConnectionFactoryProperties { public ValidationResult validate(final String subject, final String input, final ValidationContext context) { final ValidationResult.Builder builder = new ValidationResult.Builder().subject(subject).input(input); - if (input == null || input.isEmpty()) { + final String url = context.newPropertyValue(input).evaluateAttributeExpressions().getValue(); + if (url == null || url.isEmpty()) { builder.valid(false); builder.explanation("URL is required"); - } else if (isUrlAllowed(input)) { + } else if (isUrlAllowed(url)) { builder.valid(true); builder.explanation("URL scheme allowed"); } else { @@ -176,7 +177,8 @@ public class JndiJmsConnectionFactoryProperties { private boolean isUrlAllowed(final String input) { final boolean allowed; - final Matcher matcher = URL_SCHEME_PATTERN.matcher(input); + final String normalizedUrl = input.trim(); + final Matcher matcher = URL_SCHEME_PATTERN.matcher(normalizedUrl); if (matcher.matches()) { final String scheme = matcher.group(SCHEME_GROUP); allowed = isSchemeAllowed(scheme); diff --git a/nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/test/java/org/apache/nifi/jms/cf/JndiJmsConnectionFactoryProviderTest.java b/nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/test/java/org/apache/nifi/jms/cf/JndiJmsConnectionFactoryProviderTest.java index b331660200..40b90ea022 100644 --- a/nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/test/java/org/apache/nifi/jms/cf/JndiJmsConnectionFactoryProviderTest.java +++ b/nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/test/java/org/apache/nifi/jms/cf/JndiJmsConnectionFactoryProviderTest.java @@ -41,6 +41,10 @@ public class JndiJmsConnectionFactoryProviderTest { private static final String LDAP_PROVIDER_URL = "ldap://127.0.0.1"; + private static final String LDAP_PROVIDER_URL_SPACED = String.format(" %s", LDAP_PROVIDER_URL); + + private static final String LDAP_PROVIDER_URL_EXPRESSION = "ldap:${separator}//127.0.0.1"; + private static final String HOST_PORT_URL = "127.0.0.1:1024"; private static final String LDAP_ALLOWED_URL_SCHEMES = "ldap"; @@ -81,6 +85,24 @@ public class JndiJmsConnectionFactoryProviderTest { runner.assertNotValid(provider); } + @Test + void testPropertiesInvalidUrlSchemeSpaced() { + setFactoryProperties(); + + runner.setProperty(provider, JndiJmsConnectionFactoryProperties.JNDI_PROVIDER_URL, LDAP_PROVIDER_URL_SPACED); + + runner.assertNotValid(provider); + } + + @Test + void testPropertiesInvalidUrlSchemeExpression() { + setFactoryProperties(); + + runner.setProperty(provider, JndiJmsConnectionFactoryProperties.JNDI_PROVIDER_URL, LDAP_PROVIDER_URL_EXPRESSION); + + runner.assertNotValid(provider); + } + @Test void testPropertiesHostPortUrl() { setFactoryProperties(); @@ -90,7 +112,6 @@ public class JndiJmsConnectionFactoryProviderTest { runner.assertValid(provider); } - @Test void testUrlSchemeValidSystemProperty() { try { diff --git a/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-api/src/main/java/org/apache/nifi/dbcp/ConnectionUrlValidator.java b/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-api/src/main/java/org/apache/nifi/dbcp/ConnectionUrlValidator.java index c997d14006..a5863840ec 100644 --- a/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-api/src/main/java/org/apache/nifi/dbcp/ConnectionUrlValidator.java +++ b/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-api/src/main/java/org/apache/nifi/dbcp/ConnectionUrlValidator.java @@ -37,11 +37,11 @@ public class ConnectionUrlValidator implements Validator { builder.valid(false); builder.explanation("Connection URL required"); } else { - final String url = context.newPropertyValue(input).evaluateAttributeExpressions().getValue(); + final String url = context.newPropertyValue(input).evaluateAttributeExpressions().getValue().trim(); if (isUrlUnsupported(url)) { builder.valid(false); - builder.explanation(String.format("Connection URL starts with an unsupported scheme %s", UNSUPPORTED_SCHEMES)); + builder.explanation(String.format("Connection URL contains an unsupported scheme %s", UNSUPPORTED_SCHEMES)); } else { builder.valid(true); builder.explanation("Connection URL is valid"); @@ -55,7 +55,7 @@ public class ConnectionUrlValidator implements Validator { boolean unsupported = false; for (final String unsupportedScheme : UNSUPPORTED_SCHEMES) { - if (url.startsWith(unsupportedScheme)) { + if (url.contains(unsupportedScheme)) { unsupported = true; break; } diff --git a/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-api/src/main/java/org/apache/nifi/dbcp/DriverClassValidator.java b/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-api/src/main/java/org/apache/nifi/dbcp/DriverClassValidator.java new file mode 100644 index 0000000000..bffe1961e7 --- /dev/null +++ b/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-api/src/main/java/org/apache/nifi/dbcp/DriverClassValidator.java @@ -0,0 +1,57 @@ +/* + * 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 org.apache.nifi.components.ValidationContext; +import org.apache.nifi.components.ValidationResult; +import org.apache.nifi.components.Validator; + +import java.util.Collections; +import java.util.Set; + +/** + * Database Driver Class Validator supports system attribute expressions and evaluates class names against unsupported values + */ +public class DriverClassValidator implements Validator { + private static final Set UNSUPPORTED_CLASSES = Collections.singleton("org.h2.Driver"); + + @Override + public ValidationResult validate(final String subject, final String input, final ValidationContext context) { + final ValidationResult.Builder builder = new ValidationResult.Builder().subject(subject).input(input); + + if (input == null || input.isEmpty()) { + builder.valid(false); + builder.explanation("Driver Class required"); + } else { + final String driverClass = context.newPropertyValue(input).evaluateAttributeExpressions().getValue().trim(); + + if (isDriverClassUnsupported(driverClass)) { + builder.valid(false); + builder.explanation(String.format("Driver Class is listed as unsupported %s", UNSUPPORTED_CLASSES)); + } else { + builder.valid(true); + builder.explanation("Driver Class is valid"); + } + } + + return builder.build(); + } + + private boolean isDriverClassUnsupported(final String driverClass) { + return UNSUPPORTED_CLASSES.contains(driverClass); + } +} diff --git a/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-api/src/test/java/org/apache/nifi/dbcp/ConnectionUrlValidatorTest.java b/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-api/src/test/java/org/apache/nifi/dbcp/ConnectionUrlValidatorTest.java index 0421434eb0..5086b78512 100644 --- a/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-api/src/test/java/org/apache/nifi/dbcp/ConnectionUrlValidatorTest.java +++ b/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-api/src/test/java/org/apache/nifi/dbcp/ConnectionUrlValidatorTest.java @@ -37,6 +37,10 @@ class ConnectionUrlValidatorTest { private static final String UNSUPPORTED_URL = "jdbc:h2:file"; + private static final String UNSUPPORTED_URL_SPACED = String.format(" %s ", UNSUPPORTED_URL); + + private static final String UNSUPPORTED_URL_EXPRESSION = String.format("${attribute}%s", UNSUPPORTED_URL); + private static final String VENDOR_URL = "jdbc:vendor"; private ValidationContext validationContext; @@ -67,6 +71,22 @@ class ConnectionUrlValidatorTest { assertFalse(result.isValid()); } + @Test + void testValidateUnsupportedUrlExpressionLanguage() { + final ValidationResult result = validator.validate(SUBJECT, UNSUPPORTED_URL_EXPRESSION, validationContext); + + assertNotNull(result); + assertFalse(result.isValid()); + } + + @Test + void testValidateUnsupportedUrlSpaced() { + final ValidationResult result = validator.validate(SUBJECT, UNSUPPORTED_URL_SPACED, validationContext); + + assertNotNull(result); + assertFalse(result.isValid()); + } + @Test void testValidateSupportedUrl() { final ValidationResult result = validator.validate(SUBJECT, VENDOR_URL, validationContext); diff --git a/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-api/src/test/java/org/apache/nifi/dbcp/DriverClassValidatorTest.java b/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-api/src/test/java/org/apache/nifi/dbcp/DriverClassValidatorTest.java new file mode 100644 index 0000000000..02de83de89 --- /dev/null +++ b/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-api/src/test/java/org/apache/nifi/dbcp/DriverClassValidatorTest.java @@ -0,0 +1,97 @@ +/* + * 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 org.apache.nifi.components.ValidationContext; +import org.apache.nifi.components.ValidationResult; +import org.apache.nifi.util.MockProcessContext; +import org.apache.nifi.util.MockValidationContext; +import org.apache.nifi.util.NoOpProcessor; +import org.apache.nifi.util.TestRunners; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class DriverClassValidatorTest { + + private static final String SUBJECT = "Database Driver Class"; + + private static final String EMPTY = ""; + + private static final String UNSUPPORTED_DRIVER = "org.h2.Driver"; + + private static final String UNSUPPORTED_DRIVER_SPACED = String.format(" %s ", UNSUPPORTED_DRIVER); + + private static final String UNSUPPORTED_DRIVER_EXPRESSION = String.format("${attribute}%s", UNSUPPORTED_DRIVER); + + private static final String OTHER_DRIVER = "org.apache.nifi.Driver"; + + private ValidationContext validationContext; + + private DriverClassValidator validator; + + @BeforeEach + void setValidator() { + validator = new DriverClassValidator(); + + final MockProcessContext processContext = (MockProcessContext) TestRunners.newTestRunner(NoOpProcessor.class).getProcessContext(); + validationContext = new MockValidationContext(processContext); + } + + @Test + void testValidateEmpty() { + final ValidationResult result = validator.validate(SUBJECT, EMPTY, validationContext); + + assertNotNull(result); + assertFalse(result.isValid()); + } + + @Test + void testValidateUnsupportedDriver() { + final ValidationResult result = validator.validate(SUBJECT, UNSUPPORTED_DRIVER, validationContext); + + assertNotNull(result); + assertFalse(result.isValid()); + } + + @Test + void testValidateUnsupportedDriverExpressionLanguage() { + final ValidationResult result = validator.validate(SUBJECT, UNSUPPORTED_DRIVER_EXPRESSION, validationContext); + + assertNotNull(result); + assertFalse(result.isValid()); + } + + @Test + void testValidateUnsupportedDriverSpaced() { + final ValidationResult result = validator.validate(SUBJECT, UNSUPPORTED_DRIVER_SPACED, validationContext); + + assertNotNull(result); + assertFalse(result.isValid()); + } + + @Test + void testValidateSupportedDriver() { + final ValidationResult result = validator.validate(SUBJECT, OTHER_DRIVER, validationContext); + + assertNotNull(result); + assertTrue(result.isValid()); + } +} diff --git a/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-hikari-dbcp-service/src/main/java/org/apache/nifi/dbcp/HikariCPConnectionPool.java b/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-hikari-dbcp-service/src/main/java/org/apache/nifi/dbcp/HikariCPConnectionPool.java index f4f7f68e8d..876afbb599 100644 --- a/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-hikari-dbcp-service/src/main/java/org/apache/nifi/dbcp/HikariCPConnectionPool.java +++ b/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-hikari-dbcp-service/src/main/java/org/apache/nifi/dbcp/HikariCPConnectionPool.java @@ -98,7 +98,7 @@ public class HikariCPConnectionPool extends AbstractControllerService implements .description("The fully-qualified class name of the JDBC driver. Example: com.mysql.jdbc.Driver") .defaultValue(null) .required(true) - .addValidator(StandardValidators.NON_EMPTY_VALIDATOR) + .addValidator(new DriverClassValidator()) .expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY) .build();