OIiveirra commented on code in PR #69526: URL: https://github.com/apache/doris/pull/69526#discussion_r4367695723
########## fe/fe-core/src/main/java/org/apache/doris/tablefunction/HadoopTvfAdmission.java: ########## @@ -0,0 +1,476 @@ +// 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.tablefunction; + +import org.apache.doris.common.Config; +import org.apache.doris.common.util.LocationPath; +import org.apache.doris.nereids.exceptions.AnalysisException; + +import java.net.URI; +import java.net.URISyntaxException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +/** + * TVF-only admission for Hadoop consumers. Input properties have already been normalized by storage. + * This module neither loads Hadoop resources nor constructs a filesystem. Java strings and native + * C strings are projected separately before filesystem selection and mount-table interpretation. + */ +public final class HadoopTvfAdmission { + public enum Access { + NATIVE_WRITE, + JAVA_DELETE, + HADOOP_READ + } + + private enum Provider { + REMOTE, + LOCAL, + MOUNT_TABLE, + DYNAMIC, + UNKNOWN + } + + private static final Set<String> REMOTE_SCHEMES = new HashSet<>(Arrays.asList( + "hdfs", "s3a", "s3n", "oss", "ofs", "jfs", "abfs", "abfss", "wasb", "wasbs", "gs", "cosn")); + private static final Set<String> REMOTE_IMPLEMENTATIONS = new HashSet<>(Arrays.asList( + "org.apache.hadoop.hdfs.DistributedFileSystem", + "org.apache.hadoop.fs.s3a.S3AFileSystem", + "org.apache.hadoop.fs.azurebfs.AzureBlobFileSystem", + "org.apache.hadoop.fs.azurebfs.SecureAzureBlobFileSystem", + "org.apache.hadoop.fs.azure.NativeAzureFileSystem", + "org.apache.hadoop.fs.aliyun.oss.AliyunOSSFileSystem", + "com.aliyun.jindodata.oss.JindoOssFileSystem", + "org.apache.hadoop.fs.obs.OBSFileSystem", + "com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystem")); + + private final Access access; + private final Map<String, String> properties = new HashMap<>(); + + private HadoopTvfAdmission(Access access, Map<String, String> normalized, boolean nativeConsumer) { + this.access = access; + for (Map.Entry<String, String> entry : normalized.entrySet()) { + // parse_properties / generateHdfsParam consumes the canonical NameNode before JNI. + if (nativeConsumer && "fs.defaultFS".equalsIgnoreCase(entry.getKey())) { + continue; + } + String key = (nativeConsumer ? nativeStringPrefix(entry.getKey()) : entry.getKey()).trim(); + String value = nativeConsumer ? nativeStringPrefix(entry.getValue()) : entry.getValue(); + String previous = properties.put(key, value); + if (previous != null && !previous.equals(value) && isRoutingKey(key)) { + reject(); + } + } + } + + /** Reject local/unknown Hadoop TVF routes before listing, deletion, schema discovery or data IO. */ + public static void check(Access access, String path, Map<String, String> normalized) { + if (access != Access.HADOOP_READ && Config.enable_local_tvf) { + return; + } + try { + if (access == Access.NATIVE_WRITE) { + String nameNode = nativeNameNode(access, normalized); + URI output = nativeFileUri(access, path + "doris-tvf-file", nameNode); + String parent = output.getPath(); + parent = parent.substring(0, parent.lastIndexOf('/') + 1); + if (parent.length() > 1) { + parent = parent.substring(0, parent.length() - 1); + } + new HadoopTvfAdmission(access, normalized, true).checkNative(nameNode, parent); + } else { + HadoopTvfAdmission javaConsumer = new HadoopTvfAdmission(access, normalized, false); + // Project the Java consumer independently from native convert_path/JNI. + URI javaPath = hadoopPathUri(path); + javaConsumer.checkRoute(javaPath, javaPath.getPath(), new HashSet<>()); + if (access == Access.HADOOP_READ) { + // FileQueryScanNode sends this filesystem identifier in range.fs_name. BE + // falls back to hdfs_params.fs_name only when that range field is empty. + String nameNode = LocationPath.of(path).getFsIdentifier(); + if (nameNode.isEmpty()) { + nameNode = nativeNameNode(access, normalized); + } + HadoopTvfAdmission nativeConsumer = new HadoopTvfAdmission(access, normalized, true); + nativeConsumer.checkNativeRead(nameNode, path); + // Schema RPCs have no range.fs_name; they use THdfsParams.fs_name instead. + nativeConsumer.checkNativeRead(nativeNameNode(access, normalized), path); + } + } + } catch (URISyntaxException e) { + new HadoopTvfAdmission(access, normalized, false).reject(); + } + } + + private static String nativeNameNode(Access access, Map<String, String> normalized) { + String result = null; + for (Map.Entry<String, String> entry : normalized.entrySet()) { + if ("fs.defaultFS".equalsIgnoreCase(entry.getKey())) { + if (result != null && !result.equals(entry.getValue())) { + reject(access); + } + result = entry.getValue(); + } + } + return result == null ? "" : result; + } + + private void checkNative(String nameNode, String path) throws URISyntaxException { + nameNode = selector(nativeStringPrefix(nameNode)).trim(); + if (nameNode.isEmpty() && access == Access.NATIVE_WRITE) { + // BE passes fs_name.c_str(), never NULL. libhdfs prefixes this empty string + // with hdfs://, whose URI construction fails before any filesystem is selected. + // Preserve existing analysis; execution reports the invalid NameNode as before. + return; + } + if ("default".equals(nameNode) || nameNode.isEmpty()) { + reject(); + } + checkRoute(new URI(nameNode.contains("://") ? nameNode : "hdfs://" + nameNode), + path, new HashSet<>()); + } + + private void checkNativeRead(String nameNode, String path) throws URISyntaxException { + checkNative(nameNode, nativeFileUri(access, path, nameNode).getPath()); + } + + private List<URI> defaults() throws URISyntaxException { + List<URI> defaults = new ArrayList<>(); + for (String key : Arrays.asList("fs.defaultFS", "fs.default.name")) { + String value = property(key); + if (value != null) { + value = selector(value).trim(); + defaults.add(new URI("local".equals(value) ? "file:///" + : (uriScheme(value).isEmpty() ? "hdfs://" + value : value))); + } + } + if (defaults.isEmpty()) { + defaults.add(new URI("file:///")); + } + return defaults; + } + + private void checkRoute(URI filesystem, String path, Set<String> activeTables) throws URISyntaxException { + if (filesystem.getScheme() == null) { + for (URI defaultFs : defaults()) { + checkViewFsEnabled(defaultFs, path, activeTables); + } + return; + } + if (filesystem.getAuthority() == null) { + boolean completed = false; + for (URI defaultFs : defaults()) { + if (filesystem.getScheme().equals(defaultFs.getScheme()) && defaultFs.getAuthority() != null) { + checkViewFsEnabled(defaultFs, path, activeTables); + completed = true; + } + } + if (completed) { + return; + } + } + checkViewFsEnabled(filesystem, path, activeTables); + } + + private Provider provider(String scheme) { + if ("file".equalsIgnoreCase(scheme)) { + return Provider.LOCAL; + } + String implementation = property("fs." + scheme + ".impl"); + if (implementation != null) { + implementation = selector(implementation).trim(); + if ("org.apache.hadoop.fs.LocalFileSystem".equals(implementation) + || "org.apache.hadoop.fs.RawLocalFileSystem".equals(implementation)) { + return Provider.LOCAL; + } + if ("org.apache.hadoop.fs.viewfs.ViewFileSystem".equals(implementation)) { + return Provider.MOUNT_TABLE; + } + if ("org.apache.hadoop.fs.viewfs.ViewFileSystemOverloadScheme".equals(implementation) + || "org.apache.hadoop.hdfs.ViewDistributedFileSystem".equals(implementation)) { + return Provider.DYNAMIC; + } + return REMOTE_IMPLEMENTATIONS.contains(implementation) ? Provider.REMOTE : Provider.UNKNOWN; + } + if ("viewfs".equals(scheme)) { + return Provider.MOUNT_TABLE; + } + return REMOTE_SCHEMES.contains(scheme) ? Provider.REMOTE : Provider.UNKNOWN; + } + + private void checkGlobTargets(String key, Set<String> activeTables) throws URISyntaxException { + for (String target : selector(property(key)).split(",")) { + URI uri = new URI(target); + // Listing can touch any descendant of a selected mount. Reuse the same route + // interpreter for nested tables instead of assuming their root is representative. + String path = uri.getPath() == null ? "/" : uri.getPath(); + checkRoute(uri, (path.endsWith("/") ? path : path + "/") + "*", activeTables); + } + } + + private void checkViewFsEnabled(URI filesystem, String path, Set<String> activeTables) { + String scheme = filesystem.getScheme(); + Provider provider = provider(scheme); + if (provider == Provider.LOCAL || provider == Provider.UNKNOWN || provider == Provider.DYNAMIC) { + reject(); + } + if (provider != Provider.MOUNT_TABLE) { + return; + } + if (!path.startsWith("/")) { + // Java resolves relative paths against the selected filesystem's working directory. + // Its user/home configuration is not a proven absolute mount-table path here. + reject(); + } + String table = filesystem.getAuthority(); + String ignorePort = property("fs.viewfs.ignore.port.in.mount.table.name"); + if (ignorePort != null && Boolean.parseBoolean(selector(ignorePort).trim()) + && table != null && filesystem.getPort() != -1) { + table = filesystem.getHost(); + if (table == null) { + reject(); + } + } + if (table == null || table.isEmpty()) { + table = property("fs.viewfs.mounttable.default.name.key"); + table = table == null ? "default" : selector(table); + } + // Nested ViewFS cycles or arbitrarily deep delegation cannot establish remote storage. + if (activeTables.size() >= 32 || !activeTables.add(table)) { + reject(); + } + try { + String prefix = "fs.viewfs.mounttable." + table + "."; + String source = ""; + List<String> selectedKeys = new ArrayList<>(); + List<String> mountSources = new ArrayList<>(); + String fallback = null; + String rootMerge = null; + for (String key : properties.keySet()) { + if (!key.startsWith(prefix)) { + continue; + } + String entry = key.substring(prefix.length()); + if (entry.equals("linkFallback")) { + fallback = key; + continue; + } + if (entry.equals("linkMergeSlash")) { + rootMerge = key; + continue; + } + if (entry.startsWith("linkRegex")) { + // Regex interception rewrites runtime paths before ordinary mount resolution. + reject(); + } + int slash = entry.indexOf('/'); + if (slash < 0 || !(entry.startsWith("link.") || entry.startsWith("linkMerge.") + || entry.startsWith("linkNfly."))) { + continue; + } + String candidate = entry.substring(slash); + // InodeTree splits source components and discards trailing empty components. + while (candidate.length() > 1 && candidate.endsWith("/")) { + candidate = candidate.substring(0, candidate.length() - 1); + } + mountSources.add(candidate); + if (!(path.equals(candidate) || path.startsWith(candidate + "/"))) { + continue; + } + if (candidate.length() > source.length()) { + source = candidate; + selectedKeys.clear(); + } + if (candidate.equals(source)) { + selectedKeys.add(key); + } + } + if (access == Access.HADOOP_READ && hasGlob(path)) { + String literal = path.substring(0, firstGlob(path)); + int slash = literal.lastIndexOf('/'); + literal = slash < 0 ? "/" : literal.substring(0, slash + 1); + for (String key : properties.keySet()) { + if (!key.startsWith(prefix)) { + continue; + } + String entry = key.substring(prefix.length()); + int sourceStart = entry.indexOf('/'); + if (sourceStart >= 0 && (entry.startsWith("link.") || entry.startsWith("linkMerge.") + || entry.startsWith("linkNfly."))) { + String mount = entry.substring(sourceStart); + if (mount.startsWith(literal) || literal.startsWith(mount + "/")) { + checkGlobTargets(key, activeTables); + } + } + } + } + if (access == Access.HADOOP_READ && !hasGlob(path)) { + String subtree = path.endsWith("/") ? path : path + "/"; + for (String key : properties.keySet()) { + if (key.startsWith(prefix)) { + String entry = key.substring(prefix.length()); + int sourceStart = entry.indexOf('/'); + if (sourceStart >= 0 && entry.substring(sourceStart).startsWith(subtree)) { + checkGlobTargets(key, activeTables); + } + } + } + } + if (rootMerge != null) { + selectedKeys.add(rootMerge); + source = ""; + } else if (selectedKeys.isEmpty() && fallback != null) { + selectedKeys.add(fallback); + } + if (selectedKeys.isEmpty()) { + // No explicit target can prove what a BE-inherited mount table will select. + reject(); + } + for (String key : selectedKeys) { + String targets = selector(property(key)); + for (String target : targets.split(",")) { + if (target.isEmpty()) { Review Comment: Fixed in dc19f3fea28bb9b2705a734f3105f9a8fcb09c29. Selected mount targets must now be nonempty before admission succeeds. Both selected and glob-target parsing retain empty components, so empty, comma-only and trailing-empty definitions fail closed. The change stays in TVF admission; the generic Hadoop configuration builder is unchanged. Actual Hadoop classes confirmed that the master builder omits the empty entry and selects the fallback. The new cleanup test verifies rejection before filesystem-factory interaction, without performing deletion. Before tests reproduced the missing rejection; all 55 selected FE tests now pass with fresh XML, zero failures/errors/skips and unchanged inputs. An unused empty mount does not block a separate established remote write route. ########## fe/fe-core/src/main/java/org/apache/doris/tablefunction/HadoopTvfAdmission.java: ########## @@ -0,0 +1,476 @@ +// 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.tablefunction; + +import org.apache.doris.common.Config; +import org.apache.doris.common.util.LocationPath; +import org.apache.doris.nereids.exceptions.AnalysisException; + +import java.net.URI; +import java.net.URISyntaxException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +/** + * TVF-only admission for Hadoop consumers. Input properties have already been normalized by storage. + * This module neither loads Hadoop resources nor constructs a filesystem. Java strings and native + * C strings are projected separately before filesystem selection and mount-table interpretation. + */ +public final class HadoopTvfAdmission { + public enum Access { + NATIVE_WRITE, + JAVA_DELETE, + HADOOP_READ + } + + private enum Provider { + REMOTE, + LOCAL, + MOUNT_TABLE, + DYNAMIC, + UNKNOWN + } + + private static final Set<String> REMOTE_SCHEMES = new HashSet<>(Arrays.asList( + "hdfs", "s3a", "s3n", "oss", "ofs", "jfs", "abfs", "abfss", "wasb", "wasbs", "gs", "cosn")); + private static final Set<String> REMOTE_IMPLEMENTATIONS = new HashSet<>(Arrays.asList( + "org.apache.hadoop.hdfs.DistributedFileSystem", + "org.apache.hadoop.fs.s3a.S3AFileSystem", + "org.apache.hadoop.fs.azurebfs.AzureBlobFileSystem", + "org.apache.hadoop.fs.azurebfs.SecureAzureBlobFileSystem", + "org.apache.hadoop.fs.azure.NativeAzureFileSystem", + "org.apache.hadoop.fs.aliyun.oss.AliyunOSSFileSystem", + "com.aliyun.jindodata.oss.JindoOssFileSystem", + "org.apache.hadoop.fs.obs.OBSFileSystem", + "com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystem")); + + private final Access access; + private final Map<String, String> properties = new HashMap<>(); + + private HadoopTvfAdmission(Access access, Map<String, String> normalized, boolean nativeConsumer) { + this.access = access; + for (Map.Entry<String, String> entry : normalized.entrySet()) { + // parse_properties / generateHdfsParam consumes the canonical NameNode before JNI. + if (nativeConsumer && "fs.defaultFS".equalsIgnoreCase(entry.getKey())) { + continue; + } + String key = (nativeConsumer ? nativeStringPrefix(entry.getKey()) : entry.getKey()).trim(); + String value = nativeConsumer ? nativeStringPrefix(entry.getValue()) : entry.getValue(); + String previous = properties.put(key, value); Review Comment: Fixed in dc19f3fea28bb9b2705a734f3105f9a8fcb09c29. Native writer admission now rejects routing keys with equivalent identities under the actual map comparator: the ASCII-folded C-string prefix together with the original UTF-8 byte length. This happens before selecting the admitted mount and does not depend on which equivalent entry a map implementation keeps. The extra map check is limited to NATIVE_WRITE, whose consumer uses parse_properties/StringCaseMap. Read requests that generate Thrift configuration directly retain their existing projection. Tests cover the case/NUL collision, unequal raw lengths, nonrouting controls, normal read admission and explicit writer opt-in. A small oracle using the production comparator confirmed its equality behavior. Before tests reproduced the missing rejection; all 55 selected FE tests and Checkstyle pass. Generic native maps/filesystems are unchanged. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
