OIiveirra commented on code in PR #69527:
URL: https://github.com/apache/doris/pull/69527#discussion_r4366159769


##########
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 0d96b297bd650f18441aa054b9a4cc49cedb6ed5. Actual file URLs without 
a slash are now resolved from the filesystem root, matching the pinned URL 
parser, before canonical secure-root containment. Bare relative Lance paths 
continue to resolve under the canonical secure root.
   
   Expanded tests use a normal dataset (row 7) and a duplicated-prefix dataset 
(row 123), asserting schema, canonical identity and direct BE reads. All 13 
selected BE tests passed. These tests cover BE schema/direct-reader resolution; 
the existing FE Hadoop execution check for rootless URLs remains unchanged, so 
this does not add rootless SQL scan support.



##########
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:
   Fixed in 0d96b297bd650f18441aa054b9a4cc49cedb6ed5. 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]

Reply via email to