Re: Code Review 6964547: Impossible to set useV4 in SocksSocketImpl

2011-01-13 Thread Alan Bateman
Chris Hegarty wrote: : Yes, I should know better ( after trying to debug many of these spurious failures ). http://cr.openjdk.java.net/~chegar/6964547/webrev.02/webrev/test/java/net/Socks/SocksProxyVersion.java.html This looks fine to me. -Alan

Re: Code Review 6964547: Impossible to set useV4 in SocksSocketImpl

2011-01-13 Thread Chris Hegarty
On 01/13/11 09:29 AM, Alan Bateman wrote: Chris Hegarty wrote: : Ah yes, I just moved the socket close to after the version check. The client will be blocked (in the socks protocol handhake) until the socket on the server side is closed. Updated test: http://cr.openjdk.java.net/~chegar/6964547

Re: Code Review 6964547: Impossible to set useV4 in SocksSocketImpl

2011-01-13 Thread Alan Bateman
Chris Hegarty wrote: : Ah yes, I just moved the socket close to after the version check. The client will be blocked (in the socks protocol handhake) until the socket on the server side is closed. Updated test: http://cr.openjdk.java.net/~chegar/6964547/webrev.02/webrev/test/java/net/Socks/So

Re: Code Review 6964547: Impossible to set useV4 in SocksSocketImpl

2011-01-13 Thread Chris Hegarty
On 01/12/11 06:25 PM, Alan Bateman wrote: Chris Hegarty wrote: : Updated Webrev: http://cr.openjdk.java.net/~chegar/6964547/webrev.02/webrev/ Looks fine except that that main thread needs to wait for the "socks server" thread to terminate as otherwise the test might pass before the thread sets

Re: Code Review 6964547: Impossible to set useV4 in SocksSocketImpl

2011-01-12 Thread Alan Bateman
Chris Hegarty wrote: : Updated Webrev: http://cr.openjdk.java.net/~chegar/6964547/webrev.02/webrev/ Looks fine except that that main thread needs to wait for the "socks server" thread to terminate as otherwise the test might pass before the thread sets failed to true. -Alan

Re: Code Review 6964547: Impossible to set useV4 in SocksSocketImpl

2011-01-12 Thread Chris Hegarty
On 01/12/11 03:24 PM, Alan Bateman wrote: Chris Hegarty wrote: I received some offline comments on this. 1) SocksProxyVersionFour -> SocksProxy 2) SocksProxy now contains the protocol version Updated webrev: http://cr.openjdk.java.net/~chegar/6964547/webrev.01/webrev/ -Chris. This looks muc

Re: Code Review 6964547: Impossible to set useV4 in SocksSocketImpl

2011-01-12 Thread Chris Hegarty
On 01/12/11 03:36 PM, Alan Bateman wrote: Damjan Jovanovic wrote: Can I also suggest that Proxy.Type.SOCKS gets deprecated and Proxy.Type.SOCKS4 and Proxy.Type.SOCKS5 get introduced instead? Just about every application treats SOCKS 4 and 5 as different proxy types, only Java tries to lump th

Re: Code Review 6964547: Impossible to set useV4 in SocksSocketImpl

2011-01-12 Thread Alan Bateman
Damjan Jovanovic wrote: Can I also suggest that Proxy.Type.SOCKS gets deprecated and Proxy.Type.SOCKS4 and Proxy.Type.SOCKS5 get introduced instead? Just about every application treats SOCKS 4 and 5 as different proxy types, only Java tries to lump them together. I wonder if SOCKS4 is still

Re: Code Review 6964547: Impossible to set useV4 in SocksSocketImpl

2011-01-12 Thread Damjan Jovanovic
Can I also suggest that Proxy.Type.SOCKS gets deprecated and Proxy.Type.SOCKS4 and Proxy.Type.SOCKS5 get introduced instead? Just about every application treats SOCKS 4 and 5 as different proxy types, only Java tries to lump them together. On Wed, Jan 12, 2011 at 5:04 PM, Chris Hegarty wrote: >

Re: Code Review 6964547: Impossible to set useV4 in SocksSocketImpl

2011-01-12 Thread Alan Bateman
Chris Hegarty wrote: I received some offline comments on this. 1) SocksProxyVersionFour -> SocksProxy 2) SocksProxy now contains the protocol version Updated webrev: http://cr.openjdk.java.net/~chegar/6964547/webrev.01/webrev/ -Chris. This looks much cleaner. A few minor comments on the chan

Re: Code Review 6964547: Impossible to set useV4 in SocksSocketImpl

2011-01-12 Thread Chris Hegarty
I received some offline comments on this. 1) SocksProxyVersionFour -> SocksProxy 2) SocksProxy now contains the protocol version Updated webrev: http://cr.openjdk.java.net/~chegar/6964547/webrev.01/webrev/ -Chris. On 01/11/11 03:00 PM, Chris Hegarty wrote: Michael, Alan, Provide a property