[ 
https://issues.apache.org/jira/browse/NET-620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15892867#comment-15892867
 ] 

Chris Collins edited comment on NET-620 at 3/2/17 7:53 PM:
-----------------------------------------------------------

So I was hoping originally to be able to just use the library as-is and extend 
FTPClient in my code and override _connectAction_ but to recreate it and use a 
different reader as it is I couldn't as __initDefaults() and  
__autodetectEncoding are both private and not accessible by subclasses. Maybe 
I'm just missing something here but it doesn't seem possible with the current 
code.

(from commons-net-3.3)

{noformat}
    protected void _connectAction_() throws IOException
    {
        super._connectAction_(); // sets up _input_ and _output_
        __initDefaults();
        // must be after super._connectAction_(), because otherwise we get an
        // Exception claiming we're not connected
        if ( __autodetectEncoding )
        {
            ArrayList<String> oldReplyLines = new ArrayList<String> 
(_replyLines);
            int oldReplyCode = _replyCode;
            if ( hasFeature("UTF8") || hasFeature("UTF-8")) // UTF8 appears to 
be the default
            {
                 setControlEncoding("UTF-8");
                 _controlInput_ =
                     new BufferedReader(new InputStreamReader(_input_, 
getControlEncoding())); //Changed from CRLFLineReader for my purposes
                 _controlOutput_ =
                    new BufferedWriter(new OutputStreamWriter(_output_, 
getControlEncoding()));
            }
            // restore the original reply (server greeting)
            _replyLines.clear();
            _replyLines.addAll(oldReplyLines);
            _replyCode = oldReplyCode;
        }
    }
{noformat}
At least then I can choose to use the BufferedReader instead and deal with 
whatever issues I hit.

So I'm 100% not saying the CRLFLineReader should be reverted, I'm asking if we 
can look at a way of having the strictness be optional. Even restructuring to 
allow someone to override a method that lets you pass in your reader would 
suffice but a property that allows you to toggle it would certainly be more 
user-friendly similar to what setStrictMultilineParsing(boolean 
strictMultilineParsing) allows you to do currently.


was (Author: unsta):
So I was hoping originally to be able to just use the library as-is and extend 
FTPClient in my code and override _connectAction_ but to recreate it and use a 
different reader as it is I couldn't as __initDefaults() and  
__autodetectEncoding are both private and not accessible by subclasses. Maybe 
I'm just missing something here but it doesn't seem possible with the current 
code.

(from commons-net-3.3)

{noformat}
    protected void _connectAction_() throws IOException
    {
        super._connectAction_(); // sets up _input_ and _output_
        __initDefaults();
        // must be after super._connectAction_(), because otherwise we get an
        // Exception claiming we're not connected
        if ( __autodetectEncoding )
        {
            ArrayList<String> oldReplyLines = new ArrayList<String> 
(_replyLines);
            int oldReplyCode = _replyCode;
            if ( hasFeature("UTF8") || hasFeature("UTF-8")) // UTF8 appears to 
be the default
            {
                 setControlEncoding("UTF-8");
                 _controlInput_ =
                     new BufferedReader(new InputStreamReader(_input_, 
getControlEncoding()));
                 _controlOutput_ =
                    new BufferedWriter(new OutputStreamWriter(_output_, 
getControlEncoding()));
            }
            // restore the original reply (server greeting)
            _replyLines.clear();
            _replyLines.addAll(oldReplyLines);
            _replyCode = oldReplyCode;
        }
    }
{noformat}
At least then I can choose to use the BufferedReader instead and deal with 
whatever issues I hit.

So I'm 100% not saying the CRLFLineReader should be reverted, I'm asking if we 
can look at a way of having the strictness be optional. Even restructuring to 
allow someone to override a method that lets you pass in your reader would 
suffice but a property that allows you to toggle it would certainly be more 
user-friendly similar to what setStrictMultilineParsing(boolean 
strictMultilineParsing) allows you to do currently.

> 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)

Reply via email to