adoroszlai commented on a change in pull request #1582:
URL: https://github.com/apache/ozone/pull/1582#discussion_r522893582
##########
File path:
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
##########
@@ -840,8 +842,12 @@ public String pathToKey(Path path) {
if (!path.isAbsolute()) {
path = new Path(workingDir, path);
}
+ String key = path.toUri().getPath();
+ if (!OzoneFSUtils.isValidName(path.toUri().getPath())) {
Review comment:
Avoid unnecessary duplicate conversion:
```suggestion
if (!OzoneFSUtils.isValidName(key)) {
```
##########
File path:
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
##########
@@ -1210,4 +1212,33 @@ public void testTrash() throws Exception {
ofs.delete(trashRoot, true);
}
+
+ @Test
+ public void testCreateWithInvalidPaths() throws Exception {
+ // Test for path with ..
+ Path parent = new Path("../../../../../d1/d2/");
+ Path file1 = new Path(parent, "key1");
+ checkInvalidPath(file1);
+
+ // Test for path with :
+ file1 = new Path("/:/:");
+ checkInvalidPath(file1);
+
+ // Test for path with scheme and authority.
+ file1 = new Path(fs.getUri() + "/:/:");
+ checkInvalidPath(file1);
+ }
+
+ private void checkInvalidPath(Path path) throws Exception {
+ FSDataOutputStream outputStream = null;
+ try {
+ LambdaTestUtils.intercept(InvalidPathException.class, "Invalid path
Name",
+ () -> fs.create(path, false));
+ } finally {
+ if (outputStream != null) {
Review comment:
Seems like `outputStream` is never updated to non-`null` value, hence
unnecessary.
##########
File path:
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java
##########
@@ -817,7 +817,7 @@ public String pathToKey(Path path) {
if (OzoneFSUtils.isValidName(key)) {
key = path.toUri().getPath();
} else {
Review comment:
Not introduced in this patch, but would be nice to remove duplicate
conversion:
```suggestion
if (!OzoneFSUtils.isValidName(key)) {
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
