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/05/10 15:13:57 UTC
[GitHub] [nifi] exceptionfactory opened a new pull request, #6030: NIFI-9990 Improve FTP 550 file unavailable handling
exceptionfactory opened a new pull request, #6030:
URL: https://github.com/apache/nifi/pull/6030
# Summary
[NIFI-9990](https://issues.apache.org/jira/browse/NIFI-9990) Improves handling of FTP `550` reply codes to detect whether the reply message indicates that the file is not found or that there is a permission restriction.
The `550` reply code indicates that the remote file is unavailable, but does not indicate the specific reason. The current file not found detection is based on a specific message, and does not indicate permission problems.
Changes include using a more flexible regular expression pattern to detect when a file is not found. If the reply message does not match the pattern, the `550` reply code results in a permission denied exception. Additional updates include adding unit tests to exercise these conditions.
# 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
- [ ] JDK 11
- [ ] JDK 17
### Licensing
- [ ] 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] joewitt commented on a diff in pull request #6030: NIFI-9990 Improve FTP 550 file unavailable handling
Posted by GitBox <gi...@apache.org>.
joewitt commented on code in PR #6030:
URL: https://github.com/apache/nifi/pull/6030#discussion_r872417635
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/util/FTPTransfer.java:
##########
@@ -321,13 +325,22 @@ public FlowFile getRemoteFile(final String remoteFileName, final FlowFile origFl
FlowFile resultFlowFile;
try (InputStream in = client.retrieveFileStream(remoteFileName)) {
if (in == null) {
- final String response = client.getReplyString();
- // FTPClient doesn't throw exception if file not found.
- // Instead, response string will contain: "550 Can't open <absolute_path>: No such file or directory"
- if (response != null && response.trim().endsWith("No such file or directory")) {
- throw new FileNotFoundException(response);
+ final int replyCode = client.getReplyCode();
+
+ final String reply = client.getReplyString();
+ if (reply == null) {
Review Comment:
I did not review the ftp client code but I'd imagine the reply code is extracted by it from the reply string. Here we pull the code, then the string, then check if the string is null. I suspect we want code extraction AFTER the null and/or empty check.
--
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 #6030: NIFI-9990 Improve FTP 550 file unavailable handling
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6030:
URL: https://github.com/apache/nifi/pull/6030#discussion_r872434557
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/util/FTPTransfer.java:
##########
@@ -321,13 +325,22 @@ public FlowFile getRemoteFile(final String remoteFileName, final FlowFile origFl
FlowFile resultFlowFile;
try (InputStream in = client.retrieveFileStream(remoteFileName)) {
if (in == null) {
- final String response = client.getReplyString();
- // FTPClient doesn't throw exception if file not found.
- // Instead, response string will contain: "550 Can't open <absolute_path>: No such file or directory"
- if (response != null && response.trim().endsWith("No such file or directory")) {
- throw new FileNotFoundException(response);
+ final int replyCode = client.getReplyCode();
+
+ final String reply = client.getReplyString();
+ if (reply == null) {
Review Comment:
Thanks for the feedback @joewitt! After taking a closer look at the commons-net FTP code, it appears that the FTP client will through an exception if the reply string is `null` when attempting to parse the reply code, so will move `getReplyCode()` after `getReplyString()`.
--
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] asfgit closed pull request #6030: NIFI-9990 Improve FTP 550 file unavailable handling
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #6030: NIFI-9990 Improve FTP 550 file unavailable handling
URL: https://github.com/apache/nifi/pull/6030
--
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