RFR: 7100957 : Java doesn't correctly handle the SOCKS protocol when used over IPv6

[Dimitar Mavrodiev]

Thanks, Chris.

I've removed the dependency on the internal HttpServer. Here's another
webrev
https://googledrive.com/host/0B2CI6Ih--1t5bVVwbVlBRmpVMDg/2/index.html.

Best,
Dimitar


On Mon, Jan 6, 2014 at 4:18 PM, Chris Hegarty <chris.hegarty at oracle.com>wrote:

> The source changes look good to me too.
>
> I see Alan has commented on the test, and I agree. Trivially, can you also
> remove the dependency on the old internal HttpServer, and use the newer
> com.sun.net.httpserver API. It is easier to user, and more robust. You can
> see an example here [1].
>
> -Chris.
>
> [1] http://hg.openjdk.java.net/jdk9/dev/jdk/file/tip/test/
> sun/net/www/protocol/http/StreamingOutputStream.java.
>
>
>
> On 05/01/14 11:09, Alan Bateman wrote:
>
>> On 03/01/2014 11:04, Dimitar Mavrodiev wrote:
>>
>>> Greetings all,
>>>
>>> I've fixed this and created a test to cover it, is there a sponsor who
>>> could push this through? Here's a link to the webrev
>>> https://googledrive.com/host/0B2CI6Ih--1t5bVVwbVlBRmpVMDg/index.html.
>>>
>>> It's a simple fix that correctly consumes the bytes from a SOCKS reply
>>> which represent an IPv6 address or a domain name. I also had to fix
>>> SocksServer as it was not correctly constructing a String
>>> representation of an IPv6 address from byte[].
>>>
>>> I didn't find it necessary to cover the case of DOMAIN_NAME in the
>>> test as name resolution happens locally and not on the SOCKS server,
>>> which is perhaps worth another fix.
>>>
>>>  Thanks for the patch and I see that your OCA has been processed.
>>
>> I checked section 5 of RFC 1928 and it does indeed appear that the
>> DOMAINNAME (0x03) and IP V6 address (0x04) cases were not implemented
>> correctly. Your patch looks right. In passing, I see that the constants
>> for SOCKS are defined in an interface (which is an anti-pattern) and we
>> should clean that up at some point (not necessary for this patch of
>> course).
>>
>> On the test then I think it will need to check that IPv6 is enabled as
>> part of the setup, otherwise it looks like it will fail. I realize that
>> IPv6 is enabled by default everywhere these days but we regularly test
>> on machine that don't have it configured. One other thing about the test
>> is that it will require a GPL header. Would you have cycles to expand
>> the SOCKS test infrastructure to cover the DOMAIN_NAME case? I ask about
>> that case because it was the lack of test coverage that meant this
>> mis-handling slipped through (although I don't think it is actually used
>> and doesn't appear to have been noticed before).
>>
>> Otherwise, I think this is a good (and I would be happy to sponsor it).
>>
>> -Alan.
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/net-dev/attachments/20140106/84ad0760/attachment-0001.html