From 9f7aac66b9792b14e10c376f1ab5d259af75a7aa Mon Sep 17 00:00:00 2001 From: lgh Date: Fri, 3 Nov 2023 13:39:12 +0800 Subject: [PATCH 1/3] Fix isValidName error --- .../main/java/org/apache/hadoop/hdfs/DFSUtilClient.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSUtilClient.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSUtilClient.java index 71cff2e3915b0..a01d5411a1fd8 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSUtilClient.java +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSUtilClient.java @@ -661,9 +661,12 @@ public static boolean isValidName(String src) { String[] components = StringUtils.split(src, '/'); for (int i = 0; i < components.length; i++) { String element = components[i]; + // For Windows, we must allow the : in the drive letter. + if (Shell.WINDOWS && i == 1 && element.contains(":")) { + continue; + } if (element.equals(".") || - // For Windows, we must allow the : in the drive letter. - (!Shell.WINDOWS && i == 1 && element.contains(":")) || + (element.contains(":")) || (element.contains("/"))) { return false; } From 5a5ede8be1dcfc73bf4f55c1e012f547f33b1c86 Mon Sep 17 00:00:00 2001 From: lgh Date: Fri, 3 Nov 2023 20:47:30 +0800 Subject: [PATCH 2/3] add test case --- .../src/main/java/org/apache/hadoop/hdfs/DFSUtilClient.java | 2 +- .../src/test/java/org/apache/hadoop/hdfs/TestDFSUtil.java | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSUtilClient.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSUtilClient.java index a01d5411a1fd8..b2fc472aad835 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSUtilClient.java +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSUtilClient.java @@ -662,7 +662,7 @@ public static boolean isValidName(String src) { for (int i = 0; i < components.length; i++) { String element = components[i]; // For Windows, we must allow the : in the drive letter. - if (Shell.WINDOWS && i == 1 && element.contains(":")) { + if (Shell.WINDOWS && i == 1 && element.endsWith(":")) { continue; } if (element.equals(".") || diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSUtil.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSUtil.java index f8e8e4120c43f..aa5b8c450fe4d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSUtil.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSUtil.java @@ -83,6 +83,7 @@ import org.apache.hadoop.security.alias.JavaKeyStoreProvider; import org.apache.hadoop.test.GenericTestUtils; import org.apache.hadoop.test.LambdaTestUtils; +import org.apache.hadoop.util.Shell; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -872,6 +873,11 @@ public void testIsValidName() { assertTrue(DFSUtil.isValidName("/bar/")); assertFalse(DFSUtil.isValidName("/foo/:/bar")); assertFalse(DFSUtil.isValidName("/foo:bar")); + if (Shell.WINDOWS) { + assertTrue(DFSUtil.isValidName("/C:/some/path")); + } else { + assertFalse(DFSUtil.isValidName("/C:/some/path")); + } } @Test(timeout=5000) From 8eec4d1ef52ce1d3368cd97ea5081590681089b5 Mon Sep 17 00:00:00 2001 From: lgh Date: Fri, 10 Nov 2023 09:53:11 +0800 Subject: [PATCH 3/3] change test case to output more info --- .../org/apache/hadoop/hdfs/TestDFSUtil.java | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSUtil.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSUtil.java index aa5b8c450fe4d..5d7110d3d9a8b 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSUtil.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSUtil.java @@ -866,17 +866,24 @@ public void testLocalhostReverseLookup() { @Test (timeout=15000) public void testIsValidName() { - assertFalse(DFSUtil.isValidName("/foo/../bar")); - assertFalse(DFSUtil.isValidName("/foo/./bar")); - assertFalse(DFSUtil.isValidName("/foo//bar")); - assertTrue(DFSUtil.isValidName("/")); - assertTrue(DFSUtil.isValidName("/bar/")); - assertFalse(DFSUtil.isValidName("/foo/:/bar")); - assertFalse(DFSUtil.isValidName("/foo:bar")); + String validPaths[] = new String[]{"/", "/bar/"}; + for (String path : validPaths) { + assertTrue("Should have been accepted '" + path + "'", DFSUtil.isValidName(path)); + } + + String invalidPaths[] = + new String[]{"/foo/../bar", "/foo/./bar", "/foo//bar", "/foo/:/bar", "/foo:bar"}; + for (String path : invalidPaths) { + assertFalse("Should have been rejected '" + path + "'", DFSUtil.isValidName(path)); + } + + String windowsPath = "/C:/foo/bar"; if (Shell.WINDOWS) { - assertTrue(DFSUtil.isValidName("/C:/some/path")); + assertTrue("Should have been accepted '" + windowsPath + "' in windows os.", + DFSUtil.isValidName(windowsPath)); } else { - assertFalse(DFSUtil.isValidName("/C:/some/path")); + assertFalse("Should have been rejected '" + windowsPath + "' in unix os.", + DFSUtil.isValidName(windowsPath)); } }