[
https://issues.apache.org/jira/browse/NET-584?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15905925#comment-15905925
]
Nick Manley commented on NET-584:
---------------------------------
Hey, Sebb. Thank you for the prompt response. I had some time to look at this
today while I was at work. I dug into some of the code to better understand
what it was doing and I think I misunderstood what the controlKeepAlive was
doing.
bq. The method only affects the behaviour with certain retrieve/store methods
(as per the Javadoc).
I didn't realize that at first. I thought it was something that happens
continuously. As in, a NoOp would get sent every X seconds from the time I
connect to the time I disconnect.
bq. If it does not help, then don't use it - not all FTP servers support
control connection communication whilst a transfer is in progress.
I didn't know this either. I was under the impression that if I had a long
upload, I had to do something to keep the control connection alive or it would
time out. Removing the setControlKeepAliveTimeout had the opposite effect I was
expecting. I thought for sure that without it, the control connection would
timeout during a long upload. That doesn't seem to be the case though.
bq. However I don't understand why this worked in 3.3 but does not in 3.5.
Could have just been an issue of me not uploading a large enough file to
trigger it. Don't take that to mean the issue doesn't exist in 3.3, just that I
didn't happen to see it trigger when I was testing out different versions.
bq. Maybe the code should ignore SocketTimeoutException at this point in case
the responses were lost.
Maybe. But it looks like the exception is being triggered when it's reading the
reply. Prior to that, the {{CSL}} class is trying to clean-up any outstanding
NoOps. I think that's the part that might be exhausting the input stream and it
maybe eating the transfer reply along the way. I don't know if ignoring the
exception outright is the correct behavior.
After a bit more research and understanding, I'm willing to say that this
probably isn't a bug in Commons Net; just a misuse on my part. I might suggest
that some of the documentation on {{controlKeepAliveTimeout}} that exists on
the {{FTPClient}} class also be put into the setter. That would make it more
discoverable. Also, do you think it is worth adding to the javadoc what you
said about not all FTP servers supporting simultaneous writing to the
control/data socket? In some senses the javadoc sounds like it's saying "it's a
good idea to use this" (a best practice to follow) rather than "use this if you
have problems" (a workaround).
The original reporter seems to have had a similar misunderstanding as did the
users on NET-486. I don't know how much work it would be to have the code
handle this a bit nicer (either not throw an exception, or throw something
nicer than "read timed out"). But if that proves to be too much of a hassle,
then re-wording the javadocs would go a long way. Just something that would
signify it as a workaround for particular users and not a best practice that
someone should be doing.
> Error with org.apache.commons.net.ftp.FTPClient
> ------------------------------------------------
>
> Key: NET-584
> URL: https://issues.apache.org/jira/browse/NET-584
> Project: Commons Net
> Issue Type: Bug
> Components: FTP
> Reporter: Kazantsev Andrey Sergeevich
>
> I have a question about using library commons-net-3.4.jar
> Question is about org.apache.commons.net.ftp.FTPClient method
> setControlKeepAliveTimeout.
> Read about using it on:
> https://commons.apache.org/proper/commons-net/apidocs/org/apache/commons/net/ftp/FTPClient.html
> When I use it in my code I get this error:
> {code}
> java.net.SocketTimeoutException: Read timed out
> at java.net.SocketInputStream.socketRead0(Native Method)
> at java.net.SocketInputStream.read(SocketInputStream.java:163)
> at java.net.SocketInputStream.read(SocketInputStream.java:133)
> at sun.nio.cs.StreamDecoder.readBytes(StreamDecoder.java:322)
> at sun.nio.cs.StreamDecoder.implRead(StreamDecoder.java:364)
> at sun.nio.cs.StreamDecoder.read(StreamDecoder.java:210)
> at java.io.InputStreamReader.read(InputStreamReader.java:205)
> at java.io.BufferedReader.fill(BufferedReader.java:165)
> at java.io.BufferedReader.read(BufferedReader.java:186)
> at
> org.apache.commons.net.io.CRLFLineReader.readLine(CRLFLineReader.java:58)
> at org.apache.commons.net.ftp.FTP.__getReply(FTP.java:313)
> at org.apache.commons.net.ftp.FTP.__getReplyNoReport(FTP.java:303)
> at org.apache.commons.net.ftp.FTPClient$CSL.cleanUp(FTPClient.java:3838)
> at org.apache.commons.net.ftp.FTPClient._storeFile(FTPClient.java:695)
> at org.apache.commons.net.ftp.FTPClient.__storeFile(FTPClient.java:643)
> at org.apache.commons.net.ftp.FTPClient.storeFile(FTPClient.java:2033)
> at ru.mdm.File.Transfer.FTP.PutRemoteFileBinary(FTP.java:192)
> at
> ru.mdm.File.Transfer.TimeLimit.Thread.Protocol.PutRemoteFileBinaryThread.actionsToExecute(PutRemoteFileBinaryThread.java:23)
> at
> ru.mdm.File.Transfer.TimeLimit.OperationThread.run(OperationThread.java:60)
> {code}
> Without enabling this option all works fine.
> Here is the code:
> {code}
> package ru.mdm.File.Transfer;
> import bin.ru.osa.common.utils.*;
> import java.util.List;
> import java.io.*;
> import com.ibm.broker.javacompute.MbJavaComputeNode;
> import com.ibm.broker.plugin.*;
> import org.apache.commons.net.ftp.*;
> import org.apache.commons.net.*;
> import ru.mdm.File.Transfer.Options.OptionsXMLProcessor;
> public class FTP implements Protocol
> {
>
> FTPClient client = new FTPClient();
>
> OptionsXMLProcessor optionsXMLProcessor;
>
>
> boolean st;
> String LastMessage = new String();
>
> boolean ignoreErrors = false;
>
>
> public FTP()
> {
> super();
> }
>
> protected void finalize() { disconnect(); }
>
> public void connect(String CntName,
> String Host,
> String Port,
> String L,
> String P) throws Exception
> {
> try
> {
> client.setControlKeepAliveTimeout(300);
> client.connect(Host);
> client.login(L, P);
> CheckState();
> }
> catch(Exception e)
> {
> LastMessage=client.getReplyString();
> if(LastMessage == null) LastMessage = e.getMessage();
>
> e.printStackTrace();
>
> throw e;
> }
> }
>
> public void disconnect()
> {
> try
> {
> if(client.isConnected())
> {
> client.logout();
> client.disconnect();
> }
> }
> catch(Exception e)
> {
> e.printStackTrace();
> }
> }
>
> public void chmod(String RemoteFile, String Rights) throws Exception
> {
> client.sendSiteCommand("chmod "+RemoteFile+" "+Rights);
> CheckState();
> }
>
>
> public void lsMB(MbElement InputDir,MbElement filelist) throws
> Exception, MbException
> {
> MbElement xfile;
>
> for (FTPFile file :
> client.listFiles((String)InputDir.evaluateXPath("string(SOURCE_PATH)")))
> {
> if(!file.isFile()) continue; //-- No sub-dirs, No Symlinks !
>
>
> xfile=filelist.createElementAsLastChild(MbElement.TYPE_NAME,
> "File", null);
> xfile.createElementAsLastChild(MbElement.TYPE_NAME, "FileName",
> file.getName());
> xfile.createElementAsLastChild(MbElement.TYPE_NAME, "FileSize",
> file.getSize());
> xfile.createElementAsLastChild(MbElement.TYPE_NAME, "SourcePath",
> (String)InputDir.evaluateXPath("string(SOURCE_PATH)"));
> xfile.createElementAsLastChild(MbElement.TYPE_NAME,
> "SourceGateway", (String)InputDir.evaluateXPath("string(GATEWAY_NAME)"));
> }
> }
>
> public void mkdir(String RemotePath) throws Exception
> {
> client.makeDirectory(RemotePath);
> CheckState();
> }
>
> public void chdir(String RemotePath) throws Exception
> {
> client.changeWorkingDirectory(RemotePath);
> CheckState();
> }
>
> public void delete(String RemotePath) throws Exception
> {
> client.deleteFile(RemotePath);
> CheckState();
> }
>
>
>
> public void rename(String RemoteFileSrc, String RemoteFileDst)
> throws Exception
> {
> client.rename(RemoteFileSrc, RemoteFileDst);
> CheckState();
> }
>
>
> public void GetRemoteFileBinary(String RemoteFile, String LocalFile)
> throws Exception
> {
> client.enterLocalPassiveMode();
> client.setFileType(FTPClient.BINARY_FILE_TYPE);
> client.retrieveFile(RemoteFile,
> new FileOutputStream(LocalFile));
>
>
> CheckState();
> }
>
> public void PutRemoteFileBinary(String LocalFile, String RemoteFile)
> throws Exception
> {
> client.enterLocalPassiveMode();
> client.setFileType(FTPClient.BINARY_FILE_TYPE);
> client.storeFile(RemoteFile,
> new FileInputStream(LocalFile));
> CheckState();
> }
>
>
> public void ignoreErrors(boolean x) { ignoreErrors=x; }
>
>
> public boolean isOK() { return st; }
>
> public boolean isConnected()
> {
> boolean answer=false;
> try {
> answer = client.sendNoOp();
> } catch (IOException e) {
> // TODO Auto-generated catch block
> e.printStackTrace();
> }
> st = answer;
> return answer;
>
> }
>
>
> public String LastMessage() { return LastMessage; };
>
>
> public void CheckState(boolean state) throws Exception
> {
> int reply = client.getReplyCode();
> if(FTPReply.isPositiveCompletion(reply)) st=true;
> else st=false;
>
> LastMessage=client.getReplyString();
>
> if(!st && !ignoreErrors)
> throw new Exception(LastMessage);
> }
>
> public void CheckState() throws Exception
> {
> int reply = client.getReplyCode();
> if(FTPReply.isPositiveCompletion(reply)) st=true;
> else st=false;
>
> LastMessage=client.getReplyString();
>
> if(!st && !ignoreErrors)
> throw new Exception(LastMessage);
> }
>
> public void attachOptions(OptionsXMLProcessor optionsXMLProcessor)
> throws Exception
> {
> this.optionsXMLProcessor = optionsXMLProcessor;
> }
>
> public String getIP() {
>
> return "";
> }
>
> public boolean isIgnoreErrors()
> {
> return ignoreErrors;
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
