[
https://issues.apache.org/jira/browse/NET-620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15894325#comment-15894325
]
Chris Collins edited comment on NET-620 at 3/3/17 2:13 PM:
-----------------------------------------------------------
Ok, so it appears that the breaking up of \_connectAction\_() and
\_connectAction\_(Reader reader) was done to re-use existing readers when
possible instead of creating new ones, but doesn't allow you do define your own
reader and pass it in.
This appears to be due to \_input\_ not being initialized until
SocketClient.\_connectAction\_ is called through the chain mentioned in the
previous comment. I thought that maybe ToNetASCIIInputStream might get
\_input\_ initialized if the inputstream was passed by reference but it doesn't
appear to. Through remote debugging I can clearly see that when the reader is
passed in FTP.\_\_getReply:321 is definitely working on a null InputStream
within the InputStreamReader. If I reformat FTPClient.\_connectAction\_ like
this for testing purposes everything works fine including the newlines getting
fixed:
{noformat}
protected void _connectAction_(Reader socketIsReader) throws IOException {
super._connectAction_(); // sets up _input_ and _output_
if(socketIsReader == null) {
_controlInput_ =
new CRLFLineReader(new InputStreamReader(new
ToNetASCIIInputStream(_input_), getControlEncoding()));
} else {
_controlInput_ = new CRLFLineReader(socketIsReader);
}
{noformat}
This shows that wrapping the InputStream in ToNetASCIIInputStream does in fact
fix my issue, so maybe I'm back to making a strictNewlineProcessing option that
can be set and create the reader accordingly.
Looking at these results and how things are organized and where initializations
happen does this Null InputStream make sense to you as well Sebb?
was (Author: unsta):
Ok, so it appears that the breaking up of \_connectAction\_() and
\_connectAction\_(Reader reader) was done to re-use existing readers when
possible instead of creating new ones, but doesn't allow you do define your own
reader and pass it in.
This appears to be due to \_input\_ not being initialized until
SocketClient.\_connectAction\_ is called through the chain mentioned in the
previous comment. I thought that maybe ToNetASCIIInputStream might get input
initialized if the inputstream was passed by reference but it doesn't appear
to. Through remote debugging I can clearly see that when the reader is passed
in FTP.\_\_getReply:321 is definitely working on a null InputStream within the
InputStreamReader. If I reformat FTPClient.\_connectAction\_ like this for
testing purposes everything works fine including the newlines getting fixed:
{noformat}
protected void _connectAction_(Reader socketIsReader) throws IOException {
super._connectAction_(); // sets up _input_ and _output_
if(socketIsReader == null) {
_controlInput_ =
new CRLFLineReader(new InputStreamReader(new
ToNetASCIIInputStream(_input_), getControlEncoding()));
} else {
_controlInput_ = new CRLFLineReader(socketIsReader);
}
{noformat}
This shows that wrapping the InputStream in ToNetASCIIInputStream does in fact
fix my issue, so maybe I'm back to making a strictNewlineProcessing option that
can be set and create the reader accordingly.
Looking at these results and how things are organized and where initializations
happen does this Null InputStream make sense to you as well Sebb?
> Strict CRLF handling in Commons-NET FTP breaks compatibility with some FTP
> servers
> ----------------------------------------------------------------------------------
>
> Key: NET-620
> URL: https://issues.apache.org/jira/browse/NET-620
> Project: Commons Net
> Issue Type: Bug
> Components: FTP
> Affects Versions: 3.3, 3.4, 3.5, 3.6
> Environment: Java RHEL Linux
> Reporter: Chris Collins
>
> The fix for FTP.java in NET-401 to switch from using BufferedReader to
> CRLFLineReader breaks the ability to connect to servers that have varying LF
> and CRLF line termination in the banner.
> I've run into 2 different cases with slightly different banner configs, one
> where you end up hung indefinitely by not reading far enough (this is the
> sigquit from when it is hung):
> 1. Thread=FTP Provider Protocol Provider Thread: class com.xxxx.xxxxx.xxx
> (00007F35A1AF4A00) Status=Running
> at
> java/net/SocketInputStream.socketRead0(Ljava/io/FileDescriptor;[BIII)I
> (Native Method)
> at java/net/SocketInputStream.read([BIII)I
> (SocketInputStream.java:164) (Compiled Code)
> at java/net/SocketInputStream.read([BII)I (SocketInputStream.java:134)
> (Compiled Code)
> at sun/nio/cs/StreamDecoder.readBytes()I (StreamDecoder.java:323)
> (Compiled Code)
> at sun/nio/cs/StreamDecoder.implRead([CII)I (StreamDecoder.java:365)
> (Compiled Code)
> at sun/nio/cs/StreamDecoder.read([CII)I (StreamDecoder.java:211)
> (Compiled Code)
> at java/io/InputStreamReader.read([CII)I (InputStreamReader.java:206)
> (Compiled Code)
> at java/io/BufferedReader.fill()V (BufferedReader.java:166) (Compiled
> Code)
> at java/io/BufferedReader.read()I (BufferedReader.java:187) (Compiled
> Code)
> at
> org/apache/commons/net/io/CRLFLineReader.readLine()Ljava/lang/String;
> (CRLFLineReader.java:58)
> at org/apache/commons/net/ftp/FTP.__getReply(Z)V (FTP.java:357)
> at org/apache/commons/net/ftp/FTP.__getReply()V (FTP.java:300)
> at org/apache/commons/net/ftp/FTP._connectAction_(Ljava/io/Reader;)V
> (FTP.java:438)
> at
> org/apache/commons/net/ftp/FTPClient._connectAction_(Ljava/io/Reader;)V
> (FTPClient.java:962)
> at org/apache/commons/net/ftp/FTPClient._connectAction_()V
> (FTPClient.java:950)
> at
> org/apache/commons/net/SocketClient._connect(Ljava/net/InetAddress;ILjava/net/InetAddress;I)V
> (SocketClient.java:244)
> at
> org/apache/commons/net/SocketClient.connect(Ljava/net/InetAddress;I)V
> (SocketClient.java:181)
> 2. And one where you error out by reading too far and getting a null back:
> Caused by:
> org.apache.commons.net.ftp.FTPConnectionClosedException: Connection closed
> without indication.
> at org.apache.commons.net.ftp.FTP.__getReply(FTP.java:317)
> at org.apache.commons.net.ftp.FTP.__getReply(FTP.java:294)
> at org.apache.commons.net.ftp.FTP.sendCommand(FTP.java:483)
> at org.apache.commons.net.ftp.FTP.sendCommand(FTP.java:608)
> at org.apache.commons.net.ftp.FTP.user(FTP.java:753)
> at org.apache.commons.net.ftp.FTPClient.login(FTPClient.java:1034)
> I do have hex data available to show the source data, but the end result is
> there's a mix of 0d0a (CRLF) and 0a (LF) termination in the FTP banner (220-)
> I can modify the library to undo the NET-401 change, but ideally it'd be nice
> to have a strictNewline type of setting you could set on the FTPClient object
> to decide if you want to be ultra-strict, or ultra compatible. I will be
> filing a defect with Cisco about this as well, but it would be great if the
> FTPClient had the option to handle it instead of forced compatibility with no
> options to relax it.
> This is kind of hinted at in one of the comments on NET-402 by Bogdan
> Drozdowski on 12/Apr/11
> "So, on one hand, we stop supporting non-conforming servers (which could mean
> that we're supporting less servers now), but on the other hand we're fixing a
> bug that someone has found in a real-life system."
> Giving users the option to relax the conformity requirements (but strict by
> default) would allow the end user to choose the option.
> Any thoughts on this?
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
