OIiveirra commented on code in PR #69528:
URL: https://github.com/apache/doris/pull/69528#discussion_r4366160077
##########
be/src/io/fs/local_file_system.cpp:
##########
@@ -455,28 +455,89 @@ 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");
+ }
+ 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(file_path, local_path));
+ if (file_path.rfind("file:", 0) == 0) {
+ // 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("?#"));
+ 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;
+ };
+ 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];
+ }
+ // %00 must not truncate the path when canonicalization reaches C
filesystem APIs.
+ if (decoded.find('\0') != std::string::npos) {
+ return Status::InvalidArgument("local file path must not contain
NUL");
+ }
+ local_path = std::move(decoded);
+ }
+ std::string secure_path;
+ RETURN_IF_ERROR(canonicalize(dir, &secure_path));
std::string canonical_path;
- RETURN_IF_ERROR(canonicalize(absolute_path, &canonical_path));
- if (!contain_path(dir, canonical_path)) {
- return Status::InvalidArgument("file path is not allowed: {}",
canonical_path);
+ const auto status = canonicalize(Path(secure_path) / local_path,
&canonical_path);
Review Comment:
The latest review's bare-path variant is now fixed in
0c4fa140e43bd3be71e097713f0cbd3f25d38c8c. LOCAL Lance/version-zero paths use
the pinned PathAbs order: anchor relative paths to the canonical secure root,
remove bare dot segments lexically, then resolve symlinks and enforce
containment.
The earlier Lance bare-path test expectation was incorrect and has been
changed from the nested dataset (42) to the root dataset (7), with absolute and
relative controls. Ordinary LocalFS/glob physical-path semantics remain
unchanged. Empty/NUL paths, including a NUL before a removable parent segment,
remain rejected. All 13 selected BE tests passed.
##########
be/src/io/fs/local_file_system.cpp:
##########
@@ -455,28 +455,142 @@ 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");
+ }
+ // URI schemes are ASCII case-insensitive; do not alter spelling in bare
paths.
+ const bool is_file_uri = file_path.size() >= 5 &&
+ (file_path[0] == 'f' || file_path[0] == 'F') &&
+ (file_path[1] == 'i' || file_path[1] == 'I') &&
+ (file_path[2] == 'l' || file_path[2] == 'L') &&
+ (file_path[3] == 'e' || file_path[3] == 'E') &&
file_path[4] == ':';
+ std::string local_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));
Review Comment:
Fixed in 0c4fa140e43bd3be71e097713f0cbd3f25d38c8c. URL whitespace/control
preprocessing now occurs before recognizing an actual file URI. Raw surrounding
C0/space and URL tab/newline preprocessing follow the pinned parser; literal
bare-path whitespace and percent-encoded spaces are preserved. Existing NUL
rejection and canonical containment remain in place.
Tests distinguish normal row 7, a literal-space dataset with row 99, and the
other path-identity collisions, checking schema and actual BE scans. Raw-space
file URIs select row 7; `%20` URIs and bare paths with a literal trailing space
select row 99. All 13 selected filesystem/Lance tests passed with fresh XML and
unchanged source.
--
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]
