github-actions[bot] commented on code in PR #69527:
URL: https://github.com/apache/doris/pull/69527#discussion_r4366070974
##########
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;
Review Comment:
[P2] Apply URL whitespace preprocessing before resolving file URIs
For an enabled, authorized LOCAL Lance read under `/srv/user_files`, let
`data.lance` and `data.lance ` contain different datasets. The URI
`file:///srv/user_files/data.lance ` (one raw trailing space) previously opened
`data.lance`: the pinned [URL parser trims surrounding C0/space before
parsing](https://github.com/servo/rust-url/blob/v2.5.8/url/src/parser.rs#L225-L240).
FE preserves the property value, but this helper retains that space and
canonicalizes the other dataset. Passing the resulting absolute bare path to
Lance preserves its literal space, so schema discovery and scanning return the
wrong dataset. Match URI preprocessing before normalization, while keeping
whitespace literal for bare paths; cover both identities in tests.
##########
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:
[P2] Preserve absolute semantics for file URLs without a slash
With `enable_local_tvf=true`, an ADMIN/OPERATOR can request LOCAL Lance with
`file_path="file:srv/user_files/data.lance"` under
`user_files_secure_path=/srv/user_files`. The pinned [URL parser treats this as
`file:///srv/user_files/data.lance`](https://github.com/servo/rust-url/blob/v2.5.8/url/src/parser.rs#L700-L706),
but `convert_to_abs_path` returns relative `srv/user_files/data.lance`, which
line 557 joins beneath the secure root. If both datasets exist, schema
discovery and scanning silently read
`/srv/user_files/srv/user_files/data.lance`; otherwise the valid original
fails. Neither escapes nor dot segments are involved. Apply file-URL root
semantics before canonical containment, preserve bare relative paths, and add a
distinguishable-dataset regression.
--
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]
