OIiveirra commented on code in PR #69528:
URL: https://github.com/apache/doris/pull/69528#discussion_r4365918988
##########
fe/fe-core/src/main/java/org/apache/doris/tablefunction/S3TableValuedFunction.java:
##########
@@ -89,6 +106,32 @@ public S3TableValuedFunction(Map<String, String>
properties, List<TBrokerFileSta
}
+ private static boolean isRemoteLanceUri(String path) {
+ try {
+ URI uri = new URI(path == null ? "" : path);
+ String scheme = uri.getScheme();
+ String authority = uri.getRawAuthority();
+ if (scheme == null ||
!LANCE_REMOTE_SCHEMES.contains(scheme.toLowerCase(Locale.ROOT))
+ || uri.isOpaque()) {
+ return false;
+ }
+ if (authority == null || authority.isEmpty()) {
+ // GooseFS can resolve its remote master from storage options
or configuration.
+ return "goosefs".equalsIgnoreCase(scheme) && uri.getRawPath()
!= null
+ && uri.getRawPath().startsWith("/");
+ }
+ // Lance falls back to a local path if Rust's URL parser rejects
the URI. Java's URI
+ // accepts registry authorities with invalid ports, which Rust
rejects. Validate the
+ // host/port separately without disallowing legitimate bucket
names with underscores.
+ String host =
HostAndPort.fromString(authority.substring(authority.lastIndexOf('@') + 1))
Review Comment:
Fixed in 398acc158bbb9e2a9525a0960a9186f351d73fc6. The final remote Lance
URI guard now validates the original port spelling as ASCII digits, rejecting
signed zero before metadata loading. IPv6 is also validated without DNS using
`InetAddresses`, rejecting the leading-zero IPv4-tail case.
`S3TableValuedFunctionTest` covers `:-0`, `:-00`, `:+0`, the malformed
mapped IPv6 address, and valid port/IPv6 cases. Rejected inputs do not reach
the mocked metadata loader. All 35 selected FE tests passed.
##########
be/src/io/fs/local_file_system.cpp:
##########
@@ -455,28 +455,48 @@ 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));
Review Comment:
Fixed in 398acc158bbb9e2a9525a0960a9186f351d73fc6. LOCAL path validation now
decodes actual `file:` URI paths once before canonicalization, after removing
raw query/fragment components. Bare paths retain literal percent characters and
`+`; decoded NUL and out-of-root targets are rejected.
`LanceTableReaderLocalPathTest.FileUrisPreserveEscapedDatasetIdentity`
checks the canonical dataset key, real schema discovery and scanning for
escaped-space/literal-percent collisions, `%2520`, plus/percent names and
query/fragment handling. All 11 selected BE filesystem/Lance tests passed under
Clang21/ASAN; scans returned row IDs {1,2,3,4}.
--
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]
