OIiveirra commented on code in PR #69528:
URL: https://github.com/apache/doris/pull/69528#discussion_r4366382140
##########
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:
Fixed in 66d2b918a4cbcc4a5dca2271396700da91bbe365. Only LOCAL/version-zero
bare paths expand SDK-supported `~` and `~/` before lexical normalization and
canonical secure-root containment. HOME is used first with a thread-safe passwd
fallback, matching the SDK; named-user spellings and file-URI literal tildes
are preserved.
All 15 selected BE tests passed. Distinct real Lance fixtures verify schema,
canonical dataset key and actual rows for the home target versus the
literal-tilde target, plus an expanded target outside the secure root. The
tests use a unique fixture under the existing home and never override HOME.
Remote ranges and fixed-version catalogs retain their routing.
##########
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:
The exact triple-slash example has been checked against pinned url 2.5.8 on
Unix: `file:///C|/data.lance` produces `/C|/data.lance`, whereas
`file:/C|/data.lance`, `file:C|/data.lance` and `file://C|/data.lance` produce
`/C:/data.lance`. The triple-slash identity is therefore intentionally retained.
The related drive-parser gaps are fixed in
66d2b918a4cbcc4a5dca2271396700da91bbe365: single-slash/rootless/authority drive
handling, and raw-drive protection from a following parent segment, are matched
to the SDK before containment. The legacy converter and literal bare Unix names
are unchanged.
All 15 selected BE tests passed. A 19-form pure-parser matrix distinguishes
these spellings, encoded pipes and localhost authorities (18 URL expectations
independently checked against the pinned SDK). Real nested-drive datasets also
verify distinct schemas and actual scanned rows. No root-level fixture is
created.
--
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]
