[12] RFR 8187522: sun/net/ftp/FtpURLConnectionLeak.Java timed out

[Chris Yin]

Hi, Chris H.

Thanks for your review and comments, I just added a comment as you suggested and update new webrev as below, please have a review.

http://cr.openjdk.java.net/~xyin/8187522/webrev.01/

Thanks,
Chris Y.

> On 11 Oct 2018, at 7:00 PM, Chris Hegarty <chris.hegarty at oracle.com> wrote:
> 
> Chris Y,
> 
>> On 11 Oct 2018, at 09:45, Chris Yin <xu.y.yin at oracle.com <mailto:xu.y.yin at oracle.com>> 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)
>> 
>> 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/>
> 
> I think this is good.
> 
> The `dash` character in FtpCommandHandler is subtle. I kinda
> regret allowing that change, rather than creating a separate
> standalone test for 8151586. It just confusing and overloads
> the test infra. It might ok at this point, but a comment would
> be good.
> 
> -Chris. 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/net-dev/attachments/20181012/12f1e246/attachment.html>