[
https://issues.apache.org/jira/browse/HADOOP-19130?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17837648#comment-17837648
]
ASF GitHub Bot commented on HADOOP-19130:
-----------------------------------------
ayushtkn commented on code in PR #6678:
URL: https://github.com/apache/hadoop/pull/6678#discussion_r1567096724
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/ftp/TestFTPFileSystem.java:
##########
@@ -236,4 +241,52 @@ public void testFTPSetTimeout() {
ftp.setTimeout(client, conf);
assertEquals(client.getControlKeepAliveTimeout(), timeout);
}
-}
\ No newline at end of file
+
+
+ private static void touch(FileSystem fs, Path filePath)
+ throws IOException {
+ touch(fs, filePath, null);
+ }
+
+ private static void touch(FileSystem fs, Path path, byte[] data)
+ throws IOException {
+ FSDataOutputStream out = null;
+ try {
+ out = fs.create(path);
+ if (data != null) {
+ out.write(data);
+ }
+ } finally {
+ if (out != null) {
+ out.close();
+ }
+ }
+ }
+
+ /**
+ * Test renaming a file.
+ *
+ * @throws Exception
+ */
+ @Test
+ public void testRenameFileWithFullQualifiedPath() throws Exception {
+ BaseUser user = server.addUser("test", "password", new WritePermission());
+ Configuration configuration = new Configuration();
+ configuration.set("fs.defaultFS", "ftp:///";);
+ configuration.set("fs.ftp.host", "localhost");
+ configuration.setInt("fs.ftp.host.port", server.getPort());
+ configuration.set("fs.ftp.user.localhost", user.getName());
+ configuration.set("fs.ftp.password.localhost", user.getPassword());
+ configuration.setBoolean("fs.ftp.impl.disable.cache", true);
+
+ FileSystem fs = FileSystem.get(configuration);
+
+
+ Path ftpDir = fs.makeQualified(new
Path(testDir.toAbsolutePath().toString()));
+ Path file1 = fs.makeQualified(new Path(ftpDir,
name.getMethodName().toLowerCase() + "1"));
+ Path file2 = fs.makeQualified(new Path(ftpDir,
name.getMethodName().toLowerCase() + "2"));
+ touch(fs, file1);
+ assertTrue(fs.rename(file1, file2));
+ }
+
Review Comment:
nit
avoid empty line
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/ftp/TestFTPFileSystem.java:
##########
@@ -60,6 +62,9 @@ public class TestFTPFileSystem {
@Rule
public Timeout testTimeout = new Timeout(180000, TimeUnit.MILLISECONDS);
+ @Rule
+ public TestName name = new TestName();
+
Review Comment:
this ain't required as such, it is just for the directory name sake, put it
directly over there rather than doing name.getMethodName
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ftp/FTPFileSystem.java:
##########
@@ -685,21 +685,21 @@ private boolean rename(FTPClient client, Path src, Path
dst)
throw new FileAlreadyExistsException("Destination path " + dst
+ " already exists");
}
- String parentSrc = absoluteSrc.getParent().toUri().toString();
- String parentDst = absoluteDst.getParent().toUri().toString();
+ URI parentSrc = absoluteSrc.getParent().toUri();
+ URI parentDst = absoluteDst.getParent().toUri();
if (isParentOf(absoluteSrc, absoluteDst)) {
throw new IOException("Cannot rename " + absoluteSrc + " under itself"
+ " : "+ absoluteDst);
}
- if (!parentSrc.equals(parentDst)) {
+ if (!parentSrc.toString().equals(parentDst.toString())) {
throw new IOException("Cannot rename source: " + absoluteSrc
+ " to " + absoluteDst
+ " -"+ E_SAME_DIRECTORY_ONLY);
}
String from = absoluteSrc.getName();
String to = absoluteDst.getName();
- client.changeWorkingDirectory(parentSrc);
+ client.changeWorkingDirectory(parentSrc.getPath().toString());
Review Comment:
``.toString()`` isn't required here
> FTPFileSystem rename with full qualified path broken
> ----------------------------------------------------
>
> Key: HADOOP-19130
> URL: https://issues.apache.org/jira/browse/HADOOP-19130
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs
> Affects Versions: 0.20.2, 3.3.3, 3.3.4, 3.3.6
> Reporter: shawn
> Priority: Major
> Labels: pull-request-available
> Attachments: image-2024-03-27-09-59-12-381.png,
> image-2024-03-28-09-58-19-721.png
>
> Original Estimate: 2h
> Remaining Estimate: 2h
>
> When use fs shell to put/rename file in ftp server with full qualified
> path , it always get "Input/output error"(eg.
> [ftp://user:password@localhost/pathxxx]), the reason is that
> changeWorkingDirectory command underneath is being passed a string with
> [file://|file:///] uri prefix which will not be understand by ftp server
> !image-2024-03-27-09-59-12-381.png|width=948,height=156!
>
> in our case, after
> client.changeWorkingDirectory("ftp://mytest:[email protected]/files";)
> executed, the workingDirectory of ftp server is still "/", which is
> incorrect(not understand by ftp server)
> !image-2024-03-28-09-58-19-721.png|width=745,height=431!
> the solution should be pass
> absoluteSrc.getParent().toUri().getPath().toString to avoid
> [file://|file:///] uri prefix, like this:
> {code:java}
> --- a/FTPFileSystem.java
> +++ b/FTPFileSystem.java
> @@ -549,15 +549,15 @@ public class FTPFileSystem extends FileSystem {
> throw new IOException("Destination path " + dst
> + " already exist, cannot rename!");
> }
> - String parentSrc = absoluteSrc.getParent().toUri().toString();
> - String parentDst = absoluteDst.getParent().toUri().toString();
> + URI parentSrc = absoluteSrc.getParent().toUri();
> + URI parentDst = absoluteDst.getParent().toUri();
> String from = src.getName();
> String to = dst.getName();
> - if (!parentSrc.equals(parentDst)) {
> + if (!parentSrc.toString().equals(parentDst.toString())) {
> throw new IOException("Cannot rename parent(source): " + parentSrc
> + ", parent(destination): " + parentDst);
> }
> - client.changeWorkingDirectory(parentSrc);
> + client.changeWorkingDirectory(parentSrc.getPath().toString());
> boolean renamed = client.rename(from, to);
> return renamed;
> }{code}
> already related issue as follows
> https://issues.apache.org/jira/browse/HADOOP-8653
> I create this issue and add related unit test.
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
