> On 11 Oct 2018, at 12:02, vyom tewari <vyom.tew...@oracle.com> wrote: > On Thursday 11 October 2018 02:15 PM, Chris Yin wrote: >> Please review below small change to fix test >> sun/net/ftp/FtpURLConnectionLeak.Java, thanks >> >> Besides the original timeout issue, looks like the test not working as >> expected even the results is pass. Per test description, we expect >> FileNotFoundException and then verify connection closed, but look through >> recent test run log, it’s actually got “sun.net.ftp.FtpLoginException: >> Invalid username/password” and this exception been caught in IOException >> block coincidentally, so the test result is pass, but it never hit >> FileNotFoundException. The fix change will remove IOException catch block >> and close server socket in close() of try-with-resouce to avoid possible >> invalid exception catch and connection issue, also ftp 220 response message >> was modified to allow FtpClient working correctly (I leave the corrupted >> message style as it is since looks like it will be used to test JDK-8151586) > you are right, corrupted message style is intentional. Your code changes > looks good to me but i am not able to understand "FtpCommandHandler.java" > changes. > > I can see that you inserted one "-" , is it intentional ?
If the 4th char is a dash, then the implementation ( for what it’s worth ) thinks that it has seen multi-line reply `###-` sequence. It needs a comment, or maybe revert prior to 8151586. And a separate specific test added for 8151586. -Chris. > Thanks, > Vyom >> >> bug: https://bugs.openjdk.java.net/browse/JDK-8187522 >> <https://bugs.openjdk.java.net/browse/JDK-8187522> >> webrev: http://cr.openjdk.java.net/~xyin/8187522/webrev.00/ >> <http://cr.openjdk.java.net/~xyin/8187522/webrev.00/> >> >> Regards, >> Chris >
