Copilot commented on code in PR #13721:
URL: https://github.com/apache/gravitino/pull/13721#discussion_r4228260400
##########
catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetCatalogOperations.java:
##########
@@ -1796,11 +1796,18 @@ 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.
+ // Backslashes can be interpreted as separators by a client on a different
operating system.
+ Preconditions.checkArgument(
+ subPath.indexOf('\\') < 0 && subPath.indexOf('\0') < 0,
+ "subPath must not contain backslashes or null characters");
+ for (String segment : processedSubPath.split(SLASH)) {
+ Preconditions.checkArgument(
+ !"..".equals(segment), "subPath must not contain parent directory
segments");
Review Comment:
This adds user-visible validation to both the Java catalog API and the REST
endpoints, but their contracts remain stale: `FilesetCatalog#getFileLocation`
does not describe the rejected sub-path forms or `IllegalArgumentException`,
and both operations in `docs/open-api/filesets.yaml` omit the new HTTP 400
response. Please update those API docs so clients can discover and handle the
new behavior.
--
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]
