You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2022/06/30 20:49:07 UTC

[GitHub] [nifi] greyp9 opened a new pull request, #6172: NIFI-10143 - ListFTP/GetFTP charset issues

greyp9 opened a new pull request, #6172:
URL: https://github.com/apache/nifi/pull/6172

   # Summary
   
   [NIFI-10143](https://issues.apache.org/jira/browse/NIFI-10143)
   - Additional unit test for FTP processors, using embedded mina/ftpserver.
   - Additional configuration for FTP client in the case of usage in non-UTF-8 environments.
   
   # Tracking
   
   Please complete the following tracking steps prior to pull request creation.
   
   ### Issue Tracking
   
   - [x] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
   
   ### Pull Request Tracking
   
   - [x] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-00000`
   - [x] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-00000`
   
   ### Pull Request Formatting
   
   - [x] Pull Request based on current revision of the `main` branch
   - [x] Pull Request refers to a feature branch with one commit containing changes
   
   # Verification
   
   Please indicate the verification steps performed prior to pull request creation.
   
   ### Build
   
   - [x] Build completed using `mvn clean install -P contrib-check`
     - [x] JDK 8
     - [x] JDK 11
     - [x] JDK 17
   
   ### Licensing
   
   - [x] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)
   - [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
   
   ### Documentation
   
   - [ ] Documentation formatting appears as expected in rendered files
   


-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6172: NIFI-10143 - ListFTP/GetFTP charset issues

Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6172:
URL: https://github.com/apache/nifi/pull/6172#discussion_r911999082


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestFTPCharset.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.standard;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.ftpserver.FtpServer;
+import org.apache.ftpserver.FtpServerFactory;
+import org.apache.ftpserver.ftplet.FileSystemFactory;
+import org.apache.ftpserver.ftplet.FileSystemView;
+import org.apache.ftpserver.ftplet.FtpException;
+import org.apache.ftpserver.ftplet.FtpFile;
+import org.apache.ftpserver.ftplet.UserManager;
+import org.apache.ftpserver.listener.ListenerFactory;
+import org.apache.ftpserver.usermanager.ClearTextPasswordEncryptor;
+import org.apache.ftpserver.usermanager.PropertiesUserManagerFactory;
+import org.apache.ftpserver.usermanager.impl.BaseUser;
+import org.apache.nifi.processor.Processor;
+import org.apache.nifi.processor.util.list.AbstractListProcessor;
+import org.apache.nifi.processors.standard.util.FTPTransfer;
+import org.apache.nifi.util.MockFlowFile;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
+
+/**
+ * Seed a UTF-7 folder in FTP server with files using i18n known filenames.  Iterate through a set of i18n folder
+ * names, copying the known content into each new target.  Afterwards, verify that the last folder contains all
+ * files with the expected filenames.
+ * <p>
+ * To test against a live FTP server:
+ * <ol>
+ *     <li>replace EMBED_FTP_SERVER value with <code>false</code></li>
+ *     <li>replace HOSTNAME, PORT, USER, PASSWORD as needed</li>
+ * </ol>
+ */
+@TestMethodOrder(MethodOrderer.MethodName.class)
+public class TestFTPCharset {
+    private static final boolean EMBED_FTP_SERVER = true;
+    private static FtpServer FTP_SERVER;
+
+    private static final String USE_UTF8 = Boolean.TRUE.toString();
+    private static final String HOSTNAME = "localhost";
+    private static final int PORT = 2121;
+    private static final String USER = "ftpuser";
+    private static final String PASSWORD = "admin";
+    private static final File FOLDER = new File("target", TestFTPCharset.class.getSimpleName());
+
+    public static Stream<Arguments> folderNamesProvider() {
+        return Stream.of(
+                arguments("folder1", "folder2"),
+                arguments("folder2", "æøå"),
+                arguments("æøå", "folder3"),
+                arguments("folder3", "اختبار"),
+                arguments("اختبار", "folder4"),
+                arguments("folder4", "Госагїzатїой"),
+                arguments("Госагїzатїой", "folder5"),
+                arguments("folder5", "し回亡丹し工z丹卞工回几"),
+                arguments("し回亡丹し工z丹卞工回几", "folder6")
+        );
+    }
+
+    public static Stream<String> filenamesProvider() {
+        return Stream.of(
+                "1.txt",
+                "æøå.txt",
+                "اختبار.txt",
+                "Госагїzатїой.txt",
+                "し回亡丹し工z丹卞工回几.txt");
+    }
+
+    /**
+     * Start test FTP server (if using embedded).
+     *
+     * @throws FtpException on server startup failure
+     */
+    @BeforeAll
+    static void beforeAll() throws FtpException {
+        if (EMBED_FTP_SERVER) {
+            FOLDER.mkdirs();
+            // setup ftp user
+            final PropertiesUserManagerFactory userManagerFactory = new PropertiesUserManagerFactory();
+            userManagerFactory.setUrl(TestFTPCharset.class.getClassLoader()
+                    .getResource("TestFTPCharset/users.properties"));
+            userManagerFactory.setPasswordEncryptor(new ClearTextPasswordEncryptor());
+            final UserManager userManager = userManagerFactory.createUserManager();
+            final BaseUser ftpuser = (BaseUser) userManager.getUserByName(USER);
+            ftpuser.setHomeDirectory(FOLDER.getAbsolutePath());
+            userManager.save(ftpuser);
+            // setup embedded ftp server
+            final FtpServerFactory serverFactory = new FtpServerFactory();
+            serverFactory.setUserManager(userManager);
+            final FileSystemFactory fileSystemFactory = serverFactory.getFileSystem();
+            final FileSystemView view = fileSystemFactory.createFileSystemView(ftpuser);
+            final FtpFile workingDirectory = view.getWorkingDirectory();
+            final Object physicalFile = workingDirectory.getPhysicalFile();
+            assertTrue(physicalFile instanceof File);

Review Comment:
   JUnit 5 supports `assertInstanceOf`



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestFTPCharset.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.standard;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.ftpserver.FtpServer;
+import org.apache.ftpserver.FtpServerFactory;
+import org.apache.ftpserver.ftplet.FileSystemFactory;
+import org.apache.ftpserver.ftplet.FileSystemView;
+import org.apache.ftpserver.ftplet.FtpException;
+import org.apache.ftpserver.ftplet.FtpFile;
+import org.apache.ftpserver.ftplet.UserManager;
+import org.apache.ftpserver.listener.ListenerFactory;
+import org.apache.ftpserver.usermanager.ClearTextPasswordEncryptor;
+import org.apache.ftpserver.usermanager.PropertiesUserManagerFactory;
+import org.apache.ftpserver.usermanager.impl.BaseUser;
+import org.apache.nifi.processor.Processor;
+import org.apache.nifi.processor.util.list.AbstractListProcessor;
+import org.apache.nifi.processors.standard.util.FTPTransfer;
+import org.apache.nifi.util.MockFlowFile;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
+
+/**
+ * Seed a UTF-7 folder in FTP server with files using i18n known filenames.  Iterate through a set of i18n folder
+ * names, copying the known content into each new target.  Afterwards, verify that the last folder contains all
+ * files with the expected filenames.
+ * <p>
+ * To test against a live FTP server:
+ * <ol>
+ *     <li>replace EMBED_FTP_SERVER value with <code>false</code></li>
+ *     <li>replace HOSTNAME, PORT, USER, PASSWORD as needed</li>
+ * </ol>
+ */
+@TestMethodOrder(MethodOrderer.MethodName.class)
+public class TestFTPCharset {
+    private static final boolean EMBED_FTP_SERVER = true;
+    private static FtpServer FTP_SERVER;
+
+    private static final String USE_UTF8 = Boolean.TRUE.toString();
+    private static final String HOSTNAME = "localhost";
+    private static final int PORT = 2121;
+    private static final String USER = "ftpuser";
+    private static final String PASSWORD = "admin";
+    private static final File FOLDER = new File("target", TestFTPCharset.class.getSimpleName());
+
+    public static Stream<Arguments> folderNamesProvider() {
+        return Stream.of(
+                arguments("folder1", "folder2"),
+                arguments("folder2", "æøå"),
+                arguments("æøå", "folder3"),
+                arguments("folder3", "اختبار"),
+                arguments("اختبار", "folder4"),
+                arguments("folder4", "Госагїzатїой"),
+                arguments("Госагїzатїой", "folder5"),
+                arguments("folder5", "し回亡丹し工z丹卞工回几"),
+                arguments("し回亡丹し工z丹卞工回几", "folder6")
+        );
+    }
+
+    public static Stream<String> filenamesProvider() {
+        return Stream.of(
+                "1.txt",
+                "æøå.txt",
+                "اختبار.txt",
+                "Госагїzатїой.txt",
+                "し回亡丹し工z丹卞工回几.txt");
+    }
+
+    /**
+     * Start test FTP server (if using embedded).
+     *
+     * @throws FtpException on server startup failure
+     */
+    @BeforeAll
+    static void beforeAll() throws FtpException {

Review Comment:
   It would be helpful to name this method something like `startServer`, which may obviate the need for the method comments.



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestFTPCharset.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.standard;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.ftpserver.FtpServer;
+import org.apache.ftpserver.FtpServerFactory;
+import org.apache.ftpserver.ftplet.FileSystemFactory;
+import org.apache.ftpserver.ftplet.FileSystemView;
+import org.apache.ftpserver.ftplet.FtpException;
+import org.apache.ftpserver.ftplet.FtpFile;
+import org.apache.ftpserver.ftplet.UserManager;
+import org.apache.ftpserver.listener.ListenerFactory;
+import org.apache.ftpserver.usermanager.ClearTextPasswordEncryptor;
+import org.apache.ftpserver.usermanager.PropertiesUserManagerFactory;
+import org.apache.ftpserver.usermanager.impl.BaseUser;
+import org.apache.nifi.processor.Processor;
+import org.apache.nifi.processor.util.list.AbstractListProcessor;
+import org.apache.nifi.processors.standard.util.FTPTransfer;
+import org.apache.nifi.util.MockFlowFile;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
+
+/**
+ * Seed a UTF-7 folder in FTP server with files using i18n known filenames.  Iterate through a set of i18n folder
+ * names, copying the known content into each new target.  Afterwards, verify that the last folder contains all
+ * files with the expected filenames.
+ * <p>
+ * To test against a live FTP server:
+ * <ol>
+ *     <li>replace EMBED_FTP_SERVER value with <code>false</code></li>
+ *     <li>replace HOSTNAME, PORT, USER, PASSWORD as needed</li>
+ * </ol>
+ */
+@TestMethodOrder(MethodOrderer.MethodName.class)
+public class TestFTPCharset {
+    private static final boolean EMBED_FTP_SERVER = true;
+    private static FtpServer FTP_SERVER;
+
+    private static final String USE_UTF8 = Boolean.TRUE.toString();
+    private static final String HOSTNAME = "localhost";
+    private static final int PORT = 2121;

Review Comment:
   It would be better to use a random port to avoid theoretical conflicts in different environments.



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestFTPCharset.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.standard;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.ftpserver.FtpServer;
+import org.apache.ftpserver.FtpServerFactory;
+import org.apache.ftpserver.ftplet.FileSystemFactory;
+import org.apache.ftpserver.ftplet.FileSystemView;
+import org.apache.ftpserver.ftplet.FtpException;
+import org.apache.ftpserver.ftplet.FtpFile;
+import org.apache.ftpserver.ftplet.UserManager;
+import org.apache.ftpserver.listener.ListenerFactory;
+import org.apache.ftpserver.usermanager.ClearTextPasswordEncryptor;
+import org.apache.ftpserver.usermanager.PropertiesUserManagerFactory;
+import org.apache.ftpserver.usermanager.impl.BaseUser;
+import org.apache.nifi.processor.Processor;
+import org.apache.nifi.processor.util.list.AbstractListProcessor;
+import org.apache.nifi.processors.standard.util.FTPTransfer;
+import org.apache.nifi.util.MockFlowFile;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
+
+/**
+ * Seed a UTF-7 folder in FTP server with files using i18n known filenames.  Iterate through a set of i18n folder
+ * names, copying the known content into each new target.  Afterwards, verify that the last folder contains all
+ * files with the expected filenames.
+ * <p>
+ * To test against a live FTP server:
+ * <ol>
+ *     <li>replace EMBED_FTP_SERVER value with <code>false</code></li>
+ *     <li>replace HOSTNAME, PORT, USER, PASSWORD as needed</li>
+ * </ol>

Review Comment:
   If the goal is to provide the option for testing against a real FTP server, this could be parameterized through JUnit 5 annotations.



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestFTPCharset.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.standard;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.ftpserver.FtpServer;
+import org.apache.ftpserver.FtpServerFactory;
+import org.apache.ftpserver.ftplet.FileSystemFactory;
+import org.apache.ftpserver.ftplet.FileSystemView;
+import org.apache.ftpserver.ftplet.FtpException;
+import org.apache.ftpserver.ftplet.FtpFile;
+import org.apache.ftpserver.ftplet.UserManager;
+import org.apache.ftpserver.listener.ListenerFactory;
+import org.apache.ftpserver.usermanager.ClearTextPasswordEncryptor;
+import org.apache.ftpserver.usermanager.PropertiesUserManagerFactory;
+import org.apache.ftpserver.usermanager.impl.BaseUser;
+import org.apache.nifi.processor.Processor;
+import org.apache.nifi.processor.util.list.AbstractListProcessor;
+import org.apache.nifi.processors.standard.util.FTPTransfer;
+import org.apache.nifi.util.MockFlowFile;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
+
+/**
+ * Seed a UTF-7 folder in FTP server with files using i18n known filenames.  Iterate through a set of i18n folder

Review Comment:
   ```suggestion
    * Seed a UTF-8 folder in FTP server with files using i18n known filenames.  Iterate through a set of i18n folder
   ```



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestFTPCharset.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.standard;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.ftpserver.FtpServer;
+import org.apache.ftpserver.FtpServerFactory;
+import org.apache.ftpserver.ftplet.FileSystemFactory;
+import org.apache.ftpserver.ftplet.FileSystemView;
+import org.apache.ftpserver.ftplet.FtpException;
+import org.apache.ftpserver.ftplet.FtpFile;
+import org.apache.ftpserver.ftplet.UserManager;
+import org.apache.ftpserver.listener.ListenerFactory;
+import org.apache.ftpserver.usermanager.ClearTextPasswordEncryptor;
+import org.apache.ftpserver.usermanager.PropertiesUserManagerFactory;
+import org.apache.ftpserver.usermanager.impl.BaseUser;
+import org.apache.nifi.processor.Processor;
+import org.apache.nifi.processor.util.list.AbstractListProcessor;
+import org.apache.nifi.processors.standard.util.FTPTransfer;
+import org.apache.nifi.util.MockFlowFile;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
+
+/**
+ * Seed a UTF-7 folder in FTP server with files using i18n known filenames.  Iterate through a set of i18n folder
+ * names, copying the known content into each new target.  Afterwards, verify that the last folder contains all
+ * files with the expected filenames.
+ * <p>
+ * To test against a live FTP server:
+ * <ol>
+ *     <li>replace EMBED_FTP_SERVER value with <code>false</code></li>
+ *     <li>replace HOSTNAME, PORT, USER, PASSWORD as needed</li>
+ * </ol>
+ */
+@TestMethodOrder(MethodOrderer.MethodName.class)
+public class TestFTPCharset {
+    private static final boolean EMBED_FTP_SERVER = true;
+    private static FtpServer FTP_SERVER;
+
+    private static final String USE_UTF8 = Boolean.TRUE.toString();
+    private static final String HOSTNAME = "localhost";
+    private static final int PORT = 2121;
+    private static final String USER = "ftpuser";
+    private static final String PASSWORD = "admin";
+    private static final File FOLDER = new File("target", TestFTPCharset.class.getSimpleName());
+
+    public static Stream<Arguments> folderNamesProvider() {
+        return Stream.of(
+                arguments("folder1", "folder2"),
+                arguments("folder2", "æøå"),
+                arguments("æøå", "folder3"),
+                arguments("folder3", "اختبار"),
+                arguments("اختبار", "folder4"),
+                arguments("folder4", "Госагїzатїой"),
+                arguments("Госагїzатїой", "folder5"),
+                arguments("folder5", "し回亡丹し工z丹卞工回几"),
+                arguments("し回亡丹し工z丹卞工回几", "folder6")
+        );
+    }
+
+    public static Stream<String> filenamesProvider() {
+        return Stream.of(
+                "1.txt",
+                "æøå.txt",
+                "اختبار.txt",
+                "Госагїzатїой.txt",
+                "し回亡丹し工z丹卞工回几.txt");
+    }
+
+    /**
+     * Start test FTP server (if using embedded).
+     *
+     * @throws FtpException on server startup failure
+     */
+    @BeforeAll
+    static void beforeAll() throws FtpException {
+        if (EMBED_FTP_SERVER) {
+            FOLDER.mkdirs();
+            // setup ftp user
+            final PropertiesUserManagerFactory userManagerFactory = new PropertiesUserManagerFactory();
+            userManagerFactory.setUrl(TestFTPCharset.class.getClassLoader()
+                    .getResource("TestFTPCharset/users.properties"));

Review Comment:
   Although it would take a little more setup work, creating the properties file in the test class, using the parameterized username and password, would help avoid potential mismatch between class properties and file properties.



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestFTPCharset.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.standard;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.ftpserver.FtpServer;
+import org.apache.ftpserver.FtpServerFactory;
+import org.apache.ftpserver.ftplet.FileSystemFactory;
+import org.apache.ftpserver.ftplet.FileSystemView;
+import org.apache.ftpserver.ftplet.FtpException;
+import org.apache.ftpserver.ftplet.FtpFile;
+import org.apache.ftpserver.ftplet.UserManager;
+import org.apache.ftpserver.listener.ListenerFactory;
+import org.apache.ftpserver.usermanager.ClearTextPasswordEncryptor;
+import org.apache.ftpserver.usermanager.PropertiesUserManagerFactory;
+import org.apache.ftpserver.usermanager.impl.BaseUser;
+import org.apache.nifi.processor.Processor;
+import org.apache.nifi.processor.util.list.AbstractListProcessor;
+import org.apache.nifi.processors.standard.util.FTPTransfer;
+import org.apache.nifi.util.MockFlowFile;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
+
+/**
+ * Seed a UTF-7 folder in FTP server with files using i18n known filenames.  Iterate through a set of i18n folder
+ * names, copying the known content into each new target.  Afterwards, verify that the last folder contains all
+ * files with the expected filenames.
+ * <p>
+ * To test against a live FTP server:
+ * <ol>
+ *     <li>replace EMBED_FTP_SERVER value with <code>false</code></li>
+ *     <li>replace HOSTNAME, PORT, USER, PASSWORD as needed</li>
+ * </ol>
+ */
+@TestMethodOrder(MethodOrderer.MethodName.class)
+public class TestFTPCharset {
+    private static final boolean EMBED_FTP_SERVER = true;
+    private static FtpServer FTP_SERVER;
+
+    private static final String USE_UTF8 = Boolean.TRUE.toString();
+    private static final String HOSTNAME = "localhost";
+    private static final int PORT = 2121;
+    private static final String USER = "ftpuser";
+    private static final String PASSWORD = "admin";
+    private static final File FOLDER = new File("target", TestFTPCharset.class.getSimpleName());

Review Comment:
   Instead of using a static directory in the `target` directory, it would better to use the JUnit 5 `TempDir` annotation to manage the lifecycle of the temporary directory.



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ftp/StandardFTPClientProvider.java:
##########
@@ -152,10 +153,10 @@ private void setClientProperties(final FTPClient client, final PropertyContext c
         client.setRemoteVerificationEnabled(false);
 
         final boolean unicodeEnabled = context.getProperty(UTF8_ENCODING).isSet() ? context.getProperty(UTF8_ENCODING).asBoolean() : false;
-        if (unicodeEnabled) {
-            client.setControlEncoding(StandardCharsets.UTF_8.name());
-            client.setAutodetectUTF8(true);
-        }
+        final Charset charset = unicodeEnabled ? StandardCharsets.UTF_8 : Charset.defaultCharset();
+        client.setCharset(charset);

Review Comment:
   Does this change the current behavior, or does the internal character set of the client use `Charset.defaultCharset()` when not otherwise specified? This looks like the right way to go, just wanted to clarify.



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestFTPCharset.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.standard;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.ftpserver.FtpServer;
+import org.apache.ftpserver.FtpServerFactory;
+import org.apache.ftpserver.ftplet.FileSystemFactory;
+import org.apache.ftpserver.ftplet.FileSystemView;
+import org.apache.ftpserver.ftplet.FtpException;
+import org.apache.ftpserver.ftplet.FtpFile;
+import org.apache.ftpserver.ftplet.UserManager;
+import org.apache.ftpserver.listener.ListenerFactory;
+import org.apache.ftpserver.usermanager.ClearTextPasswordEncryptor;
+import org.apache.ftpserver.usermanager.PropertiesUserManagerFactory;
+import org.apache.ftpserver.usermanager.impl.BaseUser;
+import org.apache.nifi.processor.Processor;
+import org.apache.nifi.processor.util.list.AbstractListProcessor;
+import org.apache.nifi.processors.standard.util.FTPTransfer;
+import org.apache.nifi.util.MockFlowFile;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
+
+/**
+ * Seed a UTF-7 folder in FTP server with files using i18n known filenames.  Iterate through a set of i18n folder
+ * names, copying the known content into each new target.  Afterwards, verify that the last folder contains all
+ * files with the expected filenames.
+ * <p>
+ * To test against a live FTP server:
+ * <ol>
+ *     <li>replace EMBED_FTP_SERVER value with <code>false</code></li>
+ *     <li>replace HOSTNAME, PORT, USER, PASSWORD as needed</li>
+ * </ol>
+ */
+@TestMethodOrder(MethodOrderer.MethodName.class)
+public class TestFTPCharset {
+    private static final boolean EMBED_FTP_SERVER = true;
+    private static FtpServer FTP_SERVER;
+
+    private static final String USE_UTF8 = Boolean.TRUE.toString();
+    private static final String HOSTNAME = "localhost";
+    private static final int PORT = 2121;
+    private static final String USER = "ftpuser";
+    private static final String PASSWORD = "admin";
+    private static final File FOLDER = new File("target", TestFTPCharset.class.getSimpleName());
+
+    public static Stream<Arguments> folderNamesProvider() {
+        return Stream.of(
+                arguments("folder1", "folder2"),
+                arguments("folder2", "æøå"),
+                arguments("æøå", "folder3"),
+                arguments("folder3", "اختبار"),
+                arguments("اختبار", "folder4"),
+                arguments("folder4", "Госагїzатїой"),
+                arguments("Госагїzатїой", "folder5"),
+                arguments("folder5", "し回亡丹し工z丹卞工回几"),
+                arguments("し回亡丹し工z丹卞工回几", "folder6")
+        );
+    }
+
+    public static Stream<String> filenamesProvider() {
+        return Stream.of(
+                "1.txt",
+                "æøå.txt",
+                "اختبار.txt",
+                "Госагїzатїой.txt",
+                "し回亡丹し工z丹卞工回几.txt");
+    }
+
+    /**
+     * Start test FTP server (if using embedded).
+     *
+     * @throws FtpException on server startup failure
+     */
+    @BeforeAll
+    static void beforeAll() throws FtpException {
+        if (EMBED_FTP_SERVER) {
+            FOLDER.mkdirs();
+            // setup ftp user
+            final PropertiesUserManagerFactory userManagerFactory = new PropertiesUserManagerFactory();
+            userManagerFactory.setUrl(TestFTPCharset.class.getClassLoader()
+                    .getResource("TestFTPCharset/users.properties"));
+            userManagerFactory.setPasswordEncryptor(new ClearTextPasswordEncryptor());
+            final UserManager userManager = userManagerFactory.createUserManager();
+            final BaseUser ftpuser = (BaseUser) userManager.getUserByName(USER);
+            ftpuser.setHomeDirectory(FOLDER.getAbsolutePath());
+            userManager.save(ftpuser);
+            // setup embedded ftp server
+            final FtpServerFactory serverFactory = new FtpServerFactory();
+            serverFactory.setUserManager(userManager);
+            final FileSystemFactory fileSystemFactory = serverFactory.getFileSystem();
+            final FileSystemView view = fileSystemFactory.createFileSystemView(ftpuser);
+            final FtpFile workingDirectory = view.getWorkingDirectory();
+            final Object physicalFile = workingDirectory.getPhysicalFile();
+            assertTrue(physicalFile instanceof File);
+            assertEquals(FOLDER.getAbsolutePath(), ((File) physicalFile).getAbsolutePath());
+            final ListenerFactory factory = new ListenerFactory();
+            factory.setPort(PORT);
+            serverFactory.addListener("default", factory.createListener());
+            FTP_SERVER = serverFactory.createServer();
+            FTP_SERVER.start();
+        }
+    }
+
+    /**
+     * Stop test FTP server (if using embedded).
+     *
+     * @throws IOException on failure to clean up test data
+     */
+    @AfterAll
+    static void afterAll() throws IOException {
+        if (EMBED_FTP_SERVER) {
+            FTP_SERVER.stop();
+            while (!FTP_SERVER.isStopped()) {
+                try {
+                    Thread.sleep(100L);
+                } catch (InterruptedException e) {
+                    throw new RuntimeException(e);
+                }
+            }
+            FileUtils.deleteDirectory(FOLDER);
+        }
+    }
+
+
+    /**
+     * Test connectivity to FTP server.
+     */
+    @Test
+    public void test0SeedFTPPut1() {

Review Comment:
   Is this approach to naming and method ordering required? It seems potentially brittle. If that is the case, this might be better as an integration test, or a more focused unit test.



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestFTPCharset.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.standard;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.ftpserver.FtpServer;
+import org.apache.ftpserver.FtpServerFactory;
+import org.apache.ftpserver.ftplet.FileSystemFactory;
+import org.apache.ftpserver.ftplet.FileSystemView;
+import org.apache.ftpserver.ftplet.FtpException;
+import org.apache.ftpserver.ftplet.FtpFile;
+import org.apache.ftpserver.ftplet.UserManager;
+import org.apache.ftpserver.listener.ListenerFactory;
+import org.apache.ftpserver.usermanager.ClearTextPasswordEncryptor;
+import org.apache.ftpserver.usermanager.PropertiesUserManagerFactory;
+import org.apache.ftpserver.usermanager.impl.BaseUser;
+import org.apache.nifi.processor.Processor;
+import org.apache.nifi.processor.util.list.AbstractListProcessor;
+import org.apache.nifi.processors.standard.util.FTPTransfer;
+import org.apache.nifi.util.MockFlowFile;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
+
+/**
+ * Seed a UTF-7 folder in FTP server with files using i18n known filenames.  Iterate through a set of i18n folder
+ * names, copying the known content into each new target.  Afterwards, verify that the last folder contains all
+ * files with the expected filenames.
+ * <p>
+ * To test against a live FTP server:
+ * <ol>
+ *     <li>replace EMBED_FTP_SERVER value with <code>false</code></li>
+ *     <li>replace HOSTNAME, PORT, USER, PASSWORD as needed</li>
+ * </ol>
+ */
+@TestMethodOrder(MethodOrderer.MethodName.class)
+public class TestFTPCharset {
+    private static final boolean EMBED_FTP_SERVER = true;
+    private static FtpServer FTP_SERVER;
+
+    private static final String USE_UTF8 = Boolean.TRUE.toString();
+    private static final String HOSTNAME = "localhost";
+    private static final int PORT = 2121;
+    private static final String USER = "ftpuser";
+    private static final String PASSWORD = "admin";
+    private static final File FOLDER = new File("target", TestFTPCharset.class.getSimpleName());
+
+    public static Stream<Arguments> folderNamesProvider() {
+        return Stream.of(
+                arguments("folder1", "folder2"),
+                arguments("folder2", "æøå"),
+                arguments("æøå", "folder3"),
+                arguments("folder3", "اختبار"),
+                arguments("اختبار", "folder4"),
+                arguments("folder4", "Госагїzатїой"),
+                arguments("Госагїzатїой", "folder5"),
+                arguments("folder5", "し回亡丹し工z丹卞工回几"),
+                arguments("し回亡丹し工z丹卞工回几", "folder6")
+        );
+    }
+
+    public static Stream<String> filenamesProvider() {
+        return Stream.of(
+                "1.txt",
+                "æøå.txt",
+                "اختبار.txt",
+                "Госагїzатїой.txt",
+                "し回亡丹し工z丹卞工回几.txt");
+    }
+
+    /**
+     * Start test FTP server (if using embedded).
+     *
+     * @throws FtpException on server startup failure
+     */
+    @BeforeAll
+    static void beforeAll() throws FtpException {
+        if (EMBED_FTP_SERVER) {
+            FOLDER.mkdirs();
+            // setup ftp user
+            final PropertiesUserManagerFactory userManagerFactory = new PropertiesUserManagerFactory();
+            userManagerFactory.setUrl(TestFTPCharset.class.getClassLoader()
+                    .getResource("TestFTPCharset/users.properties"));
+            userManagerFactory.setPasswordEncryptor(new ClearTextPasswordEncryptor());
+            final UserManager userManager = userManagerFactory.createUserManager();
+            final BaseUser ftpuser = (BaseUser) userManager.getUserByName(USER);
+            ftpuser.setHomeDirectory(FOLDER.getAbsolutePath());
+            userManager.save(ftpuser);
+            // setup embedded ftp server
+            final FtpServerFactory serverFactory = new FtpServerFactory();
+            serverFactory.setUserManager(userManager);
+            final FileSystemFactory fileSystemFactory = serverFactory.getFileSystem();
+            final FileSystemView view = fileSystemFactory.createFileSystemView(ftpuser);
+            final FtpFile workingDirectory = view.getWorkingDirectory();
+            final Object physicalFile = workingDirectory.getPhysicalFile();
+            assertTrue(physicalFile instanceof File);
+            assertEquals(FOLDER.getAbsolutePath(), ((File) physicalFile).getAbsolutePath());
+            final ListenerFactory factory = new ListenerFactory();
+            factory.setPort(PORT);
+            serverFactory.addListener("default", factory.createListener());
+            FTP_SERVER = serverFactory.createServer();
+            FTP_SERVER.start();
+        }
+    }
+
+    /**
+     * Stop test FTP server (if using embedded).
+     *
+     * @throws IOException on failure to clean up test data
+     */
+    @AfterAll
+    static void afterAll() throws IOException {
+        if (EMBED_FTP_SERVER) {
+            FTP_SERVER.stop();
+            while (!FTP_SERVER.isStopped()) {
+                try {
+                    Thread.sleep(100L);
+                } catch (InterruptedException e) {
+                    throw new RuntimeException(e);
+                }

Review Comment:
   The `InterruptedException` could be declared on the method signature instead of caught and thrown again.



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestFTPCharset.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.standard;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.ftpserver.FtpServer;
+import org.apache.ftpserver.FtpServerFactory;
+import org.apache.ftpserver.ftplet.FileSystemFactory;
+import org.apache.ftpserver.ftplet.FileSystemView;
+import org.apache.ftpserver.ftplet.FtpException;
+import org.apache.ftpserver.ftplet.FtpFile;
+import org.apache.ftpserver.ftplet.UserManager;
+import org.apache.ftpserver.listener.ListenerFactory;
+import org.apache.ftpserver.usermanager.ClearTextPasswordEncryptor;
+import org.apache.ftpserver.usermanager.PropertiesUserManagerFactory;
+import org.apache.ftpserver.usermanager.impl.BaseUser;
+import org.apache.nifi.processor.Processor;
+import org.apache.nifi.processor.util.list.AbstractListProcessor;
+import org.apache.nifi.processors.standard.util.FTPTransfer;
+import org.apache.nifi.util.MockFlowFile;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
+
+/**
+ * Seed a UTF-7 folder in FTP server with files using i18n known filenames.  Iterate through a set of i18n folder
+ * names, copying the known content into each new target.  Afterwards, verify that the last folder contains all
+ * files with the expected filenames.
+ * <p>
+ * To test against a live FTP server:
+ * <ol>
+ *     <li>replace EMBED_FTP_SERVER value with <code>false</code></li>
+ *     <li>replace HOSTNAME, PORT, USER, PASSWORD as needed</li>
+ * </ol>
+ */
+@TestMethodOrder(MethodOrderer.MethodName.class)
+public class TestFTPCharset {
+    private static final boolean EMBED_FTP_SERVER = true;
+    private static FtpServer FTP_SERVER;
+
+    private static final String USE_UTF8 = Boolean.TRUE.toString();
+    private static final String HOSTNAME = "localhost";
+    private static final int PORT = 2121;
+    private static final String USER = "ftpuser";
+    private static final String PASSWORD = "admin";
+    private static final File FOLDER = new File("target", TestFTPCharset.class.getSimpleName());
+
+    public static Stream<Arguments> folderNamesProvider() {
+        return Stream.of(
+                arguments("folder1", "folder2"),
+                arguments("folder2", "æøå"),
+                arguments("æøå", "folder3"),
+                arguments("folder3", "اختبار"),
+                arguments("اختبار", "folder4"),
+                arguments("folder4", "Госагїzатїой"),
+                arguments("Госагїzатїой", "folder5"),
+                arguments("folder5", "し回亡丹し工z丹卞工回几"),
+                arguments("し回亡丹し工z丹卞工回几", "folder6")
+        );
+    }
+
+    public static Stream<String> filenamesProvider() {
+        return Stream.of(
+                "1.txt",
+                "æøå.txt",
+                "اختبار.txt",
+                "Госагїzатїой.txt",
+                "し回亡丹し工z丹卞工回几.txt");
+    }
+
+    /**
+     * Start test FTP server (if using embedded).
+     *
+     * @throws FtpException on server startup failure
+     */
+    @BeforeAll
+    static void beforeAll() throws FtpException {
+        if (EMBED_FTP_SERVER) {
+            FOLDER.mkdirs();
+            // setup ftp user
+            final PropertiesUserManagerFactory userManagerFactory = new PropertiesUserManagerFactory();
+            userManagerFactory.setUrl(TestFTPCharset.class.getClassLoader()
+                    .getResource("TestFTPCharset/users.properties"));
+            userManagerFactory.setPasswordEncryptor(new ClearTextPasswordEncryptor());
+            final UserManager userManager = userManagerFactory.createUserManager();
+            final BaseUser ftpuser = (BaseUser) userManager.getUserByName(USER);
+            ftpuser.setHomeDirectory(FOLDER.getAbsolutePath());
+            userManager.save(ftpuser);
+            // setup embedded ftp server
+            final FtpServerFactory serverFactory = new FtpServerFactory();
+            serverFactory.setUserManager(userManager);
+            final FileSystemFactory fileSystemFactory = serverFactory.getFileSystem();
+            final FileSystemView view = fileSystemFactory.createFileSystemView(ftpuser);
+            final FtpFile workingDirectory = view.getWorkingDirectory();
+            final Object physicalFile = workingDirectory.getPhysicalFile();
+            assertTrue(physicalFile instanceof File);
+            assertEquals(FOLDER.getAbsolutePath(), ((File) physicalFile).getAbsolutePath());
+            final ListenerFactory factory = new ListenerFactory();
+            factory.setPort(PORT);
+            serverFactory.addListener("default", factory.createListener());
+            FTP_SERVER = serverFactory.createServer();
+            FTP_SERVER.start();
+        }
+    }
+
+    /**
+     * Stop test FTP server (if using embedded).
+     *
+     * @throws IOException on failure to clean up test data
+     */
+    @AfterAll
+    static void afterAll() throws IOException {
+        if (EMBED_FTP_SERVER) {
+            FTP_SERVER.stop();
+            while (!FTP_SERVER.isStopped()) {
+                try {
+                    Thread.sleep(100L);
+                } catch (InterruptedException e) {
+                    throw new RuntimeException(e);
+                }
+            }
+            FileUtils.deleteDirectory(FOLDER);
+        }
+    }
+
+
+    /**
+     * Test connectivity to FTP server.
+     */
+    @Test
+    public void test0SeedFTPPut1() {
+        final TestRunner runnerPut = provisionTestRunner(PutFTP.class);
+        final String folderName = "folder0";
+        final MockFlowFile flowFile = new MockFlowFile(1L);
+        flowFile.putAttributes(Collections.singletonMap("filename", "0.txt"));
+        flowFile.setData(new Date().toString().getBytes(UTF_8));
+        runnerPut.enqueue(flowFile);
+        runnerPut.setValidateExpressionUsage(false);
+        runnerPut.setProperty(FTPTransfer.REMOTE_PATH, folderName);
+        runnerPut.setProperty(FTPTransfer.CREATE_DIRECTORY, Boolean.TRUE.toString());
+        runnerPut.setProperty(FTPTransfer.DOT_RENAME, Boolean.FALSE.toString());
+        runnerPut.run();
+        runnerPut.assertTransferCount(PutFileTransfer.REL_FAILURE, 0);
+        runnerPut.assertTransferCount(PutFileTransfer.REL_REJECT, 0);
+        runnerPut.assertTransferCount(PutFileTransfer.REL_SUCCESS, 1);
+    }
+
+    /**
+     * Seed FTP server with data to be propagated.
+     */
+    @Test
+    public void test0SeedFTPPutAll() {
+        int id = 0;
+        final Object[] argumentsFirstFolder = folderNamesProvider().iterator().next().get();
+        final String folderName = Arrays.stream(argumentsFirstFolder).iterator().next().toString();
+        final TestRunner runnerPut = provisionTestRunner(PutFTP.class);
+        runnerPut.setValidateExpressionUsage(false);
+        runnerPut.setProperty(FTPTransfer.REMOTE_PATH, folderName);
+        runnerPut.setProperty(FTPTransfer.CREATE_DIRECTORY, Boolean.TRUE.toString());
+        runnerPut.setProperty(FTPTransfer.DOT_RENAME, Boolean.FALSE.toString());
+
+        final Iterator<String> iteratorFilenames = filenamesProvider().iterator();
+        while (iteratorFilenames.hasNext()) {
+            final String filename = iteratorFilenames.next();
+            final MockFlowFile flowFile = new MockFlowFile(++id);
+            flowFile.putAttributes(Collections.singletonMap("filename", filename));
+            flowFile.setData(new Date().toString().getBytes(UTF_8));
+            runnerPut.enqueue(flowFile);
+        }
+        final int fileCount = (int) filenamesProvider().count();
+        runnerPut.run();
+        runnerPut.assertTransferCount(PutFileTransfer.REL_FAILURE, 0);
+        runnerPut.assertTransferCount(PutFileTransfer.REL_REJECT, 0);
+        runnerPut.assertTransferCount(PutFileTransfer.REL_SUCCESS, fileCount);
+    }
+
+    /**
+     * Verify that data has been successfully propagated.
+     */
+    @Test
+    public void test9FTPVerifyAll() {
+        final Set<String> filenamesExpected = filenamesProvider().collect(Collectors.toSet());
+
+        final Object[] argumentsLastFolder = folderNamesProvider()
+                .reduce((prev, next) -> next).orElseThrow(IllegalStateException::new).get();
+        final String folderName = Arrays.stream(argumentsLastFolder)
+                .reduce((prev, next) -> next).orElseThrow(IllegalStateException::new).toString();
+        final TestRunner runnerList = provisionTestRunner(ListFTP.class);
+        runnerList.setProperty(ListFileTransfer.FILE_TRANSFER_LISTING_STRATEGY, AbstractListProcessor.NO_TRACKING);
+        runnerList.setProperty(FTPTransfer.REMOTE_PATH, folderName);
+        runnerList.clearTransferState();
+        runnerList.run(1);
+        runnerList.assertTransferCount("success", filenamesExpected.size());
+        final List<MockFlowFile> flowFilesList = runnerList.getFlowFilesForRelationship("success");
+        for (MockFlowFile flowFile : flowFilesList) {
+            filenamesExpected.remove(flowFile.getAttribute("filename"));
+        }
+        assertTrue(filenamesExpected.isEmpty());
+    }
+
+    /**
+     * For each parameterized invocation, copy files using FTP protocol from source folder to target folder.  This
+     * implicitly verifies charset handling of ListFTP, FetchFTP, and PutFTP processors.
+     *
+     * @param source the folder name from which to retrieve data
+     * @param target the folder name to which data should be copied
+     */
+    @ParameterizedTest
+    @MethodSource("folderNamesProvider")
+    public void test1FTP(final String source, final String target) {
+        final int fileCount = (int) filenamesProvider().count();
+        // ListFTP
+        final TestRunner runnerList = provisionTestRunner(ListFTP.class);
+        runnerList.setProperty(ListFileTransfer.FILE_TRANSFER_LISTING_STRATEGY, AbstractListProcessor.NO_TRACKING);
+        runnerList.setProperty(FTPTransfer.REMOTE_PATH, source);
+        runnerList.clearTransferState();
+        runnerList.run(1);
+        runnerList.assertTransferCount("success", fileCount);

Review Comment:
   References to relationships should use the Relationship member variable from the Processor.



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestFTPCharset.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.standard;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.ftpserver.FtpServer;
+import org.apache.ftpserver.FtpServerFactory;
+import org.apache.ftpserver.ftplet.FileSystemFactory;
+import org.apache.ftpserver.ftplet.FileSystemView;
+import org.apache.ftpserver.ftplet.FtpException;
+import org.apache.ftpserver.ftplet.FtpFile;
+import org.apache.ftpserver.ftplet.UserManager;
+import org.apache.ftpserver.listener.ListenerFactory;
+import org.apache.ftpserver.usermanager.ClearTextPasswordEncryptor;
+import org.apache.ftpserver.usermanager.PropertiesUserManagerFactory;
+import org.apache.ftpserver.usermanager.impl.BaseUser;
+import org.apache.nifi.processor.Processor;
+import org.apache.nifi.processor.util.list.AbstractListProcessor;
+import org.apache.nifi.processors.standard.util.FTPTransfer;
+import org.apache.nifi.util.MockFlowFile;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
+
+/**
+ * Seed a UTF-7 folder in FTP server with files using i18n known filenames.  Iterate through a set of i18n folder
+ * names, copying the known content into each new target.  Afterwards, verify that the last folder contains all
+ * files with the expected filenames.
+ * <p>
+ * To test against a live FTP server:
+ * <ol>
+ *     <li>replace EMBED_FTP_SERVER value with <code>false</code></li>
+ *     <li>replace HOSTNAME, PORT, USER, PASSWORD as needed</li>
+ * </ol>
+ */
+@TestMethodOrder(MethodOrderer.MethodName.class)
+public class TestFTPCharset {
+    private static final boolean EMBED_FTP_SERVER = true;
+    private static FtpServer FTP_SERVER;
+
+    private static final String USE_UTF8 = Boolean.TRUE.toString();
+    private static final String HOSTNAME = "localhost";
+    private static final int PORT = 2121;
+    private static final String USER = "ftpuser";
+    private static final String PASSWORD = "admin";
+    private static final File FOLDER = new File("target", TestFTPCharset.class.getSimpleName());
+
+    public static Stream<Arguments> folderNamesProvider() {
+        return Stream.of(
+                arguments("folder1", "folder2"),
+                arguments("folder2", "æøå"),
+                arguments("æøå", "folder3"),
+                arguments("folder3", "اختبار"),
+                arguments("اختبار", "folder4"),
+                arguments("folder4", "Госагїzатїой"),
+                arguments("Госагїzатїой", "folder5"),
+                arguments("folder5", "し回亡丹し工z丹卞工回几"),
+                arguments("し回亡丹し工z丹卞工回几", "folder6")
+        );
+    }
+
+    public static Stream<String> filenamesProvider() {
+        return Stream.of(
+                "1.txt",
+                "æøå.txt",
+                "اختبار.txt",
+                "Госагїzатїой.txt",
+                "し回亡丹し工z丹卞工回几.txt");
+    }
+
+    /**
+     * Start test FTP server (if using embedded).
+     *
+     * @throws FtpException on server startup failure
+     */
+    @BeforeAll
+    static void beforeAll() throws FtpException {
+        if (EMBED_FTP_SERVER) {
+            FOLDER.mkdirs();
+            // setup ftp user
+            final PropertiesUserManagerFactory userManagerFactory = new PropertiesUserManagerFactory();
+            userManagerFactory.setUrl(TestFTPCharset.class.getClassLoader()
+                    .getResource("TestFTPCharset/users.properties"));
+            userManagerFactory.setPasswordEncryptor(new ClearTextPasswordEncryptor());
+            final UserManager userManager = userManagerFactory.createUserManager();
+            final BaseUser ftpuser = (BaseUser) userManager.getUserByName(USER);
+            ftpuser.setHomeDirectory(FOLDER.getAbsolutePath());
+            userManager.save(ftpuser);
+            // setup embedded ftp server
+            final FtpServerFactory serverFactory = new FtpServerFactory();
+            serverFactory.setUserManager(userManager);
+            final FileSystemFactory fileSystemFactory = serverFactory.getFileSystem();
+            final FileSystemView view = fileSystemFactory.createFileSystemView(ftpuser);
+            final FtpFile workingDirectory = view.getWorkingDirectory();
+            final Object physicalFile = workingDirectory.getPhysicalFile();
+            assertTrue(physicalFile instanceof File);
+            assertEquals(FOLDER.getAbsolutePath(), ((File) physicalFile).getAbsolutePath());
+            final ListenerFactory factory = new ListenerFactory();
+            factory.setPort(PORT);
+            serverFactory.addListener("default", factory.createListener());
+            FTP_SERVER = serverFactory.createServer();
+            FTP_SERVER.start();
+        }
+    }
+
+    /**
+     * Stop test FTP server (if using embedded).
+     *
+     * @throws IOException on failure to clean up test data
+     */
+    @AfterAll
+    static void afterAll() throws IOException {
+        if (EMBED_FTP_SERVER) {
+            FTP_SERVER.stop();
+            while (!FTP_SERVER.isStopped()) {
+                try {
+                    Thread.sleep(100L);
+                } catch (InterruptedException e) {
+                    throw new RuntimeException(e);
+                }
+            }
+            FileUtils.deleteDirectory(FOLDER);

Review Comment:
   Refactoring to use JUnit 5 TempDir should remove the need for deleting the directory manually, although it might be necessary to clear the directory, depending on the approach.



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] greyp9 commented on a diff in pull request #6172: NIFI-10143 - ListFTP/GetFTP charset issues

Posted by GitBox <gi...@apache.org>.
greyp9 commented on code in PR #6172:
URL: https://github.com/apache/nifi/pull/6172#discussion_r912263177


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ftp/StandardFTPClientProvider.java:
##########
@@ -152,10 +153,10 @@ private void setClientProperties(final FTPClient client, final PropertyContext c
         client.setRemoteVerificationEnabled(false);
 
         final boolean unicodeEnabled = context.getProperty(UTF8_ENCODING).isSet() ? context.getProperty(UTF8_ENCODING).asBoolean() : false;
-        if (unicodeEnabled) {
-            client.setControlEncoding(StandardCharsets.UTF_8.name());
-            client.setAutodetectUTF8(true);
-        }
+        final Charset charset = unicodeEnabled ? StandardCharsets.UTF_8 : Charset.defaultCharset();
+        client.setCharset(charset);

Review Comment:
   It was a speculative change.  I have no evidence that it makes a difference.  Removed this.



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] greyp9 commented on a diff in pull request #6172: NIFI-10143 - ListFTP/GetFTP charset issues

Posted by GitBox <gi...@apache.org>.
greyp9 commented on code in PR #6172:
URL: https://github.com/apache/nifi/pull/6172#discussion_r918266864


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ftp/StandardFTPClientProvider.java:
##########
@@ -150,12 +151,11 @@ private void setClientProperties(final FTPClient client, final PropertyContext c
         client.setDataTimeout(dataTimeout);
         client.setDefaultTimeout(connectionTimeout);
         client.setRemoteVerificationEnabled(false);
+        client.setAutodetectUTF8(true);
 
         final boolean unicodeEnabled = context.getProperty(UTF8_ENCODING).isSet() ? context.getProperty(UTF8_ENCODING).asBoolean() : false;
-        if (unicodeEnabled) {
-            client.setControlEncoding(StandardCharsets.UTF_8.name());
-            client.setAutodetectUTF8(true);
-        }
+        final Charset charset = unicodeEnabled ? StandardCharsets.UTF_8 : Charset.defaultCharset();

Review Comment:
   Added a comment to clarify expectation about default 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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] exceptionfactory closed pull request #6172: NIFI-10143 - ListFTP/GetFTP charset issues

Posted by GitBox <gi...@apache.org>.
exceptionfactory closed pull request #6172: NIFI-10143 - ListFTP/GetFTP charset issues
URL: https://github.com/apache/nifi/pull/6172


-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] greyp9 commented on a diff in pull request #6172: NIFI-10143 - ListFTP/GetFTP charset issues

Posted by GitBox <gi...@apache.org>.
greyp9 commented on code in PR #6172:
URL: https://github.com/apache/nifi/pull/6172#discussion_r912275991


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestFTPCharset.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.standard;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.ftpserver.FtpServer;
+import org.apache.ftpserver.FtpServerFactory;
+import org.apache.ftpserver.ftplet.FileSystemFactory;
+import org.apache.ftpserver.ftplet.FileSystemView;
+import org.apache.ftpserver.ftplet.FtpException;
+import org.apache.ftpserver.ftplet.FtpFile;
+import org.apache.ftpserver.ftplet.UserManager;
+import org.apache.ftpserver.listener.ListenerFactory;
+import org.apache.ftpserver.usermanager.ClearTextPasswordEncryptor;
+import org.apache.ftpserver.usermanager.PropertiesUserManagerFactory;
+import org.apache.ftpserver.usermanager.impl.BaseUser;
+import org.apache.nifi.processor.Processor;
+import org.apache.nifi.processor.util.list.AbstractListProcessor;
+import org.apache.nifi.processors.standard.util.FTPTransfer;
+import org.apache.nifi.util.MockFlowFile;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
+
+/**
+ * Seed a UTF-7 folder in FTP server with files using i18n known filenames.  Iterate through a set of i18n folder
+ * names, copying the known content into each new target.  Afterwards, verify that the last folder contains all
+ * files with the expected filenames.
+ * <p>
+ * To test against a live FTP server:
+ * <ol>
+ *     <li>replace EMBED_FTP_SERVER value with <code>false</code></li>
+ *     <li>replace HOSTNAME, PORT, USER, PASSWORD as needed</li>
+ * </ol>
+ */
+@TestMethodOrder(MethodOrderer.MethodName.class)
+public class TestFTPCharset {
+    private static final boolean EMBED_FTP_SERVER = true;
+    private static FtpServer FTP_SERVER;
+
+    private static final String USE_UTF8 = Boolean.TRUE.toString();
+    private static final String HOSTNAME = "localhost";
+    private static final int PORT = 2121;
+    private static final String USER = "ftpuser";
+    private static final String PASSWORD = "admin";
+    private static final File FOLDER = new File("target", TestFTPCharset.class.getSimpleName());
+
+    public static Stream<Arguments> folderNamesProvider() {
+        return Stream.of(
+                arguments("folder1", "folder2"),
+                arguments("folder2", "æøå"),
+                arguments("æøå", "folder3"),
+                arguments("folder3", "اختبار"),
+                arguments("اختبار", "folder4"),
+                arguments("folder4", "Госагїzатїой"),
+                arguments("Госагїzатїой", "folder5"),
+                arguments("folder5", "し回亡丹し工z丹卞工回几"),
+                arguments("し回亡丹し工z丹卞工回几", "folder6")
+        );
+    }
+
+    public static Stream<String> filenamesProvider() {
+        return Stream.of(
+                "1.txt",
+                "æøå.txt",
+                "اختبار.txt",
+                "Госагїzатїой.txt",
+                "し回亡丹し工z丹卞工回几.txt");
+    }
+
+    /**
+     * Start test FTP server (if using embedded).
+     *
+     * @throws FtpException on server startup failure
+     */
+    @BeforeAll
+    static void beforeAll() throws FtpException {
+        if (EMBED_FTP_SERVER) {
+            FOLDER.mkdirs();
+            // setup ftp user
+            final PropertiesUserManagerFactory userManagerFactory = new PropertiesUserManagerFactory();
+            userManagerFactory.setUrl(TestFTPCharset.class.getClassLoader()
+                    .getResource("TestFTPCharset/users.properties"));
+            userManagerFactory.setPasswordEncryptor(new ClearTextPasswordEncryptor());
+            final UserManager userManager = userManagerFactory.createUserManager();
+            final BaseUser ftpuser = (BaseUser) userManager.getUserByName(USER);
+            ftpuser.setHomeDirectory(FOLDER.getAbsolutePath());
+            userManager.save(ftpuser);
+            // setup embedded ftp server
+            final FtpServerFactory serverFactory = new FtpServerFactory();
+            serverFactory.setUserManager(userManager);
+            final FileSystemFactory fileSystemFactory = serverFactory.getFileSystem();
+            final FileSystemView view = fileSystemFactory.createFileSystemView(ftpuser);
+            final FtpFile workingDirectory = view.getWorkingDirectory();
+            final Object physicalFile = workingDirectory.getPhysicalFile();
+            assertTrue(physicalFile instanceof File);
+            assertEquals(FOLDER.getAbsolutePath(), ((File) physicalFile).getAbsolutePath());
+            final ListenerFactory factory = new ListenerFactory();
+            factory.setPort(PORT);
+            serverFactory.addListener("default", factory.createListener());
+            FTP_SERVER = serverFactory.createServer();
+            FTP_SERVER.start();
+        }
+    }
+
+    /**
+     * Stop test FTP server (if using embedded).
+     *
+     * @throws IOException on failure to clean up test data
+     */
+    @AfterAll
+    static void afterAll() throws IOException {
+        if (EMBED_FTP_SERVER) {
+            FTP_SERVER.stop();
+            while (!FTP_SERVER.isStopped()) {
+                try {
+                    Thread.sleep(100L);
+                } catch (InterruptedException e) {
+                    throw new RuntimeException(e);
+                }
+            }
+            FileUtils.deleteDirectory(FOLDER);
+        }
+    }
+
+
+    /**
+     * Test connectivity to FTP server.
+     */
+    @Test
+    public void test0SeedFTPPut1() {

Review Comment:
   Changing to conform to integration test class naming.



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] greyp9 commented on a diff in pull request #6172: NIFI-10143 - ListFTP/GetFTP charset issues

Posted by GitBox <gi...@apache.org>.
greyp9 commented on code in PR #6172:
URL: https://github.com/apache/nifi/pull/6172#discussion_r912276213


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestFTPCharset.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.standard;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.ftpserver.FtpServer;
+import org.apache.ftpserver.FtpServerFactory;
+import org.apache.ftpserver.ftplet.FileSystemFactory;
+import org.apache.ftpserver.ftplet.FileSystemView;
+import org.apache.ftpserver.ftplet.FtpException;
+import org.apache.ftpserver.ftplet.FtpFile;
+import org.apache.ftpserver.ftplet.UserManager;
+import org.apache.ftpserver.listener.ListenerFactory;
+import org.apache.ftpserver.usermanager.ClearTextPasswordEncryptor;
+import org.apache.ftpserver.usermanager.PropertiesUserManagerFactory;
+import org.apache.ftpserver.usermanager.impl.BaseUser;
+import org.apache.nifi.processor.Processor;
+import org.apache.nifi.processor.util.list.AbstractListProcessor;
+import org.apache.nifi.processors.standard.util.FTPTransfer;
+import org.apache.nifi.util.MockFlowFile;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
+
+/**
+ * Seed a UTF-7 folder in FTP server with files using i18n known filenames.  Iterate through a set of i18n folder
+ * names, copying the known content into each new target.  Afterwards, verify that the last folder contains all
+ * files with the expected filenames.
+ * <p>
+ * To test against a live FTP server:
+ * <ol>
+ *     <li>replace EMBED_FTP_SERVER value with <code>false</code></li>
+ *     <li>replace HOSTNAME, PORT, USER, PASSWORD as needed</li>
+ * </ol>
+ */
+@TestMethodOrder(MethodOrderer.MethodName.class)
+public class TestFTPCharset {
+    private static final boolean EMBED_FTP_SERVER = true;
+    private static FtpServer FTP_SERVER;
+
+    private static final String USE_UTF8 = Boolean.TRUE.toString();
+    private static final String HOSTNAME = "localhost";
+    private static final int PORT = 2121;
+    private static final String USER = "ftpuser";
+    private static final String PASSWORD = "admin";
+    private static final File FOLDER = new File("target", TestFTPCharset.class.getSimpleName());
+
+    public static Stream<Arguments> folderNamesProvider() {
+        return Stream.of(
+                arguments("folder1", "folder2"),
+                arguments("folder2", "æøå"),
+                arguments("æøå", "folder3"),
+                arguments("folder3", "اختبار"),
+                arguments("اختبار", "folder4"),
+                arguments("folder4", "Госагїzатїой"),
+                arguments("Госагїzатїой", "folder5"),
+                arguments("folder5", "し回亡丹し工z丹卞工回几"),
+                arguments("し回亡丹し工z丹卞工回几", "folder6")
+        );
+    }
+
+    public static Stream<String> filenamesProvider() {
+        return Stream.of(
+                "1.txt",
+                "æøå.txt",
+                "اختبار.txt",
+                "Госагїzатїой.txt",
+                "し回亡丹し工z丹卞工回几.txt");
+    }
+
+    /**
+     * Start test FTP server (if using embedded).
+     *
+     * @throws FtpException on server startup failure
+     */
+    @BeforeAll
+    static void beforeAll() throws FtpException {
+        if (EMBED_FTP_SERVER) {
+            FOLDER.mkdirs();
+            // setup ftp user
+            final PropertiesUserManagerFactory userManagerFactory = new PropertiesUserManagerFactory();
+            userManagerFactory.setUrl(TestFTPCharset.class.getClassLoader()
+                    .getResource("TestFTPCharset/users.properties"));
+            userManagerFactory.setPasswordEncryptor(new ClearTextPasswordEncryptor());
+            final UserManager userManager = userManagerFactory.createUserManager();
+            final BaseUser ftpuser = (BaseUser) userManager.getUserByName(USER);
+            ftpuser.setHomeDirectory(FOLDER.getAbsolutePath());
+            userManager.save(ftpuser);
+            // setup embedded ftp server
+            final FtpServerFactory serverFactory = new FtpServerFactory();
+            serverFactory.setUserManager(userManager);
+            final FileSystemFactory fileSystemFactory = serverFactory.getFileSystem();
+            final FileSystemView view = fileSystemFactory.createFileSystemView(ftpuser);
+            final FtpFile workingDirectory = view.getWorkingDirectory();
+            final Object physicalFile = workingDirectory.getPhysicalFile();
+            assertTrue(physicalFile instanceof File);
+            assertEquals(FOLDER.getAbsolutePath(), ((File) physicalFile).getAbsolutePath());
+            final ListenerFactory factory = new ListenerFactory();
+            factory.setPort(PORT);
+            serverFactory.addListener("default", factory.createListener());
+            FTP_SERVER = serverFactory.createServer();
+            FTP_SERVER.start();
+        }
+    }
+
+    /**
+     * Stop test FTP server (if using embedded).
+     *
+     * @throws IOException on failure to clean up test data
+     */
+    @AfterAll
+    static void afterAll() throws IOException {
+        if (EMBED_FTP_SERVER) {
+            FTP_SERVER.stop();
+            while (!FTP_SERVER.isStopped()) {
+                try {
+                    Thread.sleep(100L);
+                } catch (InterruptedException e) {
+                    throw new RuntimeException(e);
+                }
+            }
+            FileUtils.deleteDirectory(FOLDER);
+        }
+    }
+
+
+    /**
+     * Test connectivity to FTP server.
+     */
+    @Test
+    public void test0SeedFTPPut1() {
+        final TestRunner runnerPut = provisionTestRunner(PutFTP.class);
+        final String folderName = "folder0";
+        final MockFlowFile flowFile = new MockFlowFile(1L);
+        flowFile.putAttributes(Collections.singletonMap("filename", "0.txt"));
+        flowFile.setData(new Date().toString().getBytes(UTF_8));
+        runnerPut.enqueue(flowFile);
+        runnerPut.setValidateExpressionUsage(false);
+        runnerPut.setProperty(FTPTransfer.REMOTE_PATH, folderName);
+        runnerPut.setProperty(FTPTransfer.CREATE_DIRECTORY, Boolean.TRUE.toString());
+        runnerPut.setProperty(FTPTransfer.DOT_RENAME, Boolean.FALSE.toString());
+        runnerPut.run();
+        runnerPut.assertTransferCount(PutFileTransfer.REL_FAILURE, 0);
+        runnerPut.assertTransferCount(PutFileTransfer.REL_REJECT, 0);
+        runnerPut.assertTransferCount(PutFileTransfer.REL_SUCCESS, 1);
+    }
+
+    /**
+     * Seed FTP server with data to be propagated.
+     */
+    @Test
+    public void test0SeedFTPPutAll() {
+        int id = 0;
+        final Object[] argumentsFirstFolder = folderNamesProvider().iterator().next().get();
+        final String folderName = Arrays.stream(argumentsFirstFolder).iterator().next().toString();
+        final TestRunner runnerPut = provisionTestRunner(PutFTP.class);
+        runnerPut.setValidateExpressionUsage(false);
+        runnerPut.setProperty(FTPTransfer.REMOTE_PATH, folderName);
+        runnerPut.setProperty(FTPTransfer.CREATE_DIRECTORY, Boolean.TRUE.toString());
+        runnerPut.setProperty(FTPTransfer.DOT_RENAME, Boolean.FALSE.toString());
+
+        final Iterator<String> iteratorFilenames = filenamesProvider().iterator();
+        while (iteratorFilenames.hasNext()) {
+            final String filename = iteratorFilenames.next();
+            final MockFlowFile flowFile = new MockFlowFile(++id);
+            flowFile.putAttributes(Collections.singletonMap("filename", filename));
+            flowFile.setData(new Date().toString().getBytes(UTF_8));
+            runnerPut.enqueue(flowFile);
+        }
+        final int fileCount = (int) filenamesProvider().count();
+        runnerPut.run();
+        runnerPut.assertTransferCount(PutFileTransfer.REL_FAILURE, 0);
+        runnerPut.assertTransferCount(PutFileTransfer.REL_REJECT, 0);
+        runnerPut.assertTransferCount(PutFileTransfer.REL_SUCCESS, fileCount);
+    }
+
+    /**
+     * Verify that data has been successfully propagated.
+     */
+    @Test
+    public void test9FTPVerifyAll() {
+        final Set<String> filenamesExpected = filenamesProvider().collect(Collectors.toSet());
+
+        final Object[] argumentsLastFolder = folderNamesProvider()
+                .reduce((prev, next) -> next).orElseThrow(IllegalStateException::new).get();
+        final String folderName = Arrays.stream(argumentsLastFolder)
+                .reduce((prev, next) -> next).orElseThrow(IllegalStateException::new).toString();
+        final TestRunner runnerList = provisionTestRunner(ListFTP.class);
+        runnerList.setProperty(ListFileTransfer.FILE_TRANSFER_LISTING_STRATEGY, AbstractListProcessor.NO_TRACKING);
+        runnerList.setProperty(FTPTransfer.REMOTE_PATH, folderName);
+        runnerList.clearTransferState();
+        runnerList.run(1);
+        runnerList.assertTransferCount("success", filenamesExpected.size());
+        final List<MockFlowFile> flowFilesList = runnerList.getFlowFilesForRelationship("success");
+        for (MockFlowFile flowFile : flowFilesList) {
+            filenamesExpected.remove(flowFile.getAttribute("filename"));
+        }
+        assertTrue(filenamesExpected.isEmpty());
+    }
+
+    /**
+     * For each parameterized invocation, copy files using FTP protocol from source folder to target folder.  This
+     * implicitly verifies charset handling of ListFTP, FetchFTP, and PutFTP processors.
+     *
+     * @param source the folder name from which to retrieve data
+     * @param target the folder name to which data should be copied
+     */
+    @ParameterizedTest
+    @MethodSource("folderNamesProvider")
+    public void test1FTP(final String source, final String target) {
+        final int fileCount = (int) filenamesProvider().count();
+        // ListFTP
+        final TestRunner runnerList = provisionTestRunner(ListFTP.class);
+        runnerList.setProperty(ListFileTransfer.FILE_TRANSFER_LISTING_STRATEGY, AbstractListProcessor.NO_TRACKING);
+        runnerList.setProperty(FTPTransfer.REMOTE_PATH, source);
+        runnerList.clearTransferState();
+        runnerList.run(1);
+        runnerList.assertTransferCount("success", fileCount);

Review Comment:
   Agreed.  I ran into some instances where these were private or package-protected.  Changing where possible.



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6172: NIFI-10143 - ListFTP/GetFTP charset issues

Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6172:
URL: https://github.com/apache/nifi/pull/6172#discussion_r912188451


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ftp/StandardFTPClientProvider.java:
##########
@@ -152,10 +153,10 @@ private void setClientProperties(final FTPClient client, final PropertyContext c
         client.setRemoteVerificationEnabled(false);
 
         final boolean unicodeEnabled = context.getProperty(UTF8_ENCODING).isSet() ? context.getProperty(UTF8_ENCODING).asBoolean() : false;
-        if (unicodeEnabled) {
-            client.setControlEncoding(StandardCharsets.UTF_8.name());
-            client.setAutodetectUTF8(true);
-        }
+        final Charset charset = unicodeEnabled ? StandardCharsets.UTF_8 : Charset.defaultCharset();
+        client.setCharset(charset);

Review Comment:
   After evaluating the Apache FTPClient class and parent classes, it appears that the `Charset` property is not used, so it seems best to avoid setting the value.



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] greyp9 commented on a diff in pull request #6172: NIFI-10143 - ListFTP/GetFTP charset issues

Posted by GitBox <gi...@apache.org>.
greyp9 commented on code in PR #6172:
URL: https://github.com/apache/nifi/pull/6172#discussion_r918267934


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestFTPCharsetIT.java:
##########
@@ -0,0 +1,304 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.standard;
+
+import org.apache.ftpserver.FtpServer;
+import org.apache.ftpserver.FtpServerFactory;
+import org.apache.ftpserver.ftplet.FileSystemFactory;
+import org.apache.ftpserver.ftplet.FileSystemView;
+import org.apache.ftpserver.ftplet.FtpException;
+import org.apache.ftpserver.ftplet.FtpFile;
+import org.apache.ftpserver.ftplet.UserManager;
+import org.apache.ftpserver.listener.ListenerFactory;
+import org.apache.ftpserver.usermanager.ClearTextPasswordEncryptor;
+import org.apache.ftpserver.usermanager.PropertiesUserManagerFactory;
+import org.apache.ftpserver.usermanager.impl.BaseUser;
+import org.apache.nifi.processor.Processor;
+import org.apache.nifi.processor.util.list.AbstractListProcessor;
+import org.apache.nifi.processors.standard.util.FTPTransfer;
+import org.apache.nifi.remote.io.socket.NetworkUtils;
+import org.apache.nifi.util.MockFlowFile;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Properties;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
+
+/**
+ * Seed an ASCII charset folder in FTP server with files using i18n known filenames.  Iterate through a set of i18n folder
+ * names, copying the known content into each new target.  Afterwards, verify that the last folder contains all
+ * files with the expected filenames.
+ * <p>
+ * To test against a live FTP server, run test with system property set
+ * <code>TestFTPCharset=hostname,port,user,password</code>, like
+ * <code>TestFTPCharset=localhost,21,ftpuser,ftppassword</code>.
+ */
+@TestMethodOrder(MethodOrderer.MethodName.class)
+public class TestFTPCharsetIT {
+    private static final String SERVER_OVERRIDE = System.getProperty(TestFTPCharsetIT.class.getSimpleName());
+    private static final boolean EMBED_FTP_SERVER = (SERVER_OVERRIDE == null);
+    private static FtpServer FTP_SERVER;
+
+    private static final String USE_UTF8 = Boolean.TRUE.toString();
+    private static final String HOSTNAME = "localhost";
+    private static final String PORT = Integer.toString(NetworkUtils.getAvailableTcpPort());
+    private static final String USER = "ftpuser";
+    private static final String PASSWORD = "admin";
+
+    @TempDir
+    private static File FOLDER_FTP;
+    @TempDir
+    private static File FOLDER_USER_PROPERTIES;
+
+    public static Arguments serverParametersProvider() {
+        final String override = System.getProperty(TestFTPCharsetIT.class.getSimpleName());
+        if (override == null) {
+            return arguments(HOSTNAME, PORT, USER, PASSWORD);
+        } else {
+            return arguments((Object[]) override.split(","));
+        }
+    }
+
+    public static Stream<Arguments> folderNamesProvider() {
+        return Stream.of(
+                arguments("folder1", "folder2"),
+                arguments("folder2", "æøå"),
+                arguments("æøå", "folder3"),
+                arguments("folder3", "اختبار"),
+                arguments("اختبار", "folder4"),
+                arguments("folder4", "Госагїzатїой"),
+                arguments("Госагїzатїой", "folder5"),
+                arguments("folder5", "し回亡丹し工z丹卞工回几"),
+                arguments("し回亡丹し工z丹卞工回几", "folder6")
+        );
+    }
+
+    public static Stream<String> filenamesProvider() {
+        return Stream.of(
+                "1.txt",
+                "æøå.txt",
+                "اختبار.txt",
+                "Госагїzатїой.txt",
+                "し回亡丹し工z丹卞工回几.txt");
+    }
+
+    @BeforeAll
+    static void startEmbeddedServer() throws IOException, FtpException {
+        if (EMBED_FTP_SERVER) {
+            // setup ftp user
+            final Properties userProperties = new Properties();
+            userProperties.setProperty("ftpserver.user.ftpuser.idletime", "0");
+            userProperties.setProperty("ftpserver.user.ftpuser.enableflag", "true");
+            userProperties.setProperty("ftpserver.user.ftpuser.userpassword", "admin");
+            userProperties.setProperty("ftpserver.user.ftpuser.writepermission", "true");
+            userProperties.setProperty("ftpserver.user.ftpuser.homedirectory", FOLDER_FTP.getAbsolutePath());
+            final File userPropertiesFile = new File(FOLDER_USER_PROPERTIES, "user.properties");
+            try (final FileOutputStream fos = new FileOutputStream(userPropertiesFile)) {
+                userProperties.store(fos, "ftp-user-properties");
+            }
+            final PropertiesUserManagerFactory userManagerFactory = new PropertiesUserManagerFactory();
+            userManagerFactory.setUrl(userPropertiesFile.toURI().toURL());
+            userManagerFactory.setPasswordEncryptor(new ClearTextPasswordEncryptor());
+            final UserManager userManager = userManagerFactory.createUserManager();
+            final BaseUser ftpuser = (BaseUser) userManager.getUserByName(USER);
+            // setup embedded ftp server
+            final FtpServerFactory serverFactory = new FtpServerFactory();
+            serverFactory.setUserManager(userManager);
+            final FileSystemFactory fileSystemFactory = serverFactory.getFileSystem();
+            final FileSystemView view = fileSystemFactory.createFileSystemView(ftpuser);
+            final FtpFile workingDirectory = view.getWorkingDirectory();
+            final Object physicalFile = workingDirectory.getPhysicalFile();
+            assertInstanceOf(File.class, physicalFile);
+            assertEquals(FOLDER_FTP.getAbsolutePath(), ((File) physicalFile).getAbsolutePath());
+            final ListenerFactory factory = new ListenerFactory();
+            factory.setPort(Integer.parseInt(PORT));
+            serverFactory.addListener("default", factory.createListener());
+            FTP_SERVER = serverFactory.createServer();
+            FTP_SERVER.start();
+        }
+    }
+
+    @AfterAll
+    static void stopEmbeddedServer() throws InterruptedException {
+        if (EMBED_FTP_SERVER) {
+            FTP_SERVER.stop();
+            while (!FTP_SERVER.isStopped()) {
+                Thread.sleep(100L);
+            }

Review Comment:
   Thanks.  In an earlier iteration I was starting an FTP server for each test case; when done once, this is unneeded.



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6172: NIFI-10143 - ListFTP/GetFTP charset issues

Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6172:
URL: https://github.com/apache/nifi/pull/6172#discussion_r918238256


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ftp/StandardFTPClientProvider.java:
##########
@@ -150,12 +151,11 @@ private void setClientProperties(final FTPClient client, final PropertyContext c
         client.setDataTimeout(dataTimeout);
         client.setDefaultTimeout(connectionTimeout);
         client.setRemoteVerificationEnabled(false);
+        client.setAutodetectUTF8(true);
 
         final boolean unicodeEnabled = context.getProperty(UTF8_ENCODING).isSet() ? context.getProperty(UTF8_ENCODING).asBoolean() : false;
-        if (unicodeEnabled) {
-            client.setControlEncoding(StandardCharsets.UTF_8.name());
-            client.setAutodetectUTF8(true);
-        }
+        final Charset charset = unicodeEnabled ? StandardCharsets.UTF_8 : Charset.defaultCharset();

Review Comment:
   This ternary changes the default behavior. The `FTPClient` defaults to `ISO-8859-1` for the Control Encoding property, so the approach should be adjusted to follow the previous approach of setting control encoding only when `unicodeEnabled` is `true`.



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestFTPCharsetIT.java:
##########
@@ -0,0 +1,304 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.standard;
+
+import org.apache.ftpserver.FtpServer;
+import org.apache.ftpserver.FtpServerFactory;
+import org.apache.ftpserver.ftplet.FileSystemFactory;
+import org.apache.ftpserver.ftplet.FileSystemView;
+import org.apache.ftpserver.ftplet.FtpException;
+import org.apache.ftpserver.ftplet.FtpFile;
+import org.apache.ftpserver.ftplet.UserManager;
+import org.apache.ftpserver.listener.ListenerFactory;
+import org.apache.ftpserver.usermanager.ClearTextPasswordEncryptor;
+import org.apache.ftpserver.usermanager.PropertiesUserManagerFactory;
+import org.apache.ftpserver.usermanager.impl.BaseUser;
+import org.apache.nifi.processor.Processor;
+import org.apache.nifi.processor.util.list.AbstractListProcessor;
+import org.apache.nifi.processors.standard.util.FTPTransfer;
+import org.apache.nifi.remote.io.socket.NetworkUtils;
+import org.apache.nifi.util.MockFlowFile;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Properties;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
+
+/**
+ * Seed an ASCII charset folder in FTP server with files using i18n known filenames.  Iterate through a set of i18n folder
+ * names, copying the known content into each new target.  Afterwards, verify that the last folder contains all
+ * files with the expected filenames.
+ * <p>
+ * To test against a live FTP server, run test with system property set
+ * <code>TestFTPCharset=hostname,port,user,password</code>, like
+ * <code>TestFTPCharset=localhost,21,ftpuser,ftppassword</code>.
+ */
+@TestMethodOrder(MethodOrderer.MethodName.class)
+public class TestFTPCharsetIT {
+    private static final String SERVER_OVERRIDE = System.getProperty(TestFTPCharsetIT.class.getSimpleName());
+    private static final boolean EMBED_FTP_SERVER = (SERVER_OVERRIDE == null);
+    private static FtpServer FTP_SERVER;
+
+    private static final String USE_UTF8 = Boolean.TRUE.toString();
+    private static final String HOSTNAME = "localhost";
+    private static final String PORT = Integer.toString(NetworkUtils.getAvailableTcpPort());
+    private static final String USER = "ftpuser";
+    private static final String PASSWORD = "admin";
+
+    @TempDir
+    private static File FOLDER_FTP;
+    @TempDir
+    private static File FOLDER_USER_PROPERTIES;
+
+    public static Arguments serverParametersProvider() {
+        final String override = System.getProperty(TestFTPCharsetIT.class.getSimpleName());
+        if (override == null) {
+            return arguments(HOSTNAME, PORT, USER, PASSWORD);
+        } else {
+            return arguments((Object[]) override.split(","));
+        }
+    }
+
+    public static Stream<Arguments> folderNamesProvider() {
+        return Stream.of(
+                arguments("folder1", "folder2"),
+                arguments("folder2", "æøå"),
+                arguments("æøå", "folder3"),
+                arguments("folder3", "اختبار"),
+                arguments("اختبار", "folder4"),
+                arguments("folder4", "Госагїzатїой"),
+                arguments("Госагїzатїой", "folder5"),
+                arguments("folder5", "し回亡丹し工z丹卞工回几"),
+                arguments("し回亡丹し工z丹卞工回几", "folder6")
+        );
+    }
+
+    public static Stream<String> filenamesProvider() {
+        return Stream.of(
+                "1.txt",
+                "æøå.txt",
+                "اختبار.txt",
+                "Госагїzатїой.txt",
+                "し回亡丹し工z丹卞工回几.txt");
+    }
+
+    @BeforeAll
+    static void startEmbeddedServer() throws IOException, FtpException {
+        if (EMBED_FTP_SERVER) {
+            // setup ftp user
+            final Properties userProperties = new Properties();
+            userProperties.setProperty("ftpserver.user.ftpuser.idletime", "0");
+            userProperties.setProperty("ftpserver.user.ftpuser.enableflag", "true");
+            userProperties.setProperty("ftpserver.user.ftpuser.userpassword", "admin");
+            userProperties.setProperty("ftpserver.user.ftpuser.writepermission", "true");
+            userProperties.setProperty("ftpserver.user.ftpuser.homedirectory", FOLDER_FTP.getAbsolutePath());
+            final File userPropertiesFile = new File(FOLDER_USER_PROPERTIES, "user.properties");
+            try (final FileOutputStream fos = new FileOutputStream(userPropertiesFile)) {
+                userProperties.store(fos, "ftp-user-properties");
+            }
+            final PropertiesUserManagerFactory userManagerFactory = new PropertiesUserManagerFactory();
+            userManagerFactory.setUrl(userPropertiesFile.toURI().toURL());
+            userManagerFactory.setPasswordEncryptor(new ClearTextPasswordEncryptor());
+            final UserManager userManager = userManagerFactory.createUserManager();
+            final BaseUser ftpuser = (BaseUser) userManager.getUserByName(USER);
+            // setup embedded ftp server
+            final FtpServerFactory serverFactory = new FtpServerFactory();
+            serverFactory.setUserManager(userManager);
+            final FileSystemFactory fileSystemFactory = serverFactory.getFileSystem();
+            final FileSystemView view = fileSystemFactory.createFileSystemView(ftpuser);
+            final FtpFile workingDirectory = view.getWorkingDirectory();
+            final Object physicalFile = workingDirectory.getPhysicalFile();
+            assertInstanceOf(File.class, physicalFile);
+            assertEquals(FOLDER_FTP.getAbsolutePath(), ((File) physicalFile).getAbsolutePath());
+            final ListenerFactory factory = new ListenerFactory();
+            factory.setPort(Integer.parseInt(PORT));
+            serverFactory.addListener("default", factory.createListener());
+            FTP_SERVER = serverFactory.createServer();
+            FTP_SERVER.start();
+        }
+    }
+
+    @AfterAll
+    static void stopEmbeddedServer() throws InterruptedException {
+        if (EMBED_FTP_SERVER) {
+            FTP_SERVER.stop();
+            while (!FTP_SERVER.isStopped()) {
+                Thread.sleep(100L);
+            }

Review Comment:
   Is there any other way to wait for the server to stop without this loop?



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] greyp9 commented on a diff in pull request #6172: NIFI-10143 - ListFTP/GetFTP charset issues

Posted by GitBox <gi...@apache.org>.
greyp9 commented on code in PR #6172:
URL: https://github.com/apache/nifi/pull/6172#discussion_r912281332


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestFTPCharset.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.standard;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.ftpserver.FtpServer;
+import org.apache.ftpserver.FtpServerFactory;
+import org.apache.ftpserver.ftplet.FileSystemFactory;
+import org.apache.ftpserver.ftplet.FileSystemView;
+import org.apache.ftpserver.ftplet.FtpException;
+import org.apache.ftpserver.ftplet.FtpFile;
+import org.apache.ftpserver.ftplet.UserManager;
+import org.apache.ftpserver.listener.ListenerFactory;
+import org.apache.ftpserver.usermanager.ClearTextPasswordEncryptor;
+import org.apache.ftpserver.usermanager.PropertiesUserManagerFactory;
+import org.apache.ftpserver.usermanager.impl.BaseUser;
+import org.apache.nifi.processor.Processor;
+import org.apache.nifi.processor.util.list.AbstractListProcessor;
+import org.apache.nifi.processors.standard.util.FTPTransfer;
+import org.apache.nifi.util.MockFlowFile;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
+
+/**
+ * Seed a UTF-7 folder in FTP server with files using i18n known filenames.  Iterate through a set of i18n folder
+ * names, copying the known content into each new target.  Afterwards, verify that the last folder contains all
+ * files with the expected filenames.
+ * <p>
+ * To test against a live FTP server:
+ * <ol>
+ *     <li>replace EMBED_FTP_SERVER value with <code>false</code></li>
+ *     <li>replace HOSTNAME, PORT, USER, PASSWORD as needed</li>
+ * </ol>

Review Comment:
   Added method `serverParametersProvider()`, which allows for override based on a system property.



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] greyp9 commented on pull request #6172: NIFI-10143 - ListFTP/GetFTP charset issues

Posted by GitBox <gi...@apache.org>.
greyp9 commented on PR #6172:
URL: https://github.com/apache/nifi/pull/6172#issuecomment-1172776789

   > Thanks for digging into this issue and developing a new test @greyp9! The ClientProvider changes make sense.
   > 
   > The new test is helpful, and covers a lot of ground. It does a lot of work, and it seems to stretch the boundary of a unit test versus an integration test. In light of having existing unit tests for FTP processors, it seems better to maintain a minimal unit test, and then provide this new test class as an integration test. As noted in the detailed comments, there are several ways the test could be optimized. Supporting an external FTP server is also useful, and could be parameterized through System properties or environment variables.
   > 
   > What do you think about changing the test to an integration test, an updating existing unit tests with more minimal changes to incorporate evaluation of UTF-8 encoding?
   
   I've renamed to *IT to align with integration test naming convention.  I'd like to defer modifications of the existing unit tests to be part of future efforts, as needed.


-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org