[ 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:49 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) 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; } } 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) 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; } } 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)