This is an automated email from the ASF dual-hosted git repository. yiguolei pushed a commit to branch branch-4.2 in repository https://gitbox.apache.org/repos/asf/doris.git
commit 420a9c4be46e9dcf78f6ea08ecaee13bc1324688 Author: Calvin Kirs <[email protected]> AuthorDate: Mon Sep 21 16:23:31 2026 +0800 branch-4.1: [improvement](fe) Add connector property validators (#68163) https://github.com/apache/doris/pull/66153 --- .../property/storage/HdfsProperties.java | 2 + .../property/storage/OSSHdfsProperties.java | 2 + .../property/ConnectorPropertiesUtilsTest.java | 32 ++++++++++++++ .../property/storage/HdfsPropertiesTest.java | 12 ++++++ .../property/ConnectorPropertiesUtils.java | 23 ++++++++++ .../foundation/property/ConnectorProperty.java | 2 + .../property/ConnectorPropertyValidator.java | 50 ++++++++++++++++++++++ .../property/NoPathTraversalValidator.java | 45 +++++++++++++++++++ 8 files changed, 168 insertions(+) diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/HdfsProperties.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/HdfsProperties.java index 4b943973519..db60777aa8c 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/HdfsProperties.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/HdfsProperties.java @@ -20,6 +20,7 @@ package org.apache.doris.datasource.property.storage; import org.apache.doris.common.UserException; import org.apache.doris.common.security.authentication.HadoopAuthenticator; import org.apache.doris.foundation.property.ConnectorProperty; +import org.apache.doris.foundation.property.NoPathTraversalValidator; import com.google.common.base.Strings; import com.google.common.collect.ImmutableSet; @@ -73,6 +74,7 @@ public class HdfsProperties extends HdfsCompatibleProperties { @ConnectorProperty(names = {"hadoop.config.resources"}, required = false, + validator = NoPathTraversalValidator.class, description = "The xml files of Hadoop configuration.") protected String hadoopConfigResources = ""; diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/OSSHdfsProperties.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/OSSHdfsProperties.java index c0d2a57f795..86d0693d3ec 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/OSSHdfsProperties.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/OSSHdfsProperties.java @@ -19,6 +19,7 @@ package org.apache.doris.datasource.property.storage; import org.apache.doris.common.UserException; import org.apache.doris.foundation.property.ConnectorProperty; +import org.apache.doris.foundation.property.NoPathTraversalValidator; import com.google.common.collect.ImmutableSet; import lombok.Setter; @@ -74,6 +75,7 @@ public class OSSHdfsProperties extends HdfsCompatibleProperties { @ConnectorProperty(names = {"oss.hdfs.hadoop.config.resources"}, required = false, + validator = NoPathTraversalValidator.class, description = "The xml files of Hadoop configuration.") protected String hadoopConfigResources = ""; diff --git a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/ConnectorPropertiesUtilsTest.java b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/ConnectorPropertiesUtilsTest.java index cdd22c72fe5..0c79e8ce7a3 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/ConnectorPropertiesUtilsTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/ConnectorPropertiesUtilsTest.java @@ -19,6 +19,7 @@ package org.apache.doris.datasource.property; import org.apache.doris.foundation.property.ConnectorPropertiesUtils; import org.apache.doris.foundation.property.ConnectorProperty; +import org.apache.doris.foundation.property.NoPathTraversalValidator; import org.apache.doris.nereids.types.UnsupportedType; import org.junit.jupiter.api.Assertions; @@ -103,6 +104,30 @@ public class ConnectorPropertiesUtilsTest { }); } + @Test + void testPropertyValidatorRejectsInvalidValue() { + Map<String, String> props = new HashMap<>(); + props.put("path.key", "core-site.xml, ..\\secret.xml"); + + SampleConfig config = new SampleConfig(); + IllegalArgumentException ex = Assertions.assertThrows(IllegalArgumentException.class, + () -> ConnectorPropertiesUtils.bindConnectorProperties(config, props)); + + Assertions.assertTrue(ex.getMessage().contains("parent-directory references"), ex.getMessage()); + Assertions.assertNull(config.getPathValue()); + } + + @Test + void testPropertyValidatorAcceptsSafeValue() { + Map<String, String> props = new HashMap<>(); + props.put("path.key", "conf/core-site.xml"); + + SampleConfig config = new SampleConfig(); + ConnectorPropertiesUtils.bindConnectorProperties(config, props); + + Assertions.assertEquals("conf/core-site.xml", config.getPathValue()); + } + public class SampleConfig { @ConnectorProperty(names = {"string.key"}) @@ -129,6 +154,9 @@ public class ConnectorPropertiesUtilsTest { @ConnectorProperty(names = {"unsupported.key"}) private UnsupportedType unsupportedField; + @ConnectorProperty(names = {"path.key"}, validator = NoPathTraversalValidator.class) + private String pathValue; + public String getStringValue() { return stringValue; } @@ -161,6 +189,10 @@ public class ConnectorPropertiesUtilsTest { return unsupportedField; } + public String getPathValue() { + return pathValue; + } + } } diff --git a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/HdfsPropertiesTest.java b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/HdfsPropertiesTest.java index f0a4e73231f..b512ddfe4b6 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/HdfsPropertiesTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/HdfsPropertiesTest.java @@ -69,6 +69,18 @@ public class HdfsPropertiesTest { Assertions.assertEquals(HadoopKerberosAuthenticator.class, properties.hadoopAuthenticator.getClass()); } + @Test + public void testConfigResourcesRejectParentDirectoryTraversal() { + Map<String, String> origProps = createBaseHdfsProperties(); + origProps.put("hadoop.config.resources", "../outside.xml"); + + IllegalArgumentException exception = Assertions.assertThrows( + IllegalArgumentException.class, () -> StorageProperties.createAll(origProps)); + + Assertions.assertTrue(exception.getMessage().contains("parent-directory references"), + exception.getMessage()); + } + @Test public void testBasicHdfsPropertiesCreateByConfigFile() throws UserException { // Test 1: Check loading of config resources diff --git a/fe/fe-foundation/src/main/java/org/apache/doris/foundation/property/ConnectorPropertiesUtils.java b/fe/fe-foundation/src/main/java/org/apache/doris/foundation/property/ConnectorPropertiesUtils.java index 6be437c7ba4..b73e97e57ea 100644 --- a/fe/fe-foundation/src/main/java/org/apache/doris/foundation/property/ConnectorPropertiesUtils.java +++ b/fe/fe-foundation/src/main/java/org/apache/doris/foundation/property/ConnectorPropertiesUtils.java @@ -23,12 +23,15 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; /** * Utility class for handling fields annotated with {@link ConnectorProperty}. * Provides methods to extract supported connector properties from a class and bind them to an object instance. */ public class ConnectorPropertiesUtils { + private static final Map<Class<? extends ConnectorPropertyValidator>, ConnectorPropertyValidator> VALIDATORS = + new ConcurrentHashMap<>(); /** * Retrieves all fields annotated with {@link ConnectorProperty} from the given class and its superclasses, @@ -74,6 +77,7 @@ public class ConnectorPropertiesUtils { try { Object rawValue = props.get(matchedName); Object convertedValue = convertValue(rawValue, field.getType()); + validateValue(target, field, matchedName, convertedValue, props); field.set(target, convertedValue); } catch (Exception e) { throw new IllegalArgumentException( @@ -85,6 +89,25 @@ public class ConnectorPropertiesUtils { } } + private static void validateValue(Object target, Field field, String propertyName, Object value, + Map<String, String> props) { + Class<? extends ConnectorPropertyValidator> validatorClass = + field.getAnnotation(ConnectorProperty.class).validator(); + ConnectorPropertyValidator validator = VALIDATORS.computeIfAbsent( + validatorClass, ConnectorPropertiesUtils::createValidator); + validator.validate(target, field, propertyName, value, props); + } + + private static ConnectorPropertyValidator createValidator( + Class<? extends ConnectorPropertyValidator> validatorClass) { + try { + return validatorClass.getDeclaredConstructor().newInstance(); + } catch (ReflectiveOperationException e) { + throw new IllegalArgumentException( + "Failed to create connector property validator " + validatorClass.getName(), e); + } + } + /** * Finds the first matching property name from the field's {@code @ConnectorProperty#names()} list * that exists in the provided property map. diff --git a/fe/fe-foundation/src/main/java/org/apache/doris/foundation/property/ConnectorProperty.java b/fe/fe-foundation/src/main/java/org/apache/doris/foundation/property/ConnectorProperty.java index 96d607d7c76..70f5ec84baf 100644 --- a/fe/fe-foundation/src/main/java/org/apache/doris/foundation/property/ConnectorProperty.java +++ b/fe/fe-foundation/src/main/java/org/apache/doris/foundation/property/ConnectorProperty.java @@ -31,4 +31,6 @@ public @interface ConnectorProperty { boolean sensitive() default false; boolean isRegionField() default false; + + Class<? extends ConnectorPropertyValidator> validator() default ConnectorPropertyValidator.None.class; } diff --git a/fe/fe-foundation/src/main/java/org/apache/doris/foundation/property/ConnectorPropertyValidator.java b/fe/fe-foundation/src/main/java/org/apache/doris/foundation/property/ConnectorPropertyValidator.java new file mode 100644 index 00000000000..07a59701269 --- /dev/null +++ b/fe/fe-foundation/src/main/java/org/apache/doris/foundation/property/ConnectorPropertyValidator.java @@ -0,0 +1,50 @@ +// 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.doris.foundation.property; + +import java.lang.reflect.Field; +import java.util.Map; + +/** + * Validates a converted connector property before it is assigned to its target field. + * + * <p>Implementations must be stateless because one validator instance may be reused across + * property bindings. + */ +public interface ConnectorPropertyValidator { + + /** + * Validates one connector property value. + * + * @param target target object being populated + * @param field annotated target field + * @param propertyName matched property name + * @param value converted property value + * @param properties complete raw property map + * @throws IllegalArgumentException if the value is invalid + */ + void validate(Object target, Field field, String propertyName, Object value, Map<String, String> properties); + + /** Default validator used by properties that do not declare validation rules. */ + final class None implements ConnectorPropertyValidator { + @Override + public void validate(Object target, Field field, String propertyName, Object value, + Map<String, String> properties) { + } + } +} diff --git a/fe/fe-foundation/src/main/java/org/apache/doris/foundation/property/NoPathTraversalValidator.java b/fe/fe-foundation/src/main/java/org/apache/doris/foundation/property/NoPathTraversalValidator.java new file mode 100644 index 00000000000..f8d48422b61 --- /dev/null +++ b/fe/fe-foundation/src/main/java/org/apache/doris/foundation/property/NoPathTraversalValidator.java @@ -0,0 +1,45 @@ +// 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.doris.foundation.property; + +import java.lang.reflect.Field; +import java.util.Map; + +/** + * Rejects parent-directory path components in a string property. + * + * <p>Both slash forms are treated as separators so a value cannot become unsafe when it is + * forwarded to a platform or library with different path-separator semantics. Comma-separated + * resource lists are supported naturally because commas cannot hide a path component. + */ +public final class NoPathTraversalValidator implements ConnectorPropertyValidator { + + @Override + public void validate(Object target, Field field, String propertyName, Object value, + Map<String, String> properties) { + if (!(value instanceof String)) { + throw new IllegalArgumentException("Path property must be a string"); + } + for (String component : ((String) value).split("[,/\\\\]")) { + if ("..".equals(component.trim())) { + throw new IllegalArgumentException( + "Property " + propertyName + " must not contain parent-directory references"); + } + } + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
