OIiveirra commented on code in PR #69528:
URL: https://github.com/apache/doris/pull/69528#discussion_r4371079593
##########
be/src/io/fs/local_file_system.cpp:
##########
@@ -452,31 +452,206 @@ const std::shared_ptr<LocalFileSystem>&
global_local_filesystem() {
return local_fs;
}
+Status LocalFileSystem::parse_local_path(const std::string& file_path, Path&
local_path) {
+ // Reject embedded NULs before passing paths to filesystem APIs that use C
strings.
+ if (file_path.empty() || file_path.find('\0') != std::string::npos) {
+ return Status::InvalidArgument("local file path must not be empty or
contain NUL");
+ }
+ // Match URL preprocessing only for actual file URIs; bare paths keep
literal whitespace.
+ const auto is_c0_or_space = [](unsigned char c) { return c <= 0x20; };
+ size_t begin = 0;
+ size_t end = file_path.size();
+ while (begin < end && is_c0_or_space(file_path[begin])) {
+ ++begin;
+ }
+ while (end > begin && is_c0_or_space(file_path[end - 1])) {
+ --end;
+ }
+ std::string uri_input;
+ for (size_t i = begin; i < end; ++i) {
+ if (file_path[i] != '\t' && file_path[i] != '\r' && file_path[i] !=
'\n') {
+ uri_input += file_path[i];
+ }
+ }
+ const bool is_file_uri = uri_input.size() >= 5 &&
+ (uri_input[0] == 'f' || uri_input[0] == 'F') &&
+ (uri_input[1] == 'i' || uri_input[1] == 'I') &&
+ (uri_input[2] == 'l' || uri_input[2] == 'L') &&
+ (uri_input[3] == 'e' || uri_input[3] == 'E') &&
uri_input[4] == ':';
+ std::string local_input = is_file_uri ? std::move(uri_input) : file_path;
+ if (is_file_uri) {
+ local_input.replace(0, 4, "file");
+ // File URLs treat raw backslashes as separators, including before the
authority.
+ // Do this before conversion; percent-encoded backslashes must remain
literal.
+ for (size_t i = 5; i < local_input.size(); ++i) {
+ if (local_input[i] == '\\') {
+ local_input[i] = '/';
+ }
+ }
+ }
+ const auto hex_digit = [](char c) -> int {
+ if (c >= '0' && c <= '9') return c - '0';
+ if (c >= 'a' && c <= 'f') return c - 'a' + 10;
+ if (c >= 'A' && c <= 'F') return c - 'A' + 10;
+ return -1;
+ };
+ const auto percent_decode = [&](const std::string& encoded) {
+ std::string decoded;
+ decoded.reserve(encoded.size());
+ for (size_t i = 0; i < encoded.size(); ++i) {
+ if (encoded[i] == '%' && i + 2 < encoded.size()) {
+ const int high = hex_digit(encoded[i + 1]);
+ const int low = hex_digit(encoded[i + 2]);
+ if (high >= 0 && low >= 0) {
+ decoded += static_cast<char>((high << 4) | low);
+ i += 2;
+ continue;
+ }
+ }
+ decoded += encoded[i];
+ }
+ return decoded;
+ };
+ const auto is_drive = [](const std::string& segment) {
+ return segment.size() == 2 &&
+ ((segment[0] >= 'a' && segment[0] <= 'z') ||
+ (segment[0] >= 'A' && segment[0] <= 'Z')) &&
+ (segment[1] == ':' || segment[1] == '|');
+ };
+ bool empty_file_host = false;
+ if (is_file_uri && local_input.compare(5, 2, "//") == 0) {
+ const auto host_end = local_input.find_first_of("/?#", 7);
+ auto host = local_input.substr(7, host_end == std::string::npos ?
host_end : host_end - 7);
+ if (is_drive(host)) {
+ // The URL parser treats a drive in the host position as the first
path segment.
+ local_input.erase(5, 2);
+ } else {
+ host = percent_decode(host);
+ std::transform(host.begin(), host.end(), host.begin(), [](unsigned
char c) {
+ return c >= 'A' && c <= 'Z' ? c + ('a' - 'A') : c;
+ });
+ empty_file_host = host.empty() || host == "localhost";
+ }
+ }
+ // Convert file: URIs to filesystem paths and reject remote schemes before
any reader opens them.
+ RETURN_IF_ERROR(convert_to_abs_path(local_input, local_path));
Review Comment:
Fixed in 95b42fa44940649bc4492d5201ca79f3778154d4. Every non-bare LOCAL
Lance URI now uses the actual local_path returned by the existing pinned SDK
metadata parser before canonical containment and tree validation. This retains
the SDK's whole-original-string fallback identity for malformed file
authorities. Bare relative/home paths keep secure-root anchoring, and aliases
keep their selected local provider.
The real two-dataset regression reproduced the wrong schema/key/row 7 before
the repair; schema discovery, cache identity and actual scanning now select the
SDK fallback dataset and row 42. Out-of-root fallback rejects. Both releases
passed 20 selected filesystem/Lance tests; generic LocalFS, SDK ABI and
manifest-base policy remain unchanged. The pinned SDK also confirmed that the
old decoded-parent success expectation was invalid, so it is covered as a
schema/scan rejection.
##########
fe/be-java-extensions/hadoop-deps/src/main/java/org/apache/hadoop/fs/FileSystem.java:
##########
@@ -3622,13 +3633,670 @@ public static Class<? extends FileSystem>
getFileSystemClass(String scheme,
* @return the initialized filesystem.
* @throws IOException problems loading or initializing the FileSystem
*/
+ // DORIS-PATCH: TVF policy is checked against the frozen configuration used
by the consumer.
+ public static int getDorisTvfGuardVersion() {
+ return 1;
+ }
+
+ public static final class TvfGuard {
+ public static final String POLICY_KEY = "fs.doris.tvf.remote.paths";
+ public static final String NATIVE_POLICY = "native";
+ public static final String JAVA_READ_POLICY = "java-read";
+ public static final String ROOT_KEY = "fs.doris.tvf.root";
+ public static final String PATH_KEY = "fs.doris.tvf.path";
+ public static final String CONTEXT_KEY = "fs.doris.tvf.context";
+ private static final ThreadLocal<Context> LAST_NATIVE_CONTEXT = new
ThreadLocal<>();
+
+ private TvfGuard() {
+ }
+
+ public static final class Rule {
+ private final String filesystem;
+ private final String path;
+ private final boolean descendants;
+
+ public Rule(String filesystem, String path, boolean descendants) {
+ this.filesystem = filesystem;
+ this.path = path;
+ this.descendants = descendants;
+ }
+ }
+
+ public static String encode(List<Rule> rules) {
+ if (FileSystem.getDorisTvfGuardVersion() != 1) {
+ throw new IllegalStateException("TVF Hadoop runtime policy version
mismatch");
+ }
+ try {
+ java.io.ByteArrayOutputStream bytes = new
java.io.ByteArrayOutputStream();
+ java.io.DataOutputStream out = new java.io.DataOutputStream(bytes);
+ out.writeInt(1);
+ out.writeInt(rules.size());
+ for (Rule rule : rules) {
+ out.writeUTF(rule.filesystem);
+ out.writeUTF(rule.path);
+ out.writeBoolean(rule.descendants);
+ }
+ out.flush();
+ return
java.util.Base64.getEncoder().encodeToString(bytes.toByteArray());
+ } catch (IOException e) {
+ throw new IllegalArgumentException("Cannot encode TVF Hadoop route
policy", e);
+ }
+ }
+
+ /** Routes touched by Globber, without constructing a filesystem or
listing data. */
+ public static List<Rule> globRules(URI filesystem, String pattern) throws
IOException {
+ List<Rule> rules = new ArrayList<>();
+ rules.add(new Rule(filesystem.toString(), pattern, true));
+ if (pattern.indexOf('\\') < 0) {
+ return rules;
+ }
+ for (String expanded : GlobExpander.expand(pattern)) {
+ Path flattened = new Path(expanded.isEmpty() ? Path.CUR_DIR :
expanded);
+ String path = flattened.toUri().getPath();
+ Path candidate = new Path(flattened.isAbsolute() ? "/" : Path.CUR_DIR);
+ for (String component : path.split("/")) {
+ if (component.isEmpty()) {
+ continue;
+ }
+ GlobFilter filter = new GlobFilter(component);
+ if (filter.hasPattern()) {
+ // Globber lists this parent before it knows the matching child
names. Keep
+ // prior wildcard components as a conservative subtree route,
including when
+ // a later escaped '..' moves the final candidate outside that
subtree.
+ rules.add(new Rule(filesystem.toString(), new Path(candidate,
"*").toUri().getPath(), true));
+ }
+ component = component.replaceAll("\\\\(.)", "$1");
+ candidate = new Path(candidate, component);
+ }
+ rules.add(new Rule(filesystem.toString(), candidate.toUri().getPath(),
true));
+ }
+ return rules;
+ }
+
+ private static IOException rejected() {
+ return new IOException("Hadoop TVF requires a proven remote filesystem
route. "
+ + "Use local() with LOCAL TVF privileges and secure-path
restrictions.");
+ }
+
+ private static Configuration freeze(Configuration conf) throws IOException
{
+ // Service providers (notably HDFS) register their default/site
resources on first load.
+ // Load them before copying raw values, as an ordinary FileSystem lookup
does.
+ if (!FILE_SYSTEMS_LOADED) {
+ loadFileSystems();
+ }
+ if (conf instanceof FrozenConfiguration) {
+ return conf;
+ }
+ return new FrozenConfiguration(conf);
+ }
+
+ private static Configuration prepare(Configuration conf) throws
IOException {
+ String policy = conf.getRaw(POLICY_KEY);
+ if (policy == null || policy.isEmpty()) {
+ return conf;
+ }
+ Configuration frozen = freeze(conf);
+ Context context = ((FrozenConfiguration) frozen).context;
+ if (NATIVE_POLICY.equals(policy)) {
+ context.checkNative(frozen.getRaw(ROOT_KEY), frozen.getRaw(PATH_KEY));
+ } else if (JAVA_READ_POLICY.equals(policy)) {
+ context.checkJava(new Path(frozen.getRaw(PATH_KEY)));
+ } else {
+ context.checkRules(policy);
+ }
+ return frozen;
+ }
+
+ public static void check(URI ignored, Configuration conf) throws
IOException {
+ prepare(conf);
+ }
+
+ public static void clearNativeContext() {
+ LAST_NATIVE_CONTEXT.remove();
+ }
+
+ public static Context takeNativeContext() {
+ Context result = LAST_NATIVE_CONTEXT.get();
+ LAST_NATIVE_CONTEXT.remove();
+ return result;
+ }
+
+ public static Configuration prepareJavaRead(Configuration conf, Path path,
String marker) throws IOException {
+ if (marker == null || marker.isEmpty()) {
+ return conf;
+ }
+ if (!NATIVE_POLICY.equals(marker)) {
+ throw rejected();
+ }
+ Configuration request = new Configuration(conf);
+ request.set(POLICY_KEY, JAVA_READ_POLICY);
+ request.set(ROOT_KEY, path.toUri().toString());
+ request.set(PATH_KEY, path.toString());
+ return prepare(request);
+ }
+
+ public static void checkJavaRead(FileSystem fs, Path path, String marker)
throws IOException {
+ if (marker == null || marker.isEmpty()) {
+ return;
+ }
+ if (!NATIVE_POLICY.equals(marker)) {
+ throw rejected();
+ }
+ // The marker is request-owned, never inferred from an unmarked cached
filesystem.
+ Configuration conf = fs.getConf();
+ if (JAVA_READ_POLICY.equals(conf.getRaw(POLICY_KEY))) {
+ // S3A derives a Configuration for bucket options. Its copy preserves
our internal
+ // marker and raw snapshot values, but not the private Configuration
subclass.
+ ((FrozenConfiguration) freeze(conf)).context.checkJava(path);
+ } else {
+ throw rejected();
+ }
+ }
+
+ private static final class FrozenConfiguration extends Configuration {
+ private final Context context;
+
+ private FrozenConfiguration(Configuration source) throws IOException {
+ super(false);
+ setClassLoader(source.getClassLoader());
+ java.util.SortedMap<String, String> values = new java.util.TreeMap<>();
+ for (Map.Entry<String, String> entry : source) {
+ String key = entry.getKey();
+ String value = source.getRaw(key);
+ set(key, value);
+ if (!PATH_KEY.equals(key) && !ROOT_KEY.equals(key) &&
!CONTEXT_KEY.equals(key)) {
+ values.put(key, value);
+ }
+ }
+ // ViewFS's private InnerCache folds authority case (A/a). Guarded
children must
+ // instead use FileSystem's exact-authority/profile cache below.
+ set("fs.viewfs.enable.inner.cache", "false");
Review Comment:
The retained native BrokerProperties-passthrough variant is also addressed
in 95b42fa44940649bc4492d5201ca79f3778154d4. The actual native TVF backend
payload now excludes the known output-path transport field after the native
first-NUL prefix, Configuration.trim and case-insensitive TVF-field projection.
The dedicated Thrift output path stays correct, while all real Hadoop
properties and authority/configuration isolation are retained.
The production sender/FrozenConfiguration/real ViewFS-cache test uses
ordered uppercase-before-lowercase keys, NUL aliases and pre-NUL whitespace.
Before fixture cleanup, two different output prefixes share the same
profile/client; a real Hadoop setting change remains isolated (client counts
1,1,2). All 125 selected FE/Avro tests pass. Native connection close/disconnect
retains its existing lifetime behavior; this removes the newly introduced
per-output-prefix cache dimension.
--
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]
