github-actions[bot] commented on code in PR #69528:
URL: https://github.com/apache/doris/pull/69528#discussion_r4366195704
##########
be/src/format_v2/table/lance_reader.cpp:
##########
@@ -35,11 +35,50 @@
#include "format_v2/lance/lance_reader_helper.h"
#include "format_v2/lance/lance_runtime_filter_helper.h"
#include "format_v2/lance/lance_session_manager.h"
+#include "io/fs/local_file_system.h"
#include "runtime/file_scan_profile.h"
#include "runtime/runtime_state.h"
namespace doris::format::lance {
+namespace {
+Status resolve_dataset_uri(const TFileRangeDesc& range, const
TFileScanRangeParams& scan_params,
+ std::string* uri) {
+ const auto& requested_uri =
range.table_format_params.lance_params.dataset_uri;
+ // Modern plans tag each range; older plans put the type in the scan
parameters.
+ // Untagged catalog scans retain their original storage-provider selection.
+ const bool is_local = range.__isset.file_type
+ ? range.file_type == TFileType::FILE_LOCAL
+ : scan_params.__isset.file_type &&
+ scan_params.file_type ==
TFileType::FILE_LOCAL;
+ // LOCAL TVF uses version zero for both schema discovery and whole-dataset
scans.
+ // Catalog scans pin a positive snapshot version, including file://
catalogs whose
+ // operator-configured warehouse is independent of the LOCAL TVF secure
directory.
+ if (is_local && range.table_format_params.lance_params.version == 0) {
+ // LOCAL Lance bypasses glob because it opens a dataset directory.
Apply the same
+ // canonical secure-root check before either schema discovery or a
cached dataset open.
+ std::string local_input = requested_uri;
+ const auto colon = requested_uri.find(':');
+ const auto slash = requested_uri.find('/');
+ const bool bare_path =
+ colon == std::string::npos || (slash != std::string::npos &&
slash < colon);
+ if (bare_path && !requested_uri.empty() && requested_uri.find('\0') ==
std::string::npos &&
+ !config::user_files_secure_path.empty()) {
+ // Lance's PathAbs fallback resolves bare dot segments before
following symlinks.
+ // Anchor relative TVF paths to the canonical secure root, not the
process cwd.
+ std::string secure_root;
+ RETURN_IF_ERROR(io::global_local_filesystem()->canonicalize(
+ config::user_files_secure_path, &secure_root));
+ local_input = (io::Path(secure_root) /
requested_uri).lexically_normal().string();
Review Comment:
[P2] Preserve Lance's home expansion before anchoring bare paths
With `enable_local_tvf=true`, an ADMIN/OPERATOR, BE home `/home/doris`, and
`user_files_secure_path=/home/doris/user_files`,
`local("file_path"="~/user_files/data.lance", "backend_id"="1",
"format"="lance")` previously opened `/home/doris/user_files/data.lance`,
inside the secure root. The [pinned SDK expands `~` and `~/` before
PathAbs](https://github.com/lance-format/lance/blob/68c12dfd7efe02f90ad2d7f3a239a7eb64884e57/rust/lance-io/src/object_store.rs#L539-L564).
Prefixing the raw input here instead produces
`/home/doris/user_files/~/user_files/data.lance`, suppressing that expansion.
Schema discovery now fails for a valid dataset, or silently discovers and scans
the other dataset if both locations exist. Both the schema RPC and LanceSplit
retain the original string and call this resolver. This home-alias case is
distinct from the existing dot-segment findings and needs no dots or symlinks.
Expand supported bare home aliases before lexical normalization and canonical
containment
; add schema/scan cases with distinguishable datasets and an out-of-root
expanded target.
##########
be/src/io/fs/local_file_system.cpp:
##########
@@ -455,28 +455,158 @@ const std::shared_ptr<LocalFileSystem>&
global_local_filesystem() {
Status LocalFileSystem::canonicalize_local_file(const std::string& dir,
const std::string& file_path,
std::string* full_path) {
- const std::string absolute_path = dir + "/" + file_path;
+ // An empty root would turn a relative request into an unrestricted
absolute path.
+ if (dir.empty()) {
+ return Status::InvalidArgument("user_files_secure_path must not be
empty");
+ }
+ // 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] = '/';
+ }
+ }
+ }
+ Path local_path;
+ // 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));
+ if (is_file_uri) {
+ // file:a%20b names "a b", but a bare path containing "%20" must stay
literal.
+ // Decode once, preserve '+' and invalid escapes, matching Lance's
file provider.
+ auto encoded = local_path.string();
+ // Query and fragment are URI components, not part of the filesystem
path.
+ encoded = encoded.substr(0, encoded.find_first_of("?#"));
+ // A file URL without a leading slash still names a filesystem-rooted
path.
+ if (encoded.empty() || encoded[0] != '/') {
+ encoded.insert(0, "/");
+ }
+ // URL dot segments are removed before symlinks are followed and
before %2F
+ // becomes a path separator. Keep empty segments: "a//../b" denotes
"a/b".
+ std::vector<std::string> segments;
+ for (size_t start = 1;;) {
+ const size_t end = encoded.find('/', start);
+ const auto segment =
+ encoded.substr(start, end == std::string::npos ? end : end
- start);
+ const bool dot = segment == "." || segment == "%2e" || segment ==
"%2E";
+ const bool parent = segment == ".." || segment == ".%2e" ||
segment == ".%2E" ||
+ segment == "%2e." || segment == "%2E." ||
segment == "%2e%2e" ||
+ segment == "%2e%2E" || segment == "%2E%2e" ||
segment == "%2E%2E";
+ if (parent) {
+ if (!segments.empty()) {
+ segments.pop_back();
+ }
+ } else if (!dot) {
+ segments.push_back(segment);
Review Comment:
[P2] Preserve initial drive-segment normalization for file URIs
For enabled, authorized LOCAL Lance on Linux with
`user_files_secure_path=/C:` and an existing `/C:/data.lance`,
`file_path="file:///C|/data.lance"` previously opened that in-root dataset. The
[pinned URL parser normalizes an initial file-URI `C|` segment to `C:` on every
OS](https://github.com/servo/rust-url/blob/v2.5.8/url/src/parser.rs#L1343-L1355).
This loop retains `C|`, so the new pre-open canonicalization resolves
`/C|/data.lance` and fails or rejects the valid read. With secure root `/` and
datasets at both spellings, schema discovery and scanning silently select the
pipe-named dataset instead. FE preserves the original dataset URI, so this
input reaches both BE paths. This is a distinct first-segment grammar rule from
the existing percent, dot and whitespace cases; it requires the uncommon
root-level drive-name layout described above. Apply SDK-compatible drive
normalization to actual file URIs before filesystem resolution, retaining
literal bare Unix names, and add a sch
ema/scan identity case.
--
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]
