github-actions[bot] commented on code in PR #69527:
URL: https://github.com/apache/doris/pull/69527#discussion_r4365942546
##########
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:
[P2] Normalize file-URI dot segments before following symlinks
For an enabled, authorized LOCAL Lance read, let the secure root be
`/srv/user_files`, let `jump` link to `branch/deep`, and let both `data.lance`
and `branch/data.lance` contain different datasets.
`file:///srv/user_files/jump/../data.lance` denotes
`/srv/user_files/data.lance` to Lance: its [pinned URL
parser](https://github.com/servo/rust-url/blob/v2.5.8/url/src/parser.rs#L1317-L1328)
removes `jump/..` before filesystem traversal. This helper instead passes the
unnormalized path to `canonicalize`, which follows `jump` first and selects
`/srv/user_files/branch/data.lance`. Both pass containment, so schema discovery
and scanning silently use the wrong dataset. A removed intermediate directory
that does not exist also turns a valid URI into an error. This is separate from
the earlier percent-decoding finding: even an entirely unescaped file URI
triggers it in the new helper. Apply SDK-compatible URL normalization to actual
file URIs before filesystem canonicalization, preserve bar
e-path semantics, and add a symlink-plus-parent 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]
