mchades commented on code in PR #13721:
URL: https://github.com/apache/gravitino/pull/13721#discussion_r4246345329
##########
catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetCatalogOperations.java:
##########
@@ -1796,11 +1796,16 @@ private String failureCategory(Throwable throwable) {
private String getFileLocation(Fileset fileset, String subPath, String
locationName) {
Preconditions.checkArgument(subPath != null, "subPath must not be null");
- String processedSubPath;
- if (!subPath.trim().isEmpty() && !subPath.trim().startsWith(SLASH)) {
- processedSubPath = SLASH + subPath.trim();
- } else {
- processedSubPath = subPath.trim();
+ String processedSubPath = subPath.trim();
+ // Validate before constructing a Hadoop Path, which normalizes away
parent directory segments.
+ Preconditions.checkArgument(
+ subPath.indexOf('\0') < 0, "subPath must not contain null characters");
Review Comment:
Hadoop 3.3.6 preserves backslashes in these `s3a:`, `hdfs:` and `gs:` paths
even on Windows; `normalizePath` restricts conversion to local/drive-style
paths. The `file:` case can differ across platforms. Clarified in the PR
description that this change validates server-side Hadoop path resolution,
while retaining literal-name compatibility. Hadoop `Path` also preserves `.. `;
a downstream trimming case needs a concrete consumer.
##########
docs/open-api/filesets.yaml:
##########
@@ -550,6 +564,19 @@ components:
}
responses:
+ InvalidSubPathErrorResponse:
+ description: >-
+ The sub path does not satisfy the fileset path requirements.
+ For file listing, this response also applies when the sub path does
not exist.
Review Comment:
Kept the shared 400 response generic and moved the existence requirement
into the `/files` operation's `sub_path` description. `/location` no longer
displays listing-specific text.
--
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]
